diff --git a/Buffer.h b/Buffer.h index 50394a9..17e8b46 100644 --- a/Buffer.h +++ b/Buffer.h @@ -482,15 +482,22 @@ public: } + // The selected rows, clamped to the buffer: they are stored as row numbers + // and go stale when lines are removed, and commands must never act on rows + // past the end (edits there were misplaced and could not be undone). [[nodiscard]] std::size_t VisualLineStartY() const { - return visual_line_anchor_y_ < visual_line_active_y_ ? visual_line_anchor_y_ : visual_line_active_y_; + const std::size_t y = visual_line_anchor_y_ < visual_line_active_y_ ? visual_line_anchor_y_ + : visual_line_active_y_; + return clamp_row_(y); } [[nodiscard]] std::size_t VisualLineEndY() const { - return visual_line_anchor_y_ < visual_line_active_y_ ? visual_line_active_y_ : visual_line_anchor_y_; + const std::size_t y = visual_line_anchor_y_ < visual_line_active_y_ ? visual_line_active_y_ + : visual_line_anchor_y_; + return clamp_row_(y); } @@ -513,6 +520,15 @@ public: [[nodiscard]] std::string AsString() const; +private: + [[nodiscard]] std::size_t clamp_row_(std::size_t y) const + { + const std::size_t n = Nrows(); + return (n == 0) ? 0 : (y < n ? y : n - 1); + } + +public: + // Syntax highlighting integration (per-buffer) [[nodiscard]] std::uint64_t Version() const { diff --git a/Command.cc b/Command.cc index db76b18..c45e0ca 100644 --- a/Command.cc +++ b/Command.cc @@ -1120,6 +1120,8 @@ cmd_save_and_quit(CommandContext &ctx) buf->SetDirty(false); if (auto *sm = ctx.editor.Swap()) sm->ResetJournal(*buf); + if (auto *u = buf->Undo()) + u->mark_saved(); } else { ctx.editor.SetStatus(err); return false; @@ -1129,6 +1131,8 @@ cmd_save_and_quit(CommandContext &ctx) buf->SetDirty(false); if (auto *sm = ctx.editor.Swap()) sm->ResetJournal(*buf); + if (auto *u = buf->Undo()) + u->mark_saved(); } else { ctx.editor.SetStatus(err); return false; @@ -3420,10 +3424,13 @@ cmd_newline(CommandContext &ctx) buf->SetDirty(true); // Cursor: end up on the final inserted line for the original cursor line. - // Each selected line above the cursor gained `repeat` lines. - std::size_t new_y = y + static_cast(repeat); - new_y += splits_above * static_cast(repeat); + // Each selected line above the cursor gained `repeat` lines, and the + // cursor's own line only if it was selected. + const bool own_split = (y >= sy && y <= ey); + std::size_t new_y = y + (own_split ? static_cast(repeat) : 0); + new_y += splits_above * static_cast(repeat); buf->SetCursor(0, new_y); + clamp_cursor_to_buffer(*buf); ensure_cursor_visible(ctx.editor, *buf); return true; } @@ -3770,6 +3777,7 @@ cmd_undo(CommandContext &ctx) for (int i = 0; i < repeat; ++i) u->undo(); // Keep cursor within buffer bounds + clamp_cursor_to_buffer(*buf); // never leave the cursor past a line or the buffer ensure_cursor_visible(ctx.editor, *buf); ctx.editor.SetStatus("Undone"); return true; @@ -3795,6 +3803,7 @@ cmd_redo(CommandContext &ctx) } else { u->redo(); } + clamp_cursor_to_buffer(*buf); // never leave the cursor past a line or the buffer ensure_cursor_visible(ctx.editor, *buf); ctx.editor.SetStatus("Redone"); return true; @@ -3971,7 +3980,10 @@ cmd_yank(CommandContext &ctx) // Bound the total: C-u 1000000 C-y of a large kill would allocate // gigabytes (and bad_alloc or the OOM killer end the session). constexpr std::size_t kMaxYankBytes = std::size_t{256} << 20; - if (text.size() * static_cast(repeat) > kMaxYankBytes) { + // Visual-line yank inserts the text once per selected line. + const std::size_t yank_lines = + buf->VisualLineActive() ? (buf->VisualLineEndY() - buf->VisualLineStartY() + 1) : 1; + if (text.size() * static_cast(repeat) * yank_lines > kMaxYankBytes) { ctx.editor.SetStatus("Yank too large (" + std::to_string(repeat) + " x " + std::to_string(text.size()) + " bytes)"); return false; @@ -5673,12 +5685,21 @@ run_handler(Editor &ed, const Command &cmd, CommandContext &ctx) { if (!cmd.handler) return false; + // An exception can leave undo groups opened by hand (BeginGroup without + // its EndGroup); close them, or every later edit would join that group. + auto close_undo_groups = [&ed] { + if (Buffer *b = ed.CurrentBuffer()) + if (UndoSystem *u = b->Undo()) + u->AbortGroups(); + }; try { return cmd.handler(ctx); } catch (const std::exception &e) { + close_undo_groups(); kte::ErrorHandler::Instance().Error("Command", std::string(cmd.name) + ": " + e.what(), ""); ed.SetStatus(std::string("Error in ") + cmd.name + ": " + e.what()); } catch (...) { + close_undo_groups(); kte::ErrorHandler::Instance().Error("Command", std::string(cmd.name) + ": unknown exception", ""); ed.SetStatus(std::string("Error in ") + cmd.name); } diff --git a/Editor.cc b/Editor.cc index 5dffa53..82e64c4 100644 --- a/Editor.cc +++ b/Editor.cc @@ -435,6 +435,10 @@ Editor::ResolveRecoveryPrompt(const bool yes) } catch (...) { // Best effort; the replay itself succeeded. } + // Nothing in the undo history matches the file on disk now: keep + // the buffer dirty even after undoing back to the loaded state. + if (auto *u = b->Undo()) + u->mark_base_unsaved(); b->SetDirty(true); apply_pending_line(*this, req.line1); SetStatus("Recovered " + req.path); diff --git a/UndoSystem.cc b/UndoSystem.cc index ba1cc95..389cbd0 100644 --- a/UndoSystem.cc +++ b/UndoSystem.cc @@ -247,7 +247,25 @@ void UndoSystem::mark_saved() { commit(); - tree_.saved = tree_.current; + tree_.saved = tree_.current; + base_unsaved_ = false; + update_dirty_flag(); +} + + +void +UndoSystem::AbortGroups() +{ + commit(); + group_depth_ = 0; + active_group_id_ = 0; +} + + +void +UndoSystem::mark_base_unsaved() +{ + base_unsaved_ = true; update_dirty_flag(); } @@ -272,6 +290,7 @@ UndoSystem::clear() tree_.root = nullptr; tree_.current = nullptr; tree_.saved = nullptr; + base_unsaved_ = false; active_group_id_ = 0; group_depth_ = 0; next_group_id_ = 1; @@ -279,9 +298,31 @@ UndoSystem::clear() } +// Position just after `node->text` inserted at (node->col, node->row). The text +// may span lines: the column is then the length after its last newline, not +// col + size (which left the cursor past the end of the line, so later edits +// and their undo records went to the wrong place). +static void +end_of_text(const UndoNode *node, std::size_t &x, std::size_t &y) +{ + y = static_cast(node->row); + x = static_cast(node->col); + const auto nl = node->text.rfind('\n'); + if (nl == std::string::npos) { + x += node->text.size(); + return; + } + for (char c: node->text) + if (c == '\n') + ++y; + x = node->text.size() - nl - 1; +} + + void UndoSystem::apply(const UndoNode *node, int direction) { + std::size_t ex = 0, ey = 0; if (!node) return; // Cursor positioning: keep the point at a sensible location after undo/redo. @@ -291,8 +332,8 @@ UndoSystem::apply(const UndoNode *node, int direction) case UndoType::Paste: if (direction > 0) { buf_->insert_text(node->row, node->col, node->text); - buf_->SetCursor(static_cast(node->col + node->text.size()), - static_cast(node->row)); + end_of_text(node, ex, ey); + buf_->SetCursor(ex, ey); } else { buf_->delete_text(node->row, node->col, node->text.size()); buf_->SetCursor(static_cast(node->col), static_cast(node->row)); @@ -304,8 +345,8 @@ UndoSystem::apply(const UndoNode *node, int direction) buf_->SetCursor(static_cast(node->col), static_cast(node->row)); } else { buf_->insert_text(node->row, node->col, node->text); - buf_->SetCursor(static_cast(node->col + node->text.size()), - static_cast(node->row)); + end_of_text(node, ex, ey); + buf_->SetCursor(ex, ey); } break; case UndoType::Newline: @@ -409,7 +450,7 @@ void UndoSystem::update_dirty_flag() { // dirty if current != saved - bool dirty = (tree_.current != tree_.saved); + bool dirty = base_unsaved_ || (tree_.current != tree_.saved); buf_->SetDirty(dirty); } diff --git a/UndoSystem.h b/UndoSystem.h index 4102d69..b4d827d 100644 --- a/UndoSystem.h +++ b/UndoSystem.h @@ -76,6 +76,14 @@ public: void mark_saved(); + // No undo state matches the file on disk any more (e.g. after crash + // recovery replayed edits onto it): the buffer stays dirty in every state + // until the next mark_saved() or clear(). + void mark_base_unsaved(); + + // Close any open group (after an interrupted command) and seal pending. + void AbortGroups(); + void discard_pending(); void clear(); @@ -115,6 +123,7 @@ private: std::uint64_t active_group_id_ = 0; int group_depth_ = 0; // nesting depth of BeginGroup/EndGroup + bool base_unsaved_ = false; // see mark_base_unsaved() std::uint64_t next_group_id_ = 1; Buffer *buf_; diff --git a/tests/test_audit_regressions.cc b/tests/test_audit_regressions.cc index 50cb211..52feaff 100644 --- a/tests/test_audit_regressions.cc +++ b/tests/test_audit_regressions.cc @@ -1026,3 +1026,87 @@ TEST(Audit_Yank_HugeRepeatRefused) ASSERT_EQ(h.Buf().Nrows(), (std::size_t) 1); ASSERT_EQ(h.Line(0), std::string()); } + + +// Undoing a multi-line kill puts the cursor after the restored text (it was +// placed at col + size on the start row, past the end of the line, so later +// edits and their undo went to the wrong place). +TEST(Audit_Undo_MultiLineRestore_CursorAfterText) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, "a\nb\n"); + b.SetCursor(0, 0); + ASSERT_TRUE(h.Exec(CommandId::ToggleMark)); + ASSERT_TRUE(h.Exec(CommandId::MoveDown)); + ASSERT_TRUE(h.Exec(CommandId::KillRegion)); + ASSERT_EQ(h.Text(), std::string("b\n")); + ASSERT_TRUE(h.Undo()); + ASSERT_EQ(h.Text(), std::string("a\nb\n")); + ASSERT_EQ(b.Cury(), (std::size_t) 1); + ASSERT_EQ(b.Curx(), (std::size_t) 0); + ASSERT_TRUE(h.Exec(CommandId::KillLine)); + ASSERT_TRUE(h.Undo()); + ASSERT_EQ(h.Text(), std::string("a\nb\n")); +} + + +// Same shape that overflowed the heap in delete-word-prev: after the undo +// the cursor is within its line. +TEST(Audit_Undo_ThenDeleteWordPrev_NoOverflow) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, std::string(24, 'a') + "\n" + std::string(30, 'b')); + b.SetCursor(0, 0); + ASSERT_TRUE(h.Exec(CommandId::ToggleMark)); + ASSERT_TRUE(h.Exec(CommandId::MoveDown)); + ASSERT_TRUE(h.Exec(CommandId::MoveEnd)); + ASSERT_TRUE(h.Exec(CommandId::KillRegion)); + ASSERT_TRUE(h.Undo()); + ASSERT_TRUE(b.Curx() <= h.Line(b.Cury()).size()); + ASSERT_TRUE(h.Exec(CommandId::DeleteWordPrev)); +} + + +// Save-and-quit records the saved state, so undo afterwards marks the buffer +// dirty again (it differs from the file). +TEST(Audit_SaveAndQuit_MarksSaved) +{ + TempDir d("saq_marksaved"); + std::ofstream(d.path / "a.txt") << "aaa\n"; + TestHarness h; + Editor &ed = h.EditorRef(); + h.Buf().insert_text(0, 0, "dirty other"); + h.Buf().SetDirty(true); + std::string err; + ASSERT_TRUE(ed.OpenFile((d.path / "a.txt").string(), err)); + ASSERT_TRUE(h.Exec(CommandId::InsertText, "X")); + ASSERT_TRUE(h.Exec(CommandId::SaveAndQuit)); + ASSERT_TRUE(!ed.QuitRequested()); // other buffer still dirty + ASSERT_EQ(slurp(d.path / "a.txt"), std::string("Xaaa\n")); + ASSERT_TRUE(!ed.CurrentBuffer()->Dirty()); + ASSERT_TRUE(h.Undo()); + ASSERT_TRUE(ed.CurrentBuffer()->Dirty()); +} + + +// Visual-line newline with a stale selection never puts the cursor past the +// buffer, so later typing is undoable. +TEST(Audit_VisualLine_StaysInsideBuffer) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, "one\ntwo\nthree\n"); + b.SetCursor(0, 0); + const std::string orig = h.Text(); + ASSERT_TRUE(h.Exec(CommandId::VisualLineModeToggle)); + ASSERT_TRUE(h.Exec(CommandId::MarkAllAndJumpEnd)); + ASSERT_TRUE(h.Exec(CommandId::Newline)); + ASSERT_TRUE(h.Exec(CommandId::VisualLineModeToggle)); + ASSERT_TRUE(b.Cury() < b.Nrows()); + ASSERT_TRUE(h.Exec(CommandId::InsertText, "Z")); + for (int i = 0; i < 10; ++i) + (void) h.Undo(); + ASSERT_EQ(h.Text(), orig); +} diff --git a/tests/test_swap_recovery_prompt.cc b/tests/test_swap_recovery_prompt.cc index 67208aa..42e4a40 100644 --- a/tests/test_swap_recovery_prompt.cc +++ b/tests/test_swap_recovery_prompt.cc @@ -679,3 +679,29 @@ TEST(SwapRecoveryPrompt_TouchedFile_StillRecoverable) answer(ed, "y"); ASSERT_EQ(buffer_bytes_via_views(*ed.CurrentBuffer()), expected); } + + +// A recovered buffer stays modified even after an edit and its undo: its +// content differs from the file on disk in every undo state. +TEST(SwapRecoveryPrompt_Recovered_StaysDirtyAfterUndo) +{ + ktet::InstallDefaultCommandsOnce(); + XdgSandbox sb("recovered_dirty"); + const std::string file = (sb.root / "work" / "rd.txt").string(); + write_file_bytes(file, "base\n"); + std::string expected; + (void) make_journal(file, [](Buffer &b) { + b.insert_text(0, 0, std::string("IMPORTANT ")); + }, expected); + + Editor ed; + ed.SetDimensions(24, 80); + ed.AddBuffer(Buffer()); + ed.RequestOpenFile(file); + (void) ed.ProcessPendingOpens(); + answer(ed, "y"); + ASSERT_TRUE(ed.CurrentBuffer()->Dirty()); + ASSERT_TRUE(Execute(ed, CommandId::InsertText, "a")); + ASSERT_TRUE(Execute(ed, CommandId::Undo)); + ASSERT_TRUE(ed.CurrentBuffer()->Dirty()); +}