This document outlines a refactoring plan for the Lumacs codebase, focusing on improving maintainability, modularity, testability, and overall code quality. This plan is intended for execution by another LLM model.
EditorCore God Object: The EditorCore class currently handles a vast array of responsibilities, leading to high coupling and reduced modularity. The primary goal is to break down this monolithic class into smaller, more specialized, and cohesive components (manager classes, services).std::cerr debug outputs with a proper logging system for better debuggability and runtime insight.EditorCore's methods and member variables to identify distinct functional areas (e.g., buffer management, window layout, command execution, macro recording, kill ring management).BufferManager, WindowManager, KeybindingManager, MacroManager, KillRingManager, RegisterManager).EditorCore to their respective new manager classes.EditorCore to hold instances (preferably smart pointers) of these new manager classes.EditorCore methods to delegate calls to the appropriate manager classes.EditorCore and the new manager classes, and among the manager classes themselves, occurs through well-defined, minimal interfaces. This might involve abstract base classes or observer patterns where appropriate.test_framework.hpp.test_buffer.cpp and test_editor_core.cpp tests using the new framework's syntax and features.LuaApi, CommandSystem, and Window.std::cerr Calls:
std::cerr for debug/error output with appropriate calls to the new logging library.sol2 Dependency:
sol2 to a stable release tag or commit hash instead of the develop branch.sol2 is a more robust solution for stability.snake_case for functions/variables, PascalCase for classes).std::unique_ptr/std::shared_ptr, const correctness, and move semantics where appropriate.feat:, fix:, refactor:, test:, docs: prefixes). Explain why the change was made, not just what was changed.refactor(editor_core): Extract buffer management into BufferManager classinclude/lumacs/, src/, tests/).README.md, documentation/) if changes impact the project's architecture or build process.cmake --build build, ctest).This phase aims to create a modular, extensible, and UI-agnostic modeline system, addressing the hardcoded content, UI-specific rendering, and code duplication.
Subtask Y.1: Create a UI-Agnostic Modeline Content API:
src/tui_editor.cpp and src/gtk_editor.cpp. This leads to code duplication and makes it difficult to extend the modeline with new information or change its format without recompiling. The TODO: Add major mode support highlights this inflexibility.ModelineSegment interface (or a base class) that represents a piece of information to be displayed in the modeline (e.g., buffer name, cursor position, major mode, modification status).ModelineSegment would have a method to generate its display string and potentially its own styling information (face names).BufferNameSegment, ModificationStatusSegment, CursorPositionSegment, PercentageSegment.ModelineManager or ModelineComposer class responsible for assembling the complete modeline string by querying registered ModelineSegments. This manager should be able to operate on a given Window context.ModelineSegments (potentially via Lua API or configuration).Subtask Y.2: Integrate Modeline Content with UI Renderers:
TuiEditor and GtkEditor. TuiEditor has a render_status_line() that is never called, indicating dead or confusing code.TuiEditor::render_window_modeline() and GTK's modeline rendering functions (render_modeline_for_window, render_modeline) to obtain their content (a list of segmented strings and their associated face names) from the ModelineManager (or ModelineComposer).NCursesRenderer or GtkRenderer after theme decoupling from Phase X).render_status_line() from TuiEditor if it is indeed dead code. If it represents a global status bar distinct from per-window modelines, clarify its purpose and refactor it to use the new ModelineManager.Subtask Y.3: Enhance Modeline Configuration:
show_modeline config flag is global, not per-window, limiting flexibility.ModelineSegments in the configuration (e.g., via a Lua table in init.lua).Subtask Y.4: Remove Legacy ThemeElement Usage from Modeline:
Face system (from Phase X).ThemeElement::StatusLine and ThemeElement::StatusLineInactive which are part of the legacy theme system.ThemeElement is removed as part of Phase X, update modeline rendering to directly request face attributes for standardized face names like "mode-line" and "mode-line-inactive" (or their new standardized names, as per Subtask X.5).NCursesRenderer and GtkRenderer (or their color management components) will be responsible for resolving these face names to actual UI colors, further decoupling concerns.Subtask Y.5: Modularize TuiEditor and GtkEditor Rendering:
TuiEditor and GtkEditor by extracting specific rendering concerns into smaller, testable units.TuiEditor and GtkEditor classes are currently monolithic, handling all UI logic from input to rendering, making them large and harder to maintain or test.TuiWindowRenderer and GtkWindowRenderer classes that encapsulate how a single Window (including its modeline, text content, line numbers, etc.) is rendered in each backend. These could take a Window object and a ModelineManager to perform their rendering tasks. This would make TuiEditor and GtkEditor primarily orchestrators, managing the overall screen layout and delegating specific window rendering.Status Update: Modeline framework review completed. Identified issues have been documented under "Phase Y: Modeline Framework Refactoring".
This phase aims to centralize and modularize the minibuffer logic, making it UI-agnostic, extensible, and reducing code duplication across frontends. The goal is to achieve Emacs-like extensibility where new interactive commands can be added easily without modifying core C++ UI code.
Subtask Z.1: Create a Centralized Minibuffer Manager:
MinibufferManager class.TuiEditor and GtkEditor. This significantly hinders extensibility and maintainability.MinibufferManager (or similar, e.g., PromptManager). This manager should not depend on UI-specific libraries (NCurses, GTK).MinibufferPromptType enum), current input (command_buffer_), history references, and completion-related state (completion_candidates_, completion_index_, etc.) into MinibufferManager.Enter, Escape, Tab, Backspace within a minibuffer context) into methods of MinibufferManager.MinibufferManager should interact with EditorCore for high-level actions (e.g., load_file, switch_buffer, execute_command) via injected interfaces (e.g., ICommandExecutor, IBufferManager).TuiEditor and GtkEditor will then delegate minibuffer input events to this central manager and query it for current state/content.Subtask Z.2: Decouple Minibuffer History Management:
HistoryManager component.TuiEditor (separate histories) and GtkEditor (single history for all modes, with internal logic for navigation).HistoryManager class that can store and navigate a std::vector<std::string>.MinibufferManager would then own instances of HistoryManager for each distinct type of history (command, file path, buffer name, isearch query).TuiEditor and GtkEditor.Subtask Z.3: Create an Extensible Completion System:
ICompletionSource interface with a method like std::vector<CompletionCandidate> get_candidates(const std::string& input_text).ICompletionSources for commands, buffer names, file paths, theme names, etc. These would query EditorCore or CommandSystem for data.MinibufferManager would hold a map of MinibufferPromptType to ICompletionSource*, allowing it to dynamically get candidates based on the active mode.update_completion_candidates() (TUI) and run_completion() (GTK) to delegate to this new system.Subtask Z.4: Externalize Minibuffer Prompts and Message Display:
message_line_ variable is overloaded for prompts, messages, and partial key sequences.MinibufferManager should expose methods to retrieve the current prompt text and the current input buffer content.minibuffer-prompt-function).MinibufferPrompt (interactive input) from TransientMessage (non-interactive, temporary display). These should be managed and displayed independently by the UI.Subtask Z.5: UI-Agnostic Minibuffer Input Handling:
TuiEditor and GtkEditor should simply pass resolved key events to the MinibufferManager when a minibuffer mode is active.TuiEditor::handle_input() and GtkEditor::on_key_pressed() will call a single MinibufferManager::handle_key_event(std::string key_name) method when the editor is in a minibuffer-related mode.MinibufferManager will then update its internal state, history, and completion, and return a result indicating if the event was handled, if the mode should exit, and if a UI redraw is needed.Subtask Z.6: Refactor Minibuffer Rendering in Frontends:
TuiEditor and GtkEditor to primarily focus on displaying content provided by MinibufferManager.render_message_line() (TUI) and render_minibuffer() (GTK) will query the MinibufferManager for the text to display (prompt + input buffer) and any associated styling information (e.g., current completion match)."minibuffer-prompt", "minibuffer-input", "minibuffer-completion", "minibuffer-match" (aligned with Face system refactoring from Phase X) to style the output.GtkMinibufferRenderer class) that interprets the generic minibuffer face attributes.Subtask Z.7: Remove Legacy ThemeElement Usage from Minibuffer:
Face system (from Phase X).ThemeElement::MinibufferPrompt, MinibufferInput, etc., which are part of the legacy theme system.ThemeElement is removed as part of Phase X, update minibuffer rendering to directly request face attributes for standardized face names like "minibuffer-prompt", "minibuffer-input", etc.Status Update: Minibuffer framework review completed. Identified issues have been documented under "Phase Z: Minibuffer Framework Refactoring".
This phase focuses on improving the modularity, extensibility, and separation of concerns within the GTK frontend, and addressing specific user requests (removal of context menus and tooltips).
Subtask A.1: Remove Context Menu and Tooltips:
create_widget_for_layout_node.GtkEditor::create_widget_for_layout_node(), remove the creation and connection of context_menu_controller and its associated signal_pressed handler.GtkEditor::create_widget_for_layout_node(), remove the creation and connection of motion_controller and its associated signal_motion handler, including calls to drawing_area->set_tooltip_text().Subtask A.2: Decouple GTK Rendering from GtkEditor:
GtkEditor.GtkEditor is currently responsible for all aspects of GTK rendering, including Cairo drawing, Pango layout management, font metrics, and displaying various editor elements. This makes the class very large and hard to manage.GtkRenderer or GtkDrawingContext class that encapsulates the Cairo drawing logic, Pango layout management, and font metrics.on_draw(), draw_window(), render_modeline_for_window(), render_minibuffer(), and render_modeline() (if kept) methods into this new GtkRenderer class.GtkRenderer, performing it once or only when font settings change, rather than repeatedly.GtkEditor would then own an instance of GtkRenderer and delegate drawing operations to it.Subtask A.3: Integrate with Modeline and Minibuffer Managers:
GtkEditor contains duplicated logic for minibuffer state and modeline content generation, violating DRY.GtkEditor's on_key_pressed will delegate minibuffer input processing to the MinibufferManager (from Phase Z) when in a minibuffer mode.GtkRenderer's render_minibuffer will query the MinibufferManager for content (prompt, input buffer, completion candidates) (from Phase Z).GtkRenderer's render_modeline_for_window will query the ModelineManager for modeline content (from Phase Y).on_key_pressed and the rendering functions within GtkEditor.Subtask A.4: Refine GTK Widget Tree Construction and Event Handling:
GtkEditor::create_widget_for_layout_node and separate event handling responsibilities.create_widget_for_layout_node is burdened with creating widgets and attaching all input controllers (key, click, drag, scroll).GtkWindowController class that encapsulates the interactive behavior of a single DrawingArea (aligning with Subtask Y.5).create_widget_for_layout_node should primarily focus on building the widget hierarchy (Paned, DrawingArea) based on LayoutNodes, returning managed widgets, and not on attaching all interaction logic directly.Subtask A.5: Improve GTK Theme Integration:
apply_face_attributes and various rendering functions still rely on ThemeElement or direct color values rather than a fully abstracted FaceAttributes system, and the conversion from Color to GTK's color representation is mixed with drawing.apply_face_attributes (or its equivalent in the new GtkRenderer) to primarily take generic FaceAttributes objects (foreground, background, weight, slant, etc.) and map them to Pango::Attribute objects without direct reliance on ThemeElement.Theme object (via the GtkRenderer), which will handle their conversion to GTK-specific drawing commands.Subtask A.6: Move C++ Fallback Keybindings to Lua:
GtkEditor::on_key_pressed contains C++ fallback implementations for many common editing actions (e.g., Return, Backspace, Delete, Arrow keys, PageUp/Down, Home/End, character insertion). This duplicates functionality that should be managed by the Lua keybinding system.init.lua using the existing keybinding system.GtkEditor::on_key_pressed to simplify the C++ code and allow Lua to be the single source of truth for key bindings.Status Update: GTK Frontend review completed. Identified issues and improvements, including removal of context menus and tooltips, have been documented under "Phase A: GTK Frontend Refactoring".
This phase aims to create a robust, extensible, and performant keybinding system, deeply integrated with a centralized Command System, to achieve Emacs-like customizability.
Subtask B.1: Centralize Command Execution:
KeyBindingManager from direct function execution by introducing a dedicated CommandSystem.KeyBindingManager directly executes std::function<bool()> objects, making it difficult to manage named commands, pass arguments, or allow Lua to register/execute commands seamlessly. This limits extensibility and introspection (like listing all commands) challenging.KeyBinding so that KeyBindingManager stores std::string command_name (or a command ID) instead of a std::function.CommandSystem class (which will be further detailed in the next code review topic) responsible for:
command_system.register("find-file", ...)).command_system.execute("find-file", args...)).std::functions or objects that implement a common interface, potentially with an interactive specification.KeyBindingManager::process_key will then look up the bound command_name and pass it to the CommandSystem for execution, rather than executing a direct std::function.Subtask B.2: Refine Canonical Key Representation:
Key struct with a name field and separate boolean flags (ctrl, meta, shift) can lead to ambiguity (e.g., is "C-a" name="a", ctrl=true or name="C-a", ctrl=false?), and its Key::parse function might not be robust enough for all modifier combinations (e.g., "C-M-ArrowUp"). The operator< comparison order is also arbitrary.Key to use a single, canonical representation. This could involve an enum class BaseKey { A, B, ..., Return, ArrowUp, F1, ... } for the base key and a uint16_t bitmask for modifiers (e.g., Modifier::Control, Modifier::Meta, Modifier::Shift).Key::parse to robustly handle all expected input formats (e.g., "C-x", "M-ArrowUp", "S-f", "C-M-Delete") and convert them to this canonical representation.Key::to_string, Key::operator==, and Key::operator< to work with the new canonical representation, ensuring efficient lookups in std::map.Subtask B.3: Optimize KeyBindingManager Prefix Lookup with Trie:
KeyBindingManager::has_prefix_bindings_impl method performs a linear scan (O(N) where N is the number of bindings) through all registered keybindings to check for prefixes. This can become a performance bottleneck with a large number of bindings on every key press within a multi-key sequence.std::map<KeySequence, KeyBinding> with a trie (prefix tree) data structure.Key in a sequence. Nodes would store a flag indicating if an exact binding ends there, or if it serves as a prefix for other longer bindings.bind, unbind, process_key, has_exact_binding, and has_prefix_bindings_impl to utilize the trie for O(L) (length of sequence) lookups, providing much faster performance.Subtask B.4: Eliminate C++ Fallback Keybindings in UI Frontends:
KeyBindingManager and the new CommandSystem.TuiEditor::process_key and GtkEditor::on_key_pressed contain C++ fallback logic for many fundamental editing actions (e.g., navigation, basic text insertion/deletion). This bypasses the extensible keybinding system, making these actions un-rebindable by users.core_->move_up(), core_->buffer().insert_char(), core_->buffer().erase_char()) from TuiEditor and GtkEditor into distinct, named commands registered with the new CommandSystem.KeyBindingManager (likely in default C++ configuration or via init.lua).core_->lua_api()->process_key(key_name) (which will then delegate to KeyBindingManager and CommandSystem) for all key presses.Subtask B.5: Enhanced Error Reporting:
KeyBindingManager currently returns a generic KeyResult::Failed for any exception during std::function<bool()> execution, offering no details about the failure.CommandSystem should be designed to return a more descriptive result. This could be an std::optional<std::string> containing an error message, or a custom CommandResult struct.KeyBindingManager would then pass this detailed error information back to the UI frontend (e.g., to be displayed in the minibuffer as a transient message).Status Update: Keybinding System review completed. Identified issues and improvements have been documented under "Phase B: Keybinding System Refactoring".
This phase aims to enhance the Command System to support robust, type-safe, and interactive argument handling, and to fully integrate with the refactored Keybinding and Minibuffer systems, enabling true Emacs-like extensibility.
Subtask C.1: Implement Interactive Argument Gathering:
M-x).CommandFunction signature (const std::vector<std::string>&) only supports arguments provided upfront as strings. There's no mechanism for commands to request input from the user during their execution. The interactive flag in Command is just a boolean, lacking a specification for how arguments should be gathered.InteractiveSpec (e.g., a string or enum, similar to Emacs' (interactive "...") spec) within the Command struct. This spec would define how the command's arguments should be gathered when called interactively (e.g., "f" for file, "b" for buffer, "s" for string, "n" for number).CommandSystem::execute (or introduce a new execute_interactive method) to interpret this InteractiveSpec. When a command is invoked interactively, it would delegate to the MinibufferManager (from Phase Z) to prompt the user for each argument based on the spec, gather the input, and then call the command's CommandFunction with the collected arguments.Subtask C.2: Redesign Command Argument Handling:
CommandFunctions and support for optional arguments.std::vector<std::string> for arguments is untyped, requiring boilerplate parsing and error checking within each command function.CommandFunction signature. Options include:
std::variant<int, std::string, bool, ...> for arguments, passed as std::vector<std::variant>.std::vector<ArgumentMetadata>), and the CommandSystem performs type conversion and validation.CommandSystem to handle conversion from strings (provided by interactive prompts or direct execution) to the native types.Subtask C.3: Consolidate Command String Parsing:
CommandSystem::execute_string internally calls parse_command_string, which parses a single string into a command name and std::vector<std::string> arguments. This parsing duplicates logic that the MinibufferManager (from Phase Z) should ideally handle when a user types a command in the minibuffer.CommandSystem::execute_string and parse_command_string should ideally be removed or deprecated.MinibufferManager (from Phase Z) should be solely responsible for parsing the full input string (command name + arguments) provided by the user in the minibuffer, and then calling CommandSystem::execute(command_name, parsed_args) with already tokenized and potentially typed arguments.Subtask C.4: Refine Completion System Integration:
CommandSystem with the extensible completion framework defined in Phase Z.CommandSystem has a mixed approach to completion, with specific complete_* methods (e.g., complete_buffer_name, complete_file_path) and a generic register_completion_provider. These specific methods often belong to other modules.CommandSystem::complete_buffer_name, complete_file_path, complete_theme_name methods.BufferManager, ThemeManager) should implement and provide their own ICompletionSource implementations.CommandSystem should primarily serve as a registry for command-specific CompletionProviders that might be triggered by an InteractiveSpec (from Subtask C.1).MinibufferManager (from Phase Z) should orchestrate the overall completion process, determining which ICompletionSource to use based on the current interactive command's context and its InteractiveSpec.FileSystemCompletionProvider is used as a utility by other ICompletionSource implementations, not directly exposed as a top-level completer from CommandSystem's public interface for completions.Subtask C.5: Improve EditorCore Dependency:
EditorCore within CommandSystem for better testability and modularity.CommandSystem holds a direct raw reference to EditorCore. While necessary for commands to interact with the editor, this creates tight coupling and makes CommandSystem harder to test in isolation.IBufferService, IWindowService, IConfigService, ILuaService) into CommandSystem's constructor instead of the full EditorCore reference. This would allow CommandSystem and its registered commands to be tested with mock services.EditorCore access, CommandSystem could provide well-defined facades or a limited context object to those commands.Subtask C.6: Lua API Integration for Commands:
LuaApi's interaction with the CommandSystem is not fully detailed but is crucial for extensibility.LuaApi (to be reviewed next) should expose CommandSystem::register_command and CommandSystem::execute in a way that allows Lua functions to be registered as CommandFunctions and Lua to call C++ commands by name with appropriate argument marshalling.CommandFunction arguments (aligned with Subtask C.2).LuaApi can correctly expose the InteractiveSpec of commands to allow Lua to implement interactive argument gathering.Status Update: Command System review completed. Identified issues and improvements have been documented under "Phase C: Command System Refactoring".