Add non-linear undo/redo design documentation and improve UndoSystem with backspace batching and GUI integration fixes.

2025-11-30 01:49:07 -08:00
parent 35ffe6d11c
commit b77ebdfd07
17 changed files with 891 additions and 147 deletions
--- a/docs/undo-state.md
+++ b/docs/undo-state.md
@@ -0,0 +1,95 @@
+Undo/Redo + C-k GUI status (macOS) — current state snapshot
+
+Context
+ - Platform: macOS (Darwin)
+ - Target: GUI build (kge) using SDL2/ImGui path
+ - Date: 2025-11-30 00:30 local (from user)
+
+What works right now
+ - Terminal (kte): C-k keymap and UndoSystem integration have been stable in recent builds.
+ - GUI: Most C-k mappings work: C-k d (KillToEOL), C-k x (Save+Quit), C-k q (Quit) — confirmed by user.
+ - UndoSystem core is implemented and integrated for InsertText/Newline/Delete/Backspace. Buffer owns an UndoSystem and raw edit APIs are used by apply().
+
+What is broken (GUI, macOS)
+ - C-k u: Status shows "Undone" but buffer content does not change (no visible undo).
+ - C-k U: Inserts a literal 'U' into the buffer; does not execute Redo.
+ - C-k C-u / C-k C-U: No effect (expected unmapped), but the k-prefix prompt can remain in some paths.
+
+Repro steps (GUI)
+ 1) Type "Hello".
+ 2) Press C-k then press u → status becomes "Undone", but text remains "Hello".
+ 3) Press C-k then press Shift+U → a literal 'U' is inserted (becomes "HelloU").
+ 4) Press C-k then hold Ctrl on the suffix and press u → status "Undone", still no change.
+ 5) Press C-k then hold Ctrl on the suffix and press Shift+U → status shows the k-prefix prompt again ("C-k _").
+
+Keymap and input-layer changes we attempted (and kept)
+ - KKeymap.cc: Case-sensitive 'U' mapping prioritized before the lowercase table. Added ctrl→non-ctrl fall-through so C-k u/U still map even if SDL reports Ctrl held on the suffix.
+ - TerminalInputHandler: already preserved case and mapped correctly.
+ - GUIInputHandler:
+   - Preserve case for k-prefix suffix letters (Shift → uppercase). Clear esc_meta before k-suffix mapping.
+   - Strengthened SDL_TEXTINPUT suppression after a k-prefix printable suffix to avoid inserting literal characters.
+   - Added fallback to map the k-prefix suffix in the SDL_TEXTINPUT path (to catch macOS cases where uppercase arrives in TEXTINPUT rather than KEYDOWN).
+   - Fixed malformed switch block introduced during iteration.
+ - Command layer: commit pending undo batch at k-prefix entry and just before Undo/Redo so prior typing can actually be undone/redone.
+
+Diagnostics added
+ - GUIInputHandler logs k-prefix u/U suffix attempts to stderr and (previously) /tmp/kge.log. The user’s macOS session showed only KEYDOWN logs for 'u':
+   - "[kge] k-prefix suffix: sym=117 mods=0x0 ascii=117 'u' ctrl2=0 pass_ctrl=0 mapped=1 id=38"
+   - "[kge] k-prefix suffix: sym=117 mods=0x80 ascii=117 'u' ctrl2=1 pass_ctrl=0 mapped=1 id=38"
+ - No logs were produced for 'U' (neither KEYDOWN nor TEXTINPUT). The /tmp log file was not created on the user’s host in the last run (stderr logs were visible earlier from KEYDOWN).
+
+Hypotheses for current failures
+ 1) Undo appears to be invoked (status "Undone"), but no state change:
+    - The most likely cause is that no committed node exists at the time of undo (i.e., typing "Hello" is not being recorded as an undo node), because our current typing path in Command.cc directly edits buffer rows without always driving UndoSystem Begin/Append/commit at the right times for every printable char on GUI.
+    - Although we call u->Begin(Insert) and u->Append(text) in cmd_insert_text for CommandId::InsertText, the GUI printable input might be arriving through a different path or being short-circuited (e.g., via a prompt or suppression), resulting in actual text insertion but no corresponding UndoSystem pending node content, or pending but never committed.
+    - We now commit at k-prefix entry and before undo; if there is still "nothing to undo", it implies the batch never had text appended (Append not called) or is detached from the real buffer edits.
+
+ 2) Redo via C-k U inserts a literal 'U':
+    - On macOS, uppercase letters often arrive as SDL_TEXTINPUT events. We added TEXTINPUT-based k-prefix mapping, but the user's run still showed a literal insertion and no diagnostic lines for TEXTINPUT, which suggests:
+      a) The TEXTINPUT suppression didn’t trigger for that platform/sequence, or
+      b) The k-prefix flag was already cleared by the time TEXTINPUT arrived, so the TEXTINPUT path defaulted to InsertText, or
+      c) The GUI window’s input focus or SDL event ordering differs from expectations (e.g., IME/text input settings), so our suppression/mapping didn’t see the event.
+
+Relevant code pointers
+ - Key mapping tables: KKeymap.cc → KLookupKCommand() (C-k suffix), KLookupCtrlCommand(), KLookupEscCommand().
+ - Terminal input: TerminalInputHandler.cc → map_key_to_command().
+ - GUI input: GUIInputHandler.cc → map_key() and GUIInputHandler::ProcessSDLEvent() (KEYDOWN + TEXTINPUT handling, suppression, k_prefix_/esc_meta_ flags).
+ - Command dispatch: Command.cc → cmd_insert_text(), cmd_newline(), cmd_backspace(), cmd_delete_char(), cmd_undo(), cmd_redo(), cmd_kprefix().
+ - Undo core: UndoSystem.{h,cc}, UndoNode.{h,cc}, UndoTree.{h,cc}. Buffer raw methods used by apply().
+
+Immediate next steps (when we return to this)
+ 1) Verify that GUI printable insertion always flows through CommandId::InsertText so UndoSystem::Begin/Append gets called. If SDL_TEXTINPUT delivers multi-byte strings, ensure Append() is given the same text inserted into buffer.
+    - Add a one-session debug hook in cmd_insert_text to assert/trace: pending node type/text length and current cursor col before/after.
+    - If GUI sometimes sends CommandId::InsertTextEmpty or another path, unify.
+
+ 2) Ensure batching rules are satisfied so Begin() reuses pending correctly:
+    - Begin(Insert) must see same row and col == pending->col + pending->text.size() for typing sequences.
+    - If GUI accumulates multiple characters per TEXTINPUT (e.g., pasted text), Append(std::string_view) is fine, but row/col expectations remain.
+
+ 3) For C-k U uppercase mapping on macOS:
+    - Add a temporary status dump when k-prefix suffix mapping falls back to TEXTINPUT path (we added stderr prints; also set Editor status with a short code like "K-TI U" during one session) to confirm path is used and suppression is working.
+    - If TEXTINPUT never arrives, force suppression: when we detect k-prefix and KEYDOWN of a letter with Shift, preemptively handle via KEYDOWN-derived uppercase ASCII rather than deferring.
+
+ 4) Consolidate k-prefix handling:
+    - After mapping a k-prefix suffix to a command (Undo/Redo/etc.), always set suppress_text_input_once_ = true to avoid any trailing TEXTINPUT.
+    - Clear k_prefix_ reliably on both KEYDOWN and TEXTINPUT paths.
+
+ 5) Once mapping is solid, remove all diagnostics and keep the minimal, deterministic logic.
+
+Open questions for future debugging
+ - Does SDL on this macOS setup deliver Shift+U as KEYDOWN+TEXTINPUT consistently, or only TEXTINPUT? We need a small on-screen debug to avoid relying on stderr.
+ - Are there any IME/TextInput SDL hints on macOS we should set for raw key handling during k-prefix?
+ - Should we temporarily disable SDL text input (SDL_StopTextInput) during k-prefix suffix processing to eliminate TEXTINPUT races on macOS?
+
+Notes on UndoSystem correctness (unrelated to the GUI mapping bug)
+ - Undo tree invariants are implemented: pending is detached; commit attaches and clears redo branches; undo/redo apply low-level Buffer edits with no public editor paths; saved marker updated via mark_saved().
+ - Dirty flag mirrors (current != saved).
+ - Delete batching supports prepend for backspace sequences (stored text is in increasing column order from anchor).
+ - Newline joins/splits are recorded as UndoType::Newline and committed immediately for single-step undo of line joins.
+
+Owner pointers & file locations
+ - GUI mapping and suppression: GUIInputHandler.cc
+ - Command layer commit boundaries: Command.cc (cmd_kprefix, cmd_undo, cmd_redo)
+ - Undo batching entry points: Command.cc (cmd_insert_text, cmd_backspace, cmd_delete_char, cmd_newline)
+
+End of snapshot — safe to resume from here.
--- a/docs/undo.md
+++ b/docs/undo.md
@@ -0,0 +1,140 @@
+This is a design for a non-linear undo/redo system for kte. The design must be identical in behavior and correctness
+to the proven kte editor undo system.
+
+### Core Requirements
+
+1. Each open buffer has its own completely independent undo tree.
+2. Undo and redo must be non-linear: typing after undo creates a branch; old redo branches are discarded.
+3. Typing, backspacing, and pasting are batched into word-level undo steps.
+4. Undo/redo must never create new undo nodes while applying an undo/redo (silent, low-level apply).
+5. The system must be memory-safe and leak-proof even if the user types and immediately closes the buffer.
+
+### Data Structures
+
+```cpp
+enum class UndoType : uint8_t {
+    Insert,
+    Delete,
+    Paste,        // optional, can reuse Insert
+    Newline,
+    DeleteRow,
+    // future: IndentRegion, KillRegion, etc.
+};
+
+struct UndoNode {
+    UndoType type;
+    int row;           // original cursor row
+    int col;           // original cursor column (updated during batch)
+    std::string text;  // the inserted or deleted text (full batch)
+    UndoNode* child = nullptr;   // next in current timeline
+    UndoNode* next  = nullptr;   // redo branch (rarely used)
+    // no parent pointer needed — we walk from root
+};
+
+struct UndoTree {
+    UndoNode* root    = nullptr;  // first edit ever
+    UndoNode* current = nullptr;  // current state of buffer
+    UndoNode* saved   = nullptr;  // points to node matching last save (for dirty flag)
+    UndoNode* pending = nullptr;  // in-progress batch (detached)
+};
+```
+
+Each `Buffer` owns one `std::unique_ptr<UndoTree>`.
+
+### Core API (must implement exactly)
+
+```cpp
+class UndoSystem {
+public:
+    void Begin(UndoType type);
+    void Append(char ch);
+    void Append(std::string_view text);
+    void commit();                     // called on cursor move, commands, etc.
+
+    void undo();                       // Ctrl+Z
+    void redo();                       // Ctrl+Y or Ctrl+Shift+Z
+
+    void mark_saved();                 // after successful save
+    void discard_pending();            // before closing buffer or loading new file
+    void clear();                      // new file / reset
+
+private:
+    void apply(const UndoNode* node, int direction);  // +1 = redo, -1 = undo
+    void free_node(UndoNode* node);
+    void free_branch(UndoNode* node);  // frees redo siblings only
+};
+```
+
+### Critical Invariants and Rules
+
+1. `begin()` must reuse `pending` if:
+    - same type
+    - same row
+    - `pending->col + pending->text.size() == current_cursor_col`
+      → otherwise `commit()` old and create new
+
+2. `pending` is detached — never linked until `commit()`
+
+3. `commit()`:
+    - discards redo branches (`current->child`)
+    - attaches `pending` as `current->child`
+    - advances `current`
+    - clears `pending`
+    - if diverged from `saved`, null it
+
+4. `apply()` must use low-level buffer operations:
+    - Never call public insert/delete/newline
+    - Use raw `buffer.insert_text(row, col, text)` and `buffer.delete_text(row, col, len)`
+    - These must not trigger undo
+
+5. `undo()`:
+    - move current to parent
+    - apply(current, -1)
+
+6. `redo()`:
+    - move current to child
+    - apply(current, +1)
+
+7. `discard_pending()` must be called in:
+    - buffer close
+    - file reload
+    - new file
+    - any destructive operation
+
+### Example Flow: Typing "hello"
+
+```text
+begin(Insert) → pending = new node, col=0
+append('h') → pending->text = "h", pending->col = 1
+append('e') → "he", col = 2
+...
+commit() on arrow key → pending becomes current->child, current advances
+```
+
+One undo step removes all of "hello".
+
+### Required Helper in Buffer Class
+
+```cpp
+class Buffer {
+    void insert_text(int row, int col, std::string_view text);   // raw, no undo
+    void delete_text(int row, int col, size_t len);              // raw, no undo
+    void split_line(int row, int col);                           // raw newline
+    void join_lines(int row);                                    // raw join
+    void insert_row(int row, std::string_view text);             // raw
+    void delete_row(int row);                                    // raw
+};
+```
+
+### Tasks for Agent
+
+1. Implement `UndoNode`, `UndoTree`, and `UndoSystem` class exactly as specified.
+2. Add `std::unique_ptr<UndoTree> undo;` to `Buffer`.
+3. Modify `insert_char`, `delete_char`, `paste`, `newline` to use `undo.begin()/append()/commit()`.
+4. Add `undo.commit()` at start of all cursor movement and command functions.
+5. Implement `apply()` using only `Buffer`'s raw methods.
+6. Add `undo.discard_pending()` in all buffer reset/close paths.
+7. Add `Ctrl+Z` → `buffer.undo()`, `Ctrl+Y` → `buffer.redo()`.
+
+This design is used in production editors and is considered the gold standard for small, correct, non-linear undo in
+C/C++. Implement it faithfully.