PLAN.md 36 KB

Refactoring Plan: Improving Codebase Maintainability

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.

Core Refactoring Goals

  1. Decompose the 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).
  2. Enhance Testing Infrastructure and Coverage: The existing custom testing framework is basic and test coverage is limited. The goal is to migrate to a modern, standard C++ testing framework and significantly increase test coverage, especially for newly modularized components.
  3. Implement Robust Logging: Replace ad-hoc std::cerr debug outputs with a proper logging system for better debuggability and runtime insight.
  4. Refine Dependency Management: Address potential stability risks associated with external dependencies.

Detailed Plan and Subtasks

Phase 1: Modularity and Decoupling (EditorCore Decomposition)

  • Subtask 1.1: Identify and Extract Sub-systems:
    • Analyze EditorCore's methods and member variables to identify distinct functional areas (e.g., buffer management, window layout, command execution, macro recording, kill ring management).
    • Create new classes or services for each identified sub-system (e.g., BufferManager, WindowManager, KeybindingManager, MacroManager, KillRingManager, RegisterManager).
  • Subtask 1.2: Migrate Responsibilities:
    • Move relevant member variables and methods from EditorCore to their respective new manager classes.
    • Update EditorCore to hold instances (preferably smart pointers) of these new manager classes.
    • Refactor EditorCore methods to delegate calls to the appropriate manager classes.
  • Subtask 1.3: Define Clear Interfaces:
    • Ensure that the interaction between 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.
  • Subtask 1.4: Manage Dependencies between new Modules:
    • Minimize direct dependencies between manager classes. Use dependency injection where possible to provide necessary collaborators.
    • Consider an event bus or messaging system for indirect communication between loosely coupled components, if suitable for the project's architecture.

Phase 2: Testing Infrastructure Upgrade and Coverage Expansion

  • Subtask 2.1: Select and Integrate a Standard Testing Framework:
    • Recommendation: Google Test or Catch2. Integrate the chosen framework into the CMake build system.
    • Remove or deprecate the custom test_framework.hpp.
  • Subtask 2.2: Migrate Existing Tests:
    • Rewrite test_buffer.cpp and test_editor_core.cpp tests using the new framework's syntax and features.
  • Subtask 2.3: Expand Test Coverage:
    • Write unit tests for all new manager classes created in Phase 1. Focus on testing their individual functionalities in isolation.
    • Increase test coverage for existing components, especially LuaApi, CommandSystem, and Window.
    • Implement integration tests to verify the interactions between the modularized components and the overall editor behavior.

Phase 3: Logging and Observability

  • Subtask 3.1: Integrate a C++ Logging Library:
    • Recommendation: spdlog or loguru. Integrate the chosen library into the CMake build system.
  • Subtask 3.2: Replace std::cerr Calls:
    • Replace all instances of std::cerr for debug/error output with appropriate calls to the new logging library.
    • Define different log levels (e.g., DEBUG, INFO, WARN, ERROR) and configure log sinks (e.g., console, file).

Phase 4: Dependency Management

  • Subtask 4.1: Review sol2 Dependency:
    • Investigate the possibility of pinning sol2 to a stable release tag or commit hash instead of the develop branch.
    • Evaluate if vendoring sol2 is a more robust solution for stability.

General Instructions for the LLM Executor

  • Adherence to Project Conventions:
    • Style: Maintain the existing C++ coding style, formatting, and naming conventions (e.g., snake_case for functions/variables, PascalCase for classes).
    • Modern C++: Continue to leverage C++20 features, std::unique_ptr/std::shared_ptr, const correctness, and move semantics where appropriate.
    • CMake: Integrate any new libraries or changes cleanly into the existing CMake build system.
  • Granular Git Commits:
    • Make frequent, small, and atomic commits. Each commit should represent a single logical change.
    • Commit Message Format: Follow conventional commit message guidelines (e.g., feat:, fix:, refactor:, test:, docs: prefixes). Explain why the change was made, not just what was changed.
    • Example: refactor(editor_core): Extract buffer management into BufferManager class
  • Keep Repository Tidy:
    • Ensure all new files are placed in logical directories (include/lumacs/, src/, tests/).
    • Remove old, unused code or files.
    • Update relevant documentation (e.g., README.md, documentation/) if changes impact the project's architecture or build process.
  • Incremental Approach:
    • Address one subtask at a time. Do not attempt large, sweeping changes in a single step.
    • After each significant refactoring step, ensure the project still builds and all existing tests pass before moving to the next step.
  • Verification:
    • Always run build, linting, and tests after each significant change. Identify the project's specific commands for these (e.g., cmake --build build, ctest).
    • Ensure strict compiler warnings remain enabled and no new warnings are introduced.
  • User Interaction:
    • If any ambiguity or complex decision arises during the refactoring process, clarify with the user. --- Status Update: Theme framework review completed. Identified issues have been documented under "Phase X: Theme Framework Refactoring".

Phase Y: Modeline Framework Refactoring

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:

    • Goal: Centralize modeline content generation, making it extensible and independent of the UI backend.
    • Problem: The content of modelines (buffer name, modification status, cursor position, percentage) is hardcoded within 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.
    • Actions:
      • Define a 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).
      • Each ModelineSegment would have a method to generate its display string and potentially its own styling information (face names).
      • Create concrete implementations for existing segments: BufferNameSegment, ModificationStatusSegment, CursorPositionSegment, PercentageSegment.
      • Introduce a 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.
      • Allow registration/deregistration of ModelineSegments (potentially via Lua API or configuration).
  • Subtask Y.2: Integrate Modeline Content with UI Renderers:

    • Goal: Ensure each UI backend (TUI, GTK) uses the centralized modeline content API and handles rendering specific details.
    • Problem: Modeline rendering is currently intertwined with content generation in both TuiEditor and GtkEditor. TuiEditor has a render_status_line() that is never called, indicating dead or confusing code.
    • Actions:
      • Modify 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).
      • The UI renderers would then be responsible for displaying the segmented content, applying appropriate faces (which will be resolved by the respective NCursesRenderer or GtkRenderer after theme decoupling from Phase X).
      • Remove the global 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:

    • Goal: Provide flexible configuration for modeline elements and their order.
    • Problem: The order and content of modeline elements are hardcoded. The show_modeline config flag is global, not per-window, limiting flexibility.
    • Actions:
      • Allow users to specify the order and presence of ModelineSegments in the configuration (e.g., via a Lua table in init.lua).
      • Investigate and implement (if desired) an option for per-window modeline visibility or specific modeline configurations per window/buffer type.
  • Subtask Y.4: Remove Legacy ThemeElement Usage from Modeline:

    • Goal: Align modeline styling with the unified Face system (from Phase X).
    • Problem: Modeline rendering in both TUI and GTK currently relies on ThemeElement::StatusLine and ThemeElement::StatusLineInactive which are part of the legacy theme system.
    • Actions:
      • Once 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).
      • The 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:

    • Goal: Reduce the "God Object" nature of TuiEditor and GtkEditor by extracting specific rendering concerns into smaller, testable units.
    • Problem: The TuiEditor and GtkEditor classes are currently monolithic, handling all UI logic from input to rendering, making them large and harder to maintain or test.
    • Actions:
      • Consider introducing 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".

Phase Z: Minibuffer 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:

    • Goal: Extract all core minibuffer logic (state management, input processing, history, completion orchestration, command dispatch) into a single, UI-agnostic MinibufferManager class.
    • Problem: The minibuffer state machine (modes, input handling, execution) is duplicated and tightly coupled within both TuiEditor and GtkEditor. This significantly hinders extensibility and maintainability.
    • Actions:
      • Define MinibufferManager (or similar, e.g., PromptManager). This manager should not depend on UI-specific libraries (NCurses, GTK).
      • Move the concept of minibuffer modes (e.g., MinibufferPromptType enum), current input (command_buffer_), history references, and completion-related state (completion_candidates_, completion_index_, etc.) into MinibufferManager.
      • Move input processing logic (e.g., handling Enter, Escape, Tab, Backspace within a minibuffer context) into methods of MinibufferManager.
      • The 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:

    • Goal: Create a reusable, UI-agnostic HistoryManager component.
    • Problem: History management logic is duplicated across TuiEditor (separate histories) and GtkEditor (single history for all modes, with internal logic for navigation).
    • Actions:
      • Create a generic HistoryManager class that can store and navigate a std::vector<std::string>.
      • The MinibufferManager would then own instances of HistoryManager for each distinct type of history (command, file path, buffer name, isearch query).
      • Remove duplicated history logic from TuiEditor and GtkEditor.
  • Subtask Z.3: Create an Extensible Completion System:

    • Goal: Allow dynamic registration of completion sources for various minibuffer modes.
    • Problem: Completion logic is duplicated and hardcoded for specific modes within each UI frontend. Adding new completion types (e.g., LSP symbols, variables) is difficult.
    • Actions:
      • Define an ICompletionSource interface with a method like std::vector<CompletionCandidate> get_candidates(const std::string& input_text).
      • Implement concrete ICompletionSources for commands, buffer names, file paths, theme names, etc. These would query EditorCore or CommandSystem for data.
      • The MinibufferManager would hold a map of MinibufferPromptType to ICompletionSource*, allowing it to dynamically get candidates based on the active mode.
      • Refactor update_completion_candidates() (TUI) and run_completion() (GTK) to delegate to this new system.
  • Subtask Z.4: Externalize Minibuffer Prompts and Message Display:

    • Goal: Enable dynamic and customizable minibuffer prompts.
    • Problem: Minibuffer prompts are hardcoded strings in both frontends, limiting customization. The message_line_ variable is overloaded for prompts, messages, and partial key sequences.
    • Actions:
      • The MinibufferManager should expose methods to retrieve the current prompt text and the current input buffer content.
      • Allow prompts to be generated dynamically (e.g., via Lua functions, similar to Emacs' minibuffer-prompt-function).
      • Clearly separate 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:

    • Goal: Both TuiEditor and GtkEditor should simply pass resolved key events to the MinibufferManager when a minibuffer mode is active.
    • Actions:
      • 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.
      • The 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:

    • Goal: Simplify minibuffer rendering in TuiEditor and GtkEditor to primarily focus on displaying content provided by MinibufferManager.
    • Actions:
      • 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).
      • They will use faces like "minibuffer-prompt", "minibuffer-input", "minibuffer-completion", "minibuffer-match" (aligned with Face system refactoring from Phase X) to style the output.
      • GTK's specific rendering details (e.g., separator line, background shade) should be managed by a GTK-specific rendering component (potentially a GtkMinibufferRenderer class) that interprets the generic minibuffer face attributes.
  • Subtask Z.7: Remove Legacy ThemeElement Usage from Minibuffer:

    • Goal: Align minibuffer styling with the unified Face system (from Phase X).
    • Problem: Minibuffer rendering currently relies on ThemeElement::MinibufferPrompt, MinibufferInput, etc., which are part of the legacy theme system.
    • Actions:
      • Once 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".

Phase A: GTK Frontend 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:

    • Goal: Eliminate the automatically generated right-click context menu and mouse-hover tooltips as per user request.
    • Problem: The current implementation adds an unwanted context menu on right-click and tooltips on mouse hover directly in create_widget_for_layout_node.
    • Actions:
      • In GtkEditor::create_widget_for_layout_node(), remove the creation and connection of context_menu_controller and its associated signal_pressed handler.
      • In 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:

    • Goal: Extract rendering logic into specialized, testable components to reduce the "God Object" nature of GtkEditor.
    • Problem: 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.
    • Actions:
      • Introduce a GtkRenderer or GtkDrawingContext class that encapsulates the Cairo drawing logic, Pango layout management, and font metrics.
      • Move on_draw(), draw_window(), render_modeline_for_window(), render_minibuffer(), and render_modeline() (if kept) methods into this new GtkRenderer class.
      • Centralize font metrics calculation and caching within the GtkRenderer, performing it once or only when font settings change, rather than repeatedly.
      • The GtkEditor would then own an instance of GtkRenderer and delegate drawing operations to it.
  • Subtask A.3: Integrate with Modeline and Minibuffer Managers:

    • Goal: Ensure GTK frontend fully utilizes the centralized, UI-agnostic modeline and minibuffer logic.
    • Problem: GtkEditor contains duplicated logic for minibuffer state and modeline content generation, violating DRY.
    • Actions:
      • 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).
      • This integration will significantly reduce the size and complexity of on_key_pressed and the rendering functions within GtkEditor.
  • Subtask A.4: Refine GTK Widget Tree Construction and Event Handling:

    • Goal: Simplify GtkEditor::create_widget_for_layout_node and separate event handling responsibilities.
    • Problem: create_widget_for_layout_node is burdened with creating widgets and attaching all input controllers (key, click, drag, scroll).
    • Actions:
      • The event controllers for key, click, drag, scroll should be managed by a more specialized component, possibly a 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:

    • Goal: Align GTK theme application with the decoupled and unified Face system (from Phase X).
    • Problem: The 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.
    • Actions:
      • Modify 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.
      • Ensure all GTK drawing components obtain face attributes from the UI-agnostic Theme object (via the GtkRenderer), which will handle their conversion to GTK-specific drawing commands.
  • Subtask A.6: Move C++ Fallback Keybindings to Lua:

    • Goal: Centralize keybinding definitions and logic in Lua, removing redundant C++ fallbacks.
    • Problem: 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.
    • Actions:
      • Implement these functionalities as Lua commands and bind them in init.lua using the existing keybinding system.
      • Remove these fallback C++ implementations from 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".

Phase B: Keybinding System 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:

    • Goal: Decouple KeyBindingManager from direct function execution by introducing a dedicated CommandSystem.
    • Problem: 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.
    • Actions:
      • Modify KeyBinding so that KeyBindingManager stores std::string command_name (or a command ID) instead of a std::function.
      • Introduce a CommandSystem class (which will be further detailed in the next code review topic) responsible for:
        • Registering commands by name (e.g., command_system.register("find-file", ...)).
        • Executing a command by name (command_system.execute("find-file", args...)).
        • Commands should be 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:

    • Goal: Ensure a consistent, unambiguous, and efficient representation of individual keys and their modifiers.
    • Problem: The current 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.
    • Actions:
      • Redesign 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).
      • Update 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.
      • Update 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:

    • Goal: Improve performance of multi-key sequence processing.
    • Problem: The 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.
    • Actions:
      • Replace the internal std::map<KeySequence, KeyBinding> with a trie (prefix tree) data structure.
      • Each node in the trie would correspond to a 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.
      • Update 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:

    • Goal: Centralize all keybindings within the KeyBindingManager and the new CommandSystem.
    • Problem: 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.
    • Actions:
      • Move all existing C++ fallback implementations (e.g., core_->move_up(), core_->buffer().insert_char(), core_->buffer().erase_char()) from TuiEditor and GtkEditor into distinct, named commands registered with the new CommandSystem.
      • Bind these new commands to their default key sequences (e.g., "ArrowUp", "Backspace", "Return", "a") within the KeyBindingManager (likely in default C++ configuration or via init.lua).
      • The UI frontends should then only call 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:

    • Goal: Provide more informative feedback to the user when command execution fails due to a keybinding.
    • Problem: The KeyBindingManager currently returns a generic KeyResult::Failed for any exception during std::function<bool()> execution, offering no details about the failure.
    • Actions:
      • If Subtask B.1 is implemented (decoupling command execution), the 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.
      • The 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".

Phase C: Command 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:

    • Goal: Allow commands to dynamically prompt the user for arguments when called interactively (e.g., via M-x).
    • Problem: The current 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.
    • Actions:
      • Introduce an 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).
      • Modify 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:

    • Goal: Provide type-safe argument passing to CommandFunctions and support for optional arguments.
    • Problem: std::vector<std::string> for arguments is untyped, requiring boilerplate parsing and error checking within each command function.
    • Actions:
      • Define a more flexible CommandFunction signature. Options include:
        • Using std::variant<int, std::string, bool, ...> for arguments, passed as std::vector<std::variant>.
        • Allowing commands to define their expected arguments as a metadata structure (e.g., std::vector<ArgumentMetadata>), and the CommandSystem performs type conversion and validation.
        • A context object passed to the command could provide typed access to arguments.
      • The chosen approach should allow commands to declare their required arguments and types, and for CommandSystem to handle conversion from strings (provided by interactive prompts or direct execution) to the native types.
  • Subtask C.3: Consolidate Command String Parsing:

    • Goal: Remove duplicated and potentially inconsistent command string parsing logic.
    • Problem: 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.
    • Actions:
      • CommandSystem::execute_string and parse_command_string should ideally be removed or deprecated.
      • The 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:

    • Goal: Fully integrate CommandSystem with the extensible completion framework defined in Phase Z.
    • Problem: The 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.
    • Actions:
      • Remove specific CommandSystem::complete_buffer_name, complete_file_path, complete_theme_name methods.
      • Instead, relevant domain managers (e.g., BufferManager, ThemeManager) should implement and provide their own ICompletionSource implementations.
      • The CommandSystem should primarily serve as a registry for command-specific CompletionProviders that might be triggered by an InteractiveSpec (from Subtask C.1).
      • The 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.
      • Ensure 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:

    • Goal: Reduce direct and broad dependency on EditorCore within CommandSystem for better testability and modularity.
    • Problem: 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.
    • Actions:
      • Consider injecting more specific service interfaces (e.g., 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.
      • If some commands truly require broad EditorCore access, CommandSystem could provide well-defined facades or a limited context object to those commands.
  • Subtask C.6: Lua API Integration for Commands:

    • Goal: Ensure seamless registration and execution of commands from Lua, including argument marshalling.
    • Problem: The current LuaApi's interaction with the CommandSystem is not fully detailed but is crucial for extensibility.
    • Actions:
      • The 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.
      • This will require careful handling of type conversion between Lua values and C++ CommandFunction arguments (aligned with Subtask C.2).
      • Ensure 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".