Add undo system with coalescing logic and comprehensive tests.

- Implemented robust undo system supporting coalescing of text operations (insert, backspace, delete).
- Added `UndoSystem` integration into the editor/commands pipeline.
- Wrote extensive unit tests for various undo/redo scenarios, including multiline operations, cursor preservation, and history management.
- Refactored to ensure consistent cursor behavior during undo/redo actions.
- Updated CMake to include new tests.
This commit is contained in:
2026-02-10 22:39:55 -08:00
parent f3bdced3d4
commit ac0eadc345
6 changed files with 571 additions and 26 deletions

View File

@@ -302,6 +302,7 @@ if (BUILD_TESTS)
tests/test_piece_table.cc
tests/test_search.cc
tests/test_reflow_paragraph.cc
tests/test_undo.cc
tests/test_visual_line_mode.cc
# minimal engine sources required by Buffer

View File

@@ -2009,27 +2009,22 @@ cmd_insert_text(CommandContext &ctx)
return true;
}
std::size_t ins_y = y;
std::size_t ins_x = x; // remember insertion start for undo positioning
UndoSystem *u = buf->Undo();
if (u) {
// Start/extend a typed-run batch. Do NOT commit here; commit happens on boundaries
// (cursor movement, prompts, undo/redo, etc.) so consecutive InsertText commands coalesce.
buf->SetCursor(x, y);
u->Begin(UndoType::Insert);
}
// Apply edits to the underlying PieceTable through Buffer::insert_text,
// not directly to the legacy rows_ cache. This ensures Save() persists text.
for (int i = 0; i < repeat; ++i) {
buf->insert_text(static_cast<int>(y), static_cast<int>(x), std::string_view(ctx.arg));
if (u)
u->Append(std::string_view(ctx.arg));
x += ctx.arg.size();
}
buf->SetDirty(true);
// Record undo for this contiguous insert at the original insertion point
if (auto *u = buf->Undo()) {
// Position cursor at insertion start for the undo record
buf->SetCursor(ins_x, ins_y);
u->Begin(UndoType::Insert);
for (int i = 0; i < repeat; ++i) {
u->Append(std::string_view(ctx.arg));
}
// Finalize this contiguous insert as a single undoable action
u->commit();
}
buf->SetCursor(x, y);
ensure_cursor_visible(ctx.editor, *buf);
return true;
@@ -3217,6 +3212,10 @@ cmd_yank(CommandContext &ctx)
for (int i = 0; i < repeat; ++i) {
insert_text_at_cursor(*buf, text);
}
// Yank is a paste operation; it should clear the mark/region and any selection highlighting.
buf->ClearMark();
if (buf->VisualLineActive())
buf->VisualLineClear();
ensure_cursor_visible(ctx.editor, *buf);
// Start a new kill chain only from kill commands; yanking should break it
ctx.editor.SetKillChain(false);

View File

@@ -11,66 +11,216 @@ UndoSystem::UndoSystem(Buffer &owner, UndoTree &tree)
void
UndoSystem::Begin(UndoType type)
{
// STUB: Undo system incomplete - disabled until it can be properly implemented
(void) type;
if (!buf_)
return;
const int row = static_cast<int>(buf_->Cury());
const int col = static_cast<int>(buf_->Curx());
// Some operations should always be standalone undo steps.
const bool always_standalone = (type == UndoType::Newline || type == UndoType::DeleteRow);
if (always_standalone) {
commit();
}
if (tree_.pending) {
if (tree_.pending->type == type) {
// Typed-run coalescing rules.
switch (type) {
case UndoType::Insert:
case UndoType::Paste: {
// Cursor must be at the end of the pending insert.
if (tree_.pending->row == row
&& col == tree_.pending->col + static_cast<int>(tree_.pending->text.size())) {
pending_mode_ = PendingAppendMode::Append;
return;
}
break;
}
case UndoType::Delete: {
if (tree_.pending->row == row) {
// Two common delete shapes:
// 1) backspace-run: cursor moves left each time (so new col is pending.col - 1)
// 2) delete-run: cursor stays, always deleting at the same col
if (col == tree_.pending->col) {
pending_mode_ = PendingAppendMode::Append;
return;
}
if (col + 1 == tree_.pending->col) {
// Extend a backspace run to the left; update the start column now.
tree_.pending->col = col;
pending_mode_ = PendingAppendMode::Prepend;
return;
}
}
break;
}
case UndoType::Newline:
case UndoType::DeleteRow:
break;
}
}
// Can't coalesce: seal the previous pending step.
commit();
}
// Start a new pending node.
tree_.pending = new UndoNode{};
tree_.pending->type = type;
tree_.pending->row = row;
tree_.pending->col = col;
tree_.pending->text.clear();
tree_.pending->child = nullptr;
tree_.pending->next = nullptr;
pending_mode_ = PendingAppendMode::Append;
}
void
UndoSystem::Append(char ch)
{
// STUB: Undo system incomplete - disabled until it can be properly implemented
(void) ch;
if (!tree_.pending)
return;
if (pending_mode_ == PendingAppendMode::Prepend) {
tree_.pending->text.insert(tree_.pending->text.begin(), ch);
} else {
tree_.pending->text.push_back(ch);
}
}
void
UndoSystem::Append(std::string_view text)
{
// STUB: Undo system incomplete - disabled until it can be properly implemented
(void) text;
if (!tree_.pending)
return;
if (text.empty())
return;
if (pending_mode_ == PendingAppendMode::Prepend) {
tree_.pending->text.insert(0, text.data(), text.size());
} else {
tree_.pending->text.append(text.data(), text.size());
}
}
void
UndoSystem::commit()
{
// STUB: Undo system incomplete - disabled until it can be properly implemented
if (!tree_.pending)
return;
// Drop empty text batches for text-based operations.
if ((tree_.pending->type == UndoType::Insert || tree_.pending->type == UndoType::Delete
|| tree_.pending->type == UndoType::Paste)
&& tree_.pending->text.empty()) {
delete tree_.pending;
tree_.pending = nullptr;
pending_mode_ = PendingAppendMode::Append;
return;
}
// Linear semantics: if we are not at the tip, discard redo.
if (tree_.current && tree_.current->child) {
// Prevent dangling `saved` pointer if it sits in the discarded redo chain.
if (tree_.saved && is_descendant(tree_.current->child, tree_.saved)) {
tree_.saved = nullptr;
}
free_branch(tree_.current->child);
tree_.current->child = nullptr;
}
if (!tree_.root) {
tree_.root = tree_.pending;
tree_.current = tree_.pending;
} else if (!tree_.current) {
// We are at the "pre-first-edit" state. Attach as the new root child.
// For v1 linear history, this means starting the chain anew.
// The existing root represents edits from the past; attach the new node as the new root.
// (This situation happens after undoing past the first node.)
if (tree_.saved && is_descendant(tree_.root, tree_.saved)) {
// ok
}
// Discard the old root chain because it is redo from the pre-edit state.
if (tree_.saved && is_descendant(tree_.root, tree_.saved)) {
tree_.saved = nullptr;
}
free_node(tree_.root);
tree_.root = tree_.pending;
tree_.current = tree_.pending;
} else {
tree_.current->child = tree_.pending;
tree_.current = tree_.pending;
}
tree_.pending = nullptr;
pending_mode_ = PendingAppendMode::Append;
update_dirty_flag();
}
void
UndoSystem::undo()
{
// STUB: Undo system incomplete - disabled until it can be properly implemented
// Seal any in-progress typed run before undo.
commit();
if (!tree_.current)
return;
debug_log("undo");
apply(tree_.current, -1);
UndoNode *parent = find_parent(tree_.root, tree_.current);
tree_.current = parent;
update_dirty_flag();
}
void
UndoSystem::redo()
{
// STUB: Undo system incomplete - disabled until it can be properly implemented
commit();
UndoNode *next = nullptr;
if (!tree_.current) {
next = tree_.root;
} else {
next = tree_.current->child;
}
if (!next)
return;
debug_log("redo");
apply(next, +1);
tree_.current = next;
update_dirty_flag();
}
void
UndoSystem::mark_saved()
{
// STUB: Undo system incomplete - disabled until it can be properly implemented
commit();
tree_.saved = tree_.current;
update_dirty_flag();
}
void
UndoSystem::discard_pending()
{
// STUB: Undo system incomplete - disabled until it can be properly implemented
if (tree_.pending) {
delete tree_.pending;
tree_.pending = nullptr;
}
pending_mode_ = PendingAppendMode::Append;
}
void
UndoSystem::clear()
{
// STUB: Undo system incomplete - disabled until it can be properly implemented
discard_pending();
free_node(tree_.root);
tree_.root = nullptr;
tree_.current = nullptr;
tree_.saved = nullptr;
update_dirty_flag();
}
@@ -79,34 +229,46 @@ UndoSystem::apply(const UndoNode *node, int direction)
{
if (!node)
return;
// Cursor positioning: keep the point at a sensible location after undo/redo.
// Low-level Buffer edit primitives do not move the cursor.
switch (node->type) {
case UndoType::Insert:
case UndoType::Paste:
if (direction > 0) {
buf_->insert_text(node->row, node->col, node->text);
buf_->SetCursor(static_cast<std::size_t>(node->col + node->text.size()),
static_cast<std::size_t>(node->row));
} else {
buf_->delete_text(node->row, node->col, node->text.size());
buf_->SetCursor(static_cast<std::size_t>(node->col), static_cast<std::size_t>(node->row));
}
break;
case UndoType::Delete:
if (direction > 0) {
buf_->delete_text(node->row, node->col, node->text.size());
buf_->SetCursor(static_cast<std::size_t>(node->col), static_cast<std::size_t>(node->row));
} else {
buf_->insert_text(node->row, node->col, node->text);
buf_->SetCursor(static_cast<std::size_t>(node->col + node->text.size()),
static_cast<std::size_t>(node->row));
}
break;
case UndoType::Newline:
if (direction > 0) {
buf_->split_line(node->row, node->col);
buf_->SetCursor(0, static_cast<std::size_t>(node->row + 1));
} else {
buf_->join_lines(node->row);
buf_->SetCursor(static_cast<std::size_t>(node->col), static_cast<std::size_t>(node->row));
}
break;
case UndoType::DeleteRow:
if (direction > 0) {
buf_->delete_row(node->row);
buf_->SetCursor(0, static_cast<std::size_t>(node->row));
} else {
buf_->insert_row(node->row, node->text);
buf_->SetCursor(0, static_cast<std::size_t>(node->row));
}
break;
}

View File

@@ -33,6 +33,11 @@ public:
void UpdateBufferReference(Buffer &new_buf);
private:
enum class PendingAppendMode : std::uint8_t {
Append,
Prepend,
};
void apply(const UndoNode *node, int direction); // +1 redo, -1 undo
void free_node(UndoNode *node);
@@ -48,6 +53,8 @@ private:
void update_dirty_flag();
PendingAppendMode pending_mode_ = PendingAppendMode::Append;
Buffer *buf_;
UndoTree &tree_;
};

343
tests/test_undo.cc Normal file
View File

@@ -0,0 +1,343 @@
#include "Test.h"
#include "Buffer.h"
TEST (Undo_InsertRun_Coalesces)
{
Buffer b;
UndoSystem *u = b.Undo();
ASSERT_TRUE(u != nullptr);
// Simulate two separate "typed" insert commands without committing in between.
b.SetCursor(0, 0);
u->Begin(UndoType::Insert);
b.insert_text(0, 0, std::string_view("h"));
u->Append('h');
b.SetCursor(1, 0);
u->Begin(UndoType::Insert);
b.insert_text(0, 1, std::string_view("i"));
u->Append('i');
b.SetCursor(2, 0);
u->commit();
ASSERT_EQ(b.Rows().size(), (std::size_t) 1);
ASSERT_EQ(std::string(b.Rows()[0]), std::string("hi"));
u->undo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string(""));
}
TEST (Undo_BackspaceRun_Coalesces)
{
Buffer b;
UndoSystem *u = b.Undo();
ASSERT_TRUE(u != nullptr);
// Seed content.
b.insert_text(0, 0, std::string_view("abc"));
b.SetCursor(3, 0);
u->mark_saved();
// Simulate two backspaces: delete 'c' then 'b'.
{
const auto &rows = b.Rows();
char deleted = rows[0][2];
b.delete_text(0, 2, 1);
b.SetCursor(2, 0);
u->Begin(UndoType::Delete);
u->Append(deleted);
}
{
const auto &rows = b.Rows();
char deleted = rows[0][1];
b.delete_text(0, 1, 1);
b.SetCursor(1, 0);
u->Begin(UndoType::Delete);
u->Append(deleted);
}
u->commit();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("a"));
// One undo should restore both characters.
u->undo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("abc"));
}
TEST (Undo_Linear_RedoDiscardedAfterNewEdit)
{
Buffer b;
UndoSystem *u = b.Undo();
ASSERT_TRUE(u != nullptr);
b.SetCursor(0, 0);
u->Begin(UndoType::Insert);
b.insert_text(0, 0, std::string_view("a"));
u->Append('a');
b.SetCursor(1, 0);
u->commit();
u->Begin(UndoType::Insert);
b.insert_text(0, 1, std::string_view("b"));
u->Append('b');
b.SetCursor(2, 0);
u->commit();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("ab"));
u->undo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("a"));
// New edit after undo should discard redo.
u->Begin(UndoType::Insert);
b.insert_text(0, 1, std::string_view("c"));
u->Append('c');
b.SetCursor(2, 0);
u->commit();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("ac"));
u->redo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("ac"));
}
TEST (Undo_DirtyFlag_MarkSavedAndUndoRedo)
{
Buffer b;
UndoSystem *u = b.Undo();
ASSERT_TRUE(u != nullptr);
u->mark_saved();
ASSERT_TRUE(!b.Dirty());
b.SetCursor(0, 0);
u->Begin(UndoType::Insert);
b.insert_text(0, 0, std::string_view("x"));
u->Append('x');
b.SetCursor(1, 0);
u->commit();
ASSERT_TRUE(b.Dirty());
u->undo();
ASSERT_TRUE(!b.Dirty());
u->redo();
ASSERT_TRUE(b.Dirty());
}
TEST (Undo_Newline_UndoRedo_SplitJoin)
{
Buffer b;
UndoSystem *u = b.Undo();
ASSERT_TRUE(u != nullptr);
// Seed a single line and split it.
b.insert_text(0, 0, std::string_view("hello"));
b.SetCursor(2, 0); // split after "he"
u->Begin(UndoType::Newline);
b.split_line(0, 2);
u->commit();
ASSERT_EQ(b.Rows().size(), (std::size_t) 2);
ASSERT_EQ(std::string(b.Rows()[0]), std::string("he"));
ASSERT_EQ(std::string(b.Rows()[1]), std::string("llo"));
// Undo should join the lines back.
u->undo();
ASSERT_EQ(b.Rows().size(), (std::size_t) 1);
ASSERT_EQ(std::string(b.Rows()[0]), std::string("hello"));
// Redo should split again at the same point.
u->redo();
ASSERT_EQ(b.Rows().size(), (std::size_t) 2);
ASSERT_EQ(std::string(b.Rows()[0]), std::string("he"));
ASSERT_EQ(std::string(b.Rows()[1]), std::string("llo"));
}
TEST (Undo_DeleteKeyRun_Coalesces)
{
Buffer b;
UndoSystem *u = b.Undo();
ASSERT_TRUE(u != nullptr);
// Seed content: delete-key semantics keep cursor at the same column.
b.insert_text(0, 0, std::string_view("abcd"));
b.SetCursor(1, 0); // on 'b'
// Delete 'b'
{
const auto &rows = b.Rows();
char deleted = rows[0][1];
u->Begin(UndoType::Delete);
b.delete_text(0, 1, 1);
u->Append(deleted);
b.SetCursor(1, 0);
}
// Delete next char (was 'c', now at same col=1)
{
const auto &rows = b.Rows();
char deleted = rows[0][1];
u->Begin(UndoType::Delete);
b.delete_text(0, 1, 1);
u->Append(deleted);
b.SetCursor(1, 0);
}
u->commit();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("ad"));
// One undo should restore both deleted characters.
u->undo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("abcd"));
}
TEST (Undo_UndoPastFirstEdit_RedoFromPreFirstEdit)
{
Buffer b;
UndoSystem *u = b.Undo();
ASSERT_TRUE(u != nullptr);
// Commit two separate insert edits.
b.SetCursor(0, 0);
u->Begin(UndoType::Insert);
b.insert_text(0, 0, std::string_view("a"));
u->Append('a');
b.SetCursor(1, 0);
u->commit();
b.SetCursor(1, 0);
u->Begin(UndoType::Insert);
b.insert_text(0, 1, std::string_view("b"));
u->Append('b');
b.SetCursor(2, 0);
u->commit();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("ab"));
// Undo twice: we should reach the pre-first-edit state.
u->undo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("a"));
u->undo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string(""));
// Redo twice should restore both edits.
u->redo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("a"));
u->redo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("ab"));
}
TEST (Undo_NewEditFromPreFirstEdit_DiscardsOldHistory)
{
Buffer b;
UndoSystem *u = b.Undo();
ASSERT_TRUE(u != nullptr);
// Build up two edits.
b.SetCursor(0, 0);
u->Begin(UndoType::Insert);
b.insert_text(0, 0, std::string_view("a"));
u->Append('a');
b.SetCursor(1, 0);
u->commit();
b.SetCursor(1, 0);
u->Begin(UndoType::Insert);
b.insert_text(0, 1, std::string_view("b"));
u->Append('b');
b.SetCursor(2, 0);
u->commit();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("ab"));
// Undo past first edit so current becomes null.
u->undo();
u->undo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string(""));
// Commit a new edit from the pre-first-edit state.
b.SetCursor(0, 0);
u->Begin(UndoType::Insert);
b.insert_text(0, 0, std::string_view("x"));
u->Append('x');
b.SetCursor(1, 0);
u->commit();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("x"));
// Old history should be gone: redo should not resurrect "ab".
u->redo();
ASSERT_EQ(std::string(b.Rows()[0]), std::string("x"));
}
TEST (Undo_MultiLineDelete_ConsumesNewline_UndoRestores)
{
Buffer b;
UndoSystem *u = b.Undo();
ASSERT_TRUE(u != nullptr);
// Create two lines. PieceTable treats '\n' between logical lines.
b.insert_text(0, 0, std::string_view("ab\ncd"));
ASSERT_EQ(b.Rows().size(), (std::size_t) 2);
ASSERT_EQ(std::string(b.Rows()[0]), std::string("ab"));
ASSERT_EQ(std::string(b.Rows()[1]), std::string("cd"));
// Delete spanning the newline: delete "b\n" starting at (0,1).
b.SetCursor(1, 0);
u->Begin(UndoType::Delete);
b.delete_text(0, 1, 2);
u->Append(std::string_view("b\n"));
u->commit();
ASSERT_EQ(b.Rows().size(), (std::size_t) 1);
ASSERT_EQ(std::string(b.Rows()[0]), std::string("acd"));
// Undo should restore exact original text/line structure.
u->undo();
ASSERT_EQ(b.Rows().size(), (std::size_t) 2);
ASSERT_EQ(std::string(b.Rows()[0]), std::string("ab"));
ASSERT_EQ(std::string(b.Rows()[1]), std::string("cd"));
}
TEST (Undo_DeleteIndent_UndoRestoresCursorAtText)
{
Buffer b;
UndoSystem *u = b.Undo();
ASSERT_TRUE(u != nullptr);
// Seed 3-line content with indentation on the middle line.
b.insert_text(0, 0,
std::string_view("I did a thing\n and then I edited a thing\nbut there were gaps"));
ASSERT_EQ(b.Rows().size(), (std::size_t) 3);
// Cursor at start of the line (before spaces), then C-d C-d deletes two spaces.
b.SetCursor(0, 1);
for (int i = 0; i < 2; ++i) {
const auto &rows = b.Rows();
char deleted = rows[1][0];
ASSERT_EQ(deleted, ' ');
u->Begin(UndoType::Delete);
b.delete_text(1, 0, 1);
u->Append(deleted);
b.SetCursor(0, 1); // delete-key keeps col the same
}
u->commit();
ASSERT_EQ(std::string(b.Rows()[1]), std::string("and then I edited a thing"));
ASSERT_EQ(b.Cury(), (std::size_t) 1);
ASSERT_EQ(b.Curx(), (std::size_t) 0);
// Undo should restore indentation, and keep cursor on the text (at 'a'), not at EOL.
u->undo();
ASSERT_EQ(std::string(b.Rows()[1]), std::string(" and then I edited a thing"));
ASSERT_EQ(b.Cury(), (std::size_t) 1);
ASSERT_EQ(b.Curx(), (std::size_t) 2);
}

View File

@@ -123,3 +123,36 @@ TEST (VisualLineMode_CancelWithCtrlG)
}
ASSERT_TRUE(got == exp);
}
TEST (Yank_ClearsMarkAndVisualLine)
{
InstallDefaultCommands();
Editor ed;
ed.SetDimensions(24, 80);
Buffer b;
b.insert_text(0, 0, "foo\nbar\n");
b.SetCursor(1, 0);
ed.AddBuffer(std::move(b));
ASSERT_TRUE(ed.CurrentBuffer() != nullptr);
Buffer *buf = ed.CurrentBuffer();
// Seed mark + visual-line highlighting.
buf->SetMark(buf->Curx(), buf->Cury());
ASSERT_TRUE(buf->MarkSet());
ASSERT_TRUE(Execute(ed, std::string("visual-line-toggle")));
ASSERT_TRUE(Execute(ed, std::string("down"), std::string(), 1));
ASSERT_TRUE(buf->VisualLineActive());
// Yank should clear mark and any highlighting.
ed.KillRingClear();
ed.KillRingPush("X");
ASSERT_TRUE(Execute(ed, std::string("yank")));
ASSERT_TRUE(!buf->MarkSet());
ASSERT_TRUE(!buf->VisualLineActive());
}