From 27dcb41857ec709b56bd41f6d24933833756c72f Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Thu, 26 Feb 2026 13:21:07 -0800 Subject: [PATCH] Add ReflowUndo tests and integrate InsertRow undo support - Added `test_reflow_undo.cc` to validate undo/redo workflows for reflow operations. - Introduced `UndoType::InsertRow` in `UndoSystem` for tracking row insertion changes in undo history. - Updated `UndoNode.h` and `UndoSystem.cc` to support row insertion as a standalone undo step. - Enhanced reflow paragraph functionality to properly record undo/redo actions for both row deletion and insertion. - Enabled legacy/extended undo tests in `test_undo.cc` for comprehensive validation. - Updated `CMakeLists.txt` to include new test file in the build target. --- CMakeLists.txt | 1 + Command.cc | 13 +++ UndoNode.h | 3 +- UndoSystem.cc | 17 +++- tests/test_reflow_undo.cc | 69 ++++++++++++++ tests/test_undo.cc | 188 +++++++++++++++++++++++++++++++++++--- 6 files changed, 276 insertions(+), 15 deletions(-) create mode 100644 tests/test_reflow_undo.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index a12cc26..0dc1b36 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -337,6 +337,7 @@ if (BUILD_TESTS) tests/test_benchmarks.cc tests/test_migration_coverage.cc tests/test_smart_newline.cc + tests/test_reflow_undo.cc # minimal engine sources required by Buffer PieceTable.cc diff --git a/Command.cc b/Command.cc index 782101e..e816d57 100644 --- a/Command.cc +++ b/Command.cc @@ -4675,7 +4675,14 @@ cmd_reflow_paragraph(CommandContext &ctx) new_lines.push_back(""); // Replace paragraph lines via PieceTable-backed operations + UndoSystem *u = buf->Undo(); for (std::size_t i = para_end; i + 1 > para_start; --i) { + if (u) { + buf->SetCursor(0, i); + u->Begin(UndoType::DeleteRow); + u->Append(static_cast(buf->Rows()[i])); + u->commit(); + } buf->delete_row(static_cast(i)); if (i == 0) break; // prevent wrap on size_t @@ -4684,6 +4691,12 @@ cmd_reflow_paragraph(CommandContext &ctx) std::size_t insert_y = para_start; for (const auto &ln: new_lines) { buf->insert_row(static_cast(insert_y), std::string_view(ln)); + if (u) { + buf->SetCursor(0, insert_y); + u->Begin(UndoType::InsertRow); + u->Append(std::string_view(ln)); + u->commit(); + } insert_y += 1; } diff --git a/UndoNode.h b/UndoNode.h index 1ab0c86..a0b6504 100644 --- a/UndoNode.h +++ b/UndoNode.h @@ -9,6 +9,7 @@ enum class UndoType : std::uint8_t { Paste, Newline, DeleteRow, + InsertRow, }; struct UndoNode { @@ -20,4 +21,4 @@ struct UndoNode { UndoNode *parent = nullptr; // previous state; null means pre-first-edit UndoNode *child = nullptr; // next in current timeline UndoNode *next = nullptr; // redo branch -}; +}; \ No newline at end of file diff --git a/UndoSystem.cc b/UndoSystem.cc index 4c03fe4..45e3ea4 100644 --- a/UndoSystem.cc +++ b/UndoSystem.cc @@ -36,7 +36,8 @@ UndoSystem::Begin(UndoType type) const int col = static_cast(buf_->Curx()); // Some operations should always be standalone undo steps. - const bool always_standalone = (type == UndoType::Newline || type == UndoType::DeleteRow); + const bool always_standalone = (type == UndoType::Newline || type == UndoType::DeleteRow || type == + UndoType::InsertRow); if (always_standalone) { commit(); } @@ -75,6 +76,7 @@ UndoSystem::Begin(UndoType type) } case UndoType::Newline: case UndoType::DeleteRow: + case UndoType::InsertRow: break; } } @@ -314,6 +316,15 @@ UndoSystem::apply(const UndoNode *node, int direction) buf_->SetCursor(0, static_cast(node->row)); } break; + case UndoType::InsertRow: + if (direction > 0) { + buf_->insert_row(node->row, node->text); + buf_->SetCursor(0, static_cast(node->row)); + } else { + buf_->delete_row(node->row); + buf_->SetCursor(0, static_cast(node->row)); + } + break; } } @@ -411,6 +422,8 @@ UndoSystem::type_str(UndoType t) return "Newline"; case UndoType::DeleteRow: return "DeleteRow"; + case UndoType::InsertRow: + return "InsertRow"; } return "?"; } @@ -452,4 +465,4 @@ UndoSystem::debug_log(const char *op) const #else (void) op; #endif -} +} \ No newline at end of file diff --git a/tests/test_reflow_undo.cc b/tests/test_reflow_undo.cc new file mode 100644 index 0000000..8bcfff4 --- /dev/null +++ b/tests/test_reflow_undo.cc @@ -0,0 +1,69 @@ +#include "Test.h" + +#include "Buffer.h" +#include "Command.h" +#include "Editor.h" +#include "UndoSystem.h" + +#include + + +static std::string +to_string_rows(const Buffer &buf) +{ + std::string out; + for (const auto &r: buf.Rows()) { + out += static_cast(r); + out.push_back('\n'); + } + return out; +} + + +TEST (ReflowUndo) +{ + InstallDefaultCommands(); + + Editor ed; + ed.SetDimensions(24, 80); + + Buffer b; + const std::string initial = + "This is a very long line that should be reflowed into multiple lines to see if undo works correctly.\n"; + b.insert_text(0, 0, initial); + b.SetCursor(0, 0); + + // Commit initial insertion so it's its own undo step + if (auto *u = b.Undo()) + u->commit(); + + ed.AddBuffer(std::move(b)); + + Buffer *buf = ed.CurrentBuffer(); + ASSERT_TRUE(buf != nullptr); + + const std::string original_dump = to_string_rows(*buf); + + // Reflow with small width + const int width = 20; + ASSERT_TRUE(Execute(ed, "reflow-paragraph", "", width)); + + const std::string reflowed_dump = to_string_rows(*buf); + ASSERT_TRUE(reflowed_dump != original_dump); + ASSERT_TRUE(buf->Rows().size() > 1); + + // Undo reflow + ASSERT_TRUE(Execute(ed, "undo", "", 1)); + const std::string after_undo_dump = to_string_rows(*buf); + + if (after_undo_dump != original_dump) { + fprintf(stderr, "Undo failed.\nExpected:\n%s\nGot:\n%s\n", original_dump.c_str(), + after_undo_dump.c_str()); + } + EXPECT_TRUE(after_undo_dump == original_dump); + + // Redo reflow + ASSERT_TRUE(Execute(ed, "redo", "", 1)); + const std::string after_redo_dump = to_string_rows(*buf); + EXPECT_TRUE(after_redo_dump == reflowed_dump); +} \ No newline at end of file diff --git a/tests/test_undo.cc b/tests/test_undo.cc index 1716332..21606e4 100644 --- a/tests/test_undo.cc +++ b/tests/test_undo.cc @@ -57,7 +57,7 @@ validate_undo_tree(const UndoSystem &u) // The undo suite aims to cover invariants with a small, adversarial test matrix. -TEST(Undo_InsertRun_Coalesces_OneStep) +TEST (Undo_InsertRun_Coalesces_OneStep) { Buffer b; UndoSystem *u = b.Undo(); @@ -81,7 +81,7 @@ TEST(Undo_InsertRun_Coalesces_OneStep) } -TEST(Undo_InsertRun_BreaksOnNonAdjacentCursor) +TEST (Undo_InsertRun_BreaksOnNonAdjacentCursor) { Buffer b; UndoSystem *u = b.Undo(); @@ -109,7 +109,7 @@ TEST(Undo_InsertRun_BreaksOnNonAdjacentCursor) } -TEST(Undo_BackspaceRun_Coalesces_OneStep) +TEST (Undo_BackspaceRun_Coalesces_OneStep) { Buffer b; UndoSystem *u = b.Undo(); @@ -143,7 +143,7 @@ TEST(Undo_BackspaceRun_Coalesces_OneStep) } -TEST(Undo_DeleteKeyRun_Coalesces_OneStep) +TEST (Undo_DeleteKeyRun_Coalesces_OneStep) { Buffer b; UndoSystem *u = b.Undo(); @@ -176,7 +176,7 @@ TEST(Undo_DeleteKeyRun_Coalesces_OneStep) } -TEST(Undo_Newline_IsStandalone) +TEST (Undo_Newline_IsStandalone) { Buffer b; UndoSystem *u = b.Undo(); @@ -211,7 +211,7 @@ TEST(Undo_Newline_IsStandalone) } -TEST(Undo_ExplicitGroup_UndoesAsUnit) +TEST (Undo_ExplicitGroup_UndoesAsUnit) { Buffer b; UndoSystem *u = b.Undo(); @@ -239,7 +239,7 @@ TEST(Undo_ExplicitGroup_UndoesAsUnit) } -TEST(Undo_Branching_RedoBranchSelectionDeterministic) +TEST (Undo_Branching_RedoBranchSelectionDeterministic) { Buffer b; UndoSystem *u = b.Undo(); @@ -283,7 +283,7 @@ TEST(Undo_Branching_RedoBranchSelectionDeterministic) } -TEST(Undo_DirtyFlag_CrossesMarkSaved) +TEST (Undo_DirtyFlag_CrossesMarkSaved) { Buffer b; UndoSystem *u = b.Undo(); @@ -312,7 +312,7 @@ TEST(Undo_DirtyFlag_CrossesMarkSaved) } -TEST(Undo_RoundTrip_Lossless_RandomEdits) +TEST (Undo_RoundTrip_Lossless_RandomEdits) { Buffer b; UndoSystem *u = b.Undo(); @@ -368,7 +368,7 @@ TEST(Undo_RoundTrip_Lossless_RandomEdits) // Legacy/extended undo tests follow. Keep them available for debugging, // but disable them by default to keep the suite focused (~10 tests). -#if 0 +#if 1 TEST (Undo_Branching_RedoPreservedAfterNewEdit) @@ -713,6 +713,7 @@ TEST (Undo_StructuralInvariants_BranchingAndRoots) validate_undo_tree(*u); } + TEST (Undo_BranchSelection_ThreeSiblingsAndHeadPersists) { Buffer b; @@ -796,7 +797,7 @@ TEST (Undo_BranchSelection_ThreeSiblingsAndHeadPersists) // Additional legacy tests below are useful, but kept disabled by default. -#if 0 +#if 1 TEST (Undo_Branching_SwitchBetweenTwoRedoBranches_TextAndCursor) { @@ -1196,4 +1197,167 @@ TEST (Undo_Command_RedoCountSelectsBranch) validate_undo_tree(*u); } -#endif // legacy tests + +TEST (Undo_InsertRow_UndoDeletesRow) +{ + Buffer b; + UndoSystem *u = b.Undo(); + ASSERT_TRUE(u != nullptr); + + // Seed two lines so insert_row has proper newline context. + b.insert_text(0, 0, std::string_view("first\nlast")); + ASSERT_EQ(b.Rows().size(), (std::size_t) 2); + + // Insert a row at position 1 (between first and last), then record it. + b.insert_row(1, std::string_view("second")); + ASSERT_EQ(b.Rows().size(), (std::size_t) 3); + ASSERT_EQ(std::string(b.Rows()[1]), std::string("second")); + + b.SetCursor(0, 1); + u->Begin(UndoType::InsertRow); + u->Append(std::string_view("second")); + u->commit(); + + // Undo should remove the inserted row. + u->undo(); + ASSERT_EQ(b.Rows().size(), (std::size_t) 2); + ASSERT_EQ(std::string(b.Rows()[0]), std::string("first")); + ASSERT_EQ(std::string(b.Rows()[1]), std::string("last")); + + // Redo should re-insert it. + u->redo(); + ASSERT_EQ(b.Rows().size(), (std::size_t) 3); + ASSERT_EQ(std::string(b.Rows()[1]), std::string("second")); + + validate_undo_tree(*u); +} + + +TEST (Undo_DeleteRow_UndoRestoresRow) +{ + Buffer b; + UndoSystem *u = b.Undo(); + ASSERT_TRUE(u != nullptr); + + b.insert_text(0, 0, std::string_view("alpha\nbeta\ngamma")); + ASSERT_EQ(b.Rows().size(), (std::size_t) 3); + + // Record a DeleteRow for row 1 ("beta"). + b.SetCursor(0, 1); + u->Begin(UndoType::DeleteRow); + u->Append(static_cast(b.Rows()[1])); + u->commit(); + b.delete_row(1); + + ASSERT_EQ(b.Rows().size(), (std::size_t) 2); + ASSERT_EQ(std::string(b.Rows()[0]), std::string("alpha")); + ASSERT_EQ(std::string(b.Rows()[1]), std::string("gamma")); + + // Undo should restore "beta" at row 1. + u->undo(); + ASSERT_EQ(b.Rows().size(), (std::size_t) 3); + ASSERT_EQ(std::string(b.Rows()[1]), std::string("beta")); + + // Redo should delete it again. + u->redo(); + ASSERT_EQ(b.Rows().size(), (std::size_t) 2); + ASSERT_EQ(std::string(b.Rows()[1]), std::string("gamma")); + + validate_undo_tree(*u); +} + + +TEST (Undo_InsertRow_IsStandalone) +{ + Buffer b; + UndoSystem *u = b.Undo(); + ASSERT_TRUE(u != nullptr); + + // Seed with two lines so InsertRow has proper newline context. + b.insert_text(0, 0, std::string_view("x\nend")); + ASSERT_EQ(b.Rows().size(), (std::size_t) 2); + + // Start a pending insert on row 0. + b.SetCursor(1, 0); + u->Begin(UndoType::Insert); + b.insert_text(0, 1, std::string_view("y")); + u->Append('y'); + b.SetCursor(2, 0); + + // InsertRow should seal the pending "y" and become its own step. + b.insert_row(1, std::string_view("row2")); + b.SetCursor(0, 1); + u->Begin(UndoType::InsertRow); + u->Append(std::string_view("row2")); + u->commit(); + + ASSERT_EQ(std::string(b.Rows()[0]), std::string("xy")); + ASSERT_EQ(std::string(b.Rows()[1]), std::string("row2")); + ASSERT_EQ(b.Rows().size(), (std::size_t) 3); + + // Undo InsertRow only. + u->undo(); + ASSERT_EQ(b.Rows().size(), (std::size_t) 2); + ASSERT_EQ(std::string(b.Rows()[0]), std::string("xy")); + + // Undo the insert "y". + u->undo(); + ASSERT_EQ(std::string(b.Rows()[0]), std::string("x")); + + validate_undo_tree(*u); +} + + +TEST (Undo_GroupedDeleteAndInsertRows_UndoesAsUnit) +{ + Buffer b; + UndoSystem *u = b.Undo(); + ASSERT_TRUE(u != nullptr); + + // Seed three lines (with trailing newline so delete_row/insert_row work cleanly). + b.insert_text(0, 0, std::string_view("aaa\nbbb\nccc\n")); + ASSERT_EQ(b.Rows().size(), (std::size_t) 4); // 3 content + 1 empty trailing + const std::string original = b.AsString(); + + // Group: delete content rows then insert replacements (simulates reflow). + (void) u->BeginGroup(); + + // Delete rows 2,1,0 in reverse order (like reflow does). + for (int i = 2; i >= 0; --i) { + b.SetCursor(0, static_cast(i)); + u->Begin(UndoType::DeleteRow); + u->Append(static_cast(b.Rows()[static_cast(i)])); + u->commit(); + b.delete_row(i); + } + + // Insert replacement rows. + b.insert_row(0, std::string_view("aaa bbb")); + b.SetCursor(0, 0); + u->Begin(UndoType::InsertRow); + u->Append(std::string_view("aaa bbb")); + u->commit(); + + b.insert_row(1, std::string_view("ccc")); + b.SetCursor(0, 1); + u->Begin(UndoType::InsertRow); + u->Append(std::string_view("ccc")); + u->commit(); + + u->EndGroup(); + + const std::string reflowed = b.AsString(); + + // Single undo should restore original content. + u->undo(); + ASSERT_EQ(b.AsString(), original); + + // Redo should restore the reflowed state. + u->redo(); + ASSERT_EQ(b.AsString(), reflowed); + + validate_undo_tree(*u); +} + + +#endif // legacy tests \ No newline at end of file