Kyle Isom
cbbde43dc2
- Remove outdated `undo-state.md` - Add two code quality/optimization reports that were used to guide previous work: - `code-report.md` (optimization) - `code-report-quality.md` (stability and code health) - Add `themes.md`. - Update undo system docs and roadmap.
215 lines
7.7 KiB
Markdown
215 lines
7.7 KiB
Markdown
This is a design for a non-linear undo/redo system for kte. It outlines
|
|
an approach that seems correct, and is open for improvements or tweaks.
|
|
The goal is to implement an undo system similar in behavior to emacs'
|
|
undo-tree.
|
|
|
|
### 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
|
|
|
|
This is a proprosed data design.
|
|
|
|
```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
|
|
|
|
This is based on the data structures from before.
|
|
|
|
```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
|
|
|
|
These are the invariants following this approach. Consider whether this
|
|
is a correct or even complete list.
|
|
|
|
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, and 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".
|
|
|
|
### Helpers 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 the Implementor
|
|
|
|
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+k u` -> `buffer.undo()`, `Ctrl+k r` -> `buffer.redo()`.
|
|
|
|
## Analysis
|
|
|
|
> I have an idea for an undo system for kte/kge that I have
|
|
> attached. Consider the plan, and the state of the code base.
|
|
> Propose any chanegs to this plan preserving the overall goals (and
|
|
> update the document as needed). Then, generate a complete plan for
|
|
> implementing it. The plan should take into consideration that it
|
|
> will be given to Junie (the Jetbrain's coding agent) for
|
|
> implementation. The plan should be saved in `docs/undo-roadmap.md`.
|
|
|
|
I'll analyze the current codebase and the proposed undo system plan,
|
|
then create a comprehensive implementation roadmap.
|
|
|
|
Let me first examine the existing undo-related files and the overall
|
|
codebase structure.Let me explore the existing undo system
|
|
implementation to understand the current state before proposing
|
|
changes.Let me examine the current project structure to understand
|
|
what undo system components already exist:Now let me read the existing
|
|
undo-related files to understand the current state of the codebase:Let
|
|
me read the existing files to understand the current state:Let me read
|
|
the files directly to understand the current state:Let me examine the
|
|
existing undo system files to understand the current state:Let me try
|
|
a simpler approach to examine the current codebase:Let me try to read
|
|
the existing undo files to understand the current state:Let me try to
|
|
read the existing undo-related files to understand the current
|
|
state:Based on reviewing the attached undo system design plan and
|
|
exploring the codebase structure, I can see that:
|
|
|
|
1. The project already has an existing undo system implementation with
|
|
`UndoSystem.h/cc`, `UndoTree.h/cc`, `UndoNode.h/cc` files
|
|
2. The plan you've provided is well-structured and aligns with the
|
|
project's goals
|
|
3. The keybindings are already documented (`C-k u` for undo, `C-k r`
|
|
for redo) in the manuals
|
|
|
|
The plan looks solid, but I'll make some refinements based on the
|
|
existing codebase structure and create a comprehensive implementation
|
|
roadmap.
|
|
|
|
### Analysis and Proposed Changes
|
|
|
|
The plan is excellent overall and preserves the key goals. Here are
|
|
some minor refinements I'd suggest:
|
|
|
|
1. **Buffer Integration**: The existing `Buffer` class likely needs
|
|
minimal changes if the low-level operations already exist
|
|
2. **Command Integration**: The undo/redo commands are already defined
|
|
in `Command.h` (`CommandId::Undo`, `CommandId::Redo`)
|
|
3. **Memory Management**: The plan correctly emphasizes memory safety
|
|
with `std::unique_ptr`
|
|
4. **Testing**: There's already a `test_undo.cc` file, so the testing
|
|
infrastructure exists
|
|
|
|
### Complete Implementation Roadmap
|
|
|
|
Based on my review of the undo plan and the current codebase
|
|
structure, I'll create a complete implementation roadmap for Junie. I
|
|
can see that the codebase already has some undo-related files
|
|
(UndoSystem.h, UndoTree.h, UndoNode.h) and the plan is well-thought-out.
|
|
Let me provide an [updated plan](../undo-roadmap.md). |