From 812cde090facb0c0ecb1727d8130e1fc83976757 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 04:18:52 +0000 Subject: [PATCH] Commands: second-pass fixes from the follow-up review - Motion and delete commands read rows through RowsView (on-demand, version-cached) instead of Buffer::Rows(), which rebuilt every line of the file after each edit. On a 500k-line buffer after an edit: backspace/delete/motions 80-90 ms -> ~0.1 ms; C-u 3000 C-d 273 s -> 0.25 s. Whole-buffer search scans keep Rows() (cached between edits). - While a prompt is open only prompt-aware and view-only commands run. Others edited the buffer behind the prompt (bypassing the read-only check) or switched/closed buffers under a pending confirmation, which then acted on the wrong buffer. The ':' command path now applies the read-only check too. - Pasting into a prompt (ImGui) flattens line breaks to spaces instead of emitting Newline, which accepted the prompt and sent the rest of the paste to the buffer (or ran replace-all). - The mark region clamps both ends; a stale mark past the end of the buffer is treated as end of buffer. This fixes kill-region deleting the wrong text and leaving the cursor past end-of-line. - Kill-line with a count stops at end of buffer instead of killing lines above the cursor (regression from the D3 change). - Undo groups nest, so SmartNewline in visual-line mode is one step. - Word motions and word deletion treat non-ASCII bytes as word characters and so never split a UTF-8 character. - The universal-argument count saturates at 1,000,000 instead of overflowing int. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB --- Command.cc | 228 +++++++++++++++++++++++++------- Editor.cc | 10 +- ImGuiInputHandler.cc | 5 +- PasteSplit.h | 24 ++++ UndoSystem.cc | 9 +- UndoSystem.h | 1 + tests/test_audit_regressions.cc | 117 ++++++++++++++++ tests/test_paste_split.cc | 12 ++ 8 files changed, 355 insertions(+), 51 deletions(-) diff --git a/Command.cc b/Command.cc index 77db59b..7d69937 100644 --- a/Command.cc +++ b/Command.cc @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -111,6 +112,13 @@ line_view(const Buffer::Line &l) } +static inline std::string_view +line_view(const std::string &l) +{ + return std::string_view(l); +} + + // Keep buffer viewport offsets so that the cursor stays within the visible // window based on the editor's current dimensions. The bottom row is reserved // for the status line. @@ -155,26 +163,39 @@ compute_render_x(std::string_view line, const std::size_t curx, const std::size_ } -// Clamp the cursor to an existing row and to that row's length. Commands that -// set the cursor from remembered or scrolled positions must call this: edits -// at a column past end-of-line are misapplied (and mis-recorded for undo). +// Clamp (x, y) to an existing row and to that row's length. static void -clamp_cursor_to_buffer(Buffer &buf) +clamp_position(const Buffer &buf, std::size_t &x, std::size_t &y) { const std::size_t nrows = buf.Nrows(); - std::size_t y = buf.Cury(); if (nrows == 0) { - buf.SetCursor(0, 0); + x = y = 0; return; } - if (y >= nrows) + // A row past the end means a position past the end of the buffer: clamp + // it to the end of the last line, not to its column on that line. + const bool past_end = y >= nrows; + if (past_end) y = nrows - 1; const auto [start, end] = buf.GetLineRange(y); std::size_t len = end - start; // GetLineRange includes the trailing newline, if any. if (len > 0 && y + 1 < nrows) --len; - buf.SetCursor(std::min(buf.Curx(), len), y); + x = past_end ? len : std::min(x, len); +} + + +// Clamp the cursor to an existing row and to that row's length. Commands that +// set the cursor from remembered or scrolled positions must call this: edits +// at a column past end-of-line are misapplied (and mis-recorded for undo). +static void +clamp_cursor_to_buffer(Buffer &buf) +{ + std::size_t x = buf.Curx(); + std::size_t y = buf.Cury(); + clamp_position(buf, x, y); + buf.SetCursor(x, y); } @@ -320,6 +341,42 @@ is_mutating_command(CommandId id) } +// Commands that may run while a prompt is open: prompt editing and +// navigation (their handlers check PromptActive), cancel, and view-only +// commands that do not touch buffer contents or buffer selection. +static bool +allowed_during_prompt(CommandId id) +{ + switch (id) { + case CommandId::Refresh: + case CommandId::InsertText: + case CommandId::Newline: + case CommandId::SmartNewline: + case CommandId::Backspace: + case CommandId::MoveLeft: + case CommandId::MoveRight: + case CommandId::MoveUp: + case CommandId::MoveDown: + case CommandId::UArgStatus: + case CommandId::KPrefix: + case CommandId::UnknownKCommand: + case CommandId::UnknownEscCommand: + case CommandId::ScrollUp: + case CommandId::ScrollDown: + case CommandId::ThemeNext: + case CommandId::ThemePrev: + case CommandId::FontZoomIn: + case CommandId::FontZoomOut: + case CommandId::FontZoomReset: + case CommandId::VisualFilePickerToggle: + case CommandId::VisualFontPickerToggle: + return true; + default: + return false; + } +} + + // --- UI/status helpers --- static bool cmd_uarg_status(CommandContext &ctx) @@ -340,6 +397,10 @@ compute_mark_region(Buffer &buf, std::size_t &sx, std::size_t &sy, std::size_t & std::size_t cy = buf.Cury(); std::size_t mx = buf.MarkCurx(); std::size_t my = buf.MarkCury(); + // Marks are not adjusted by edits, so either end may now lie past the end + // of its line or of the buffer; clamp both before ordering them. + clamp_position(buf, cx, cy); + clamp_position(buf, mx, my); if (cy < my || (cy == my && cx < mx)) { sy = cy; sx = cx; @@ -357,11 +418,63 @@ compute_mark_region(Buffer &buf, std::size_t &sx, std::size_t &sy, std::size_t & } +// Row access for commands that touch a few rows. Buffer::Rows() rebuilds a +// string for every line of the file on the first call after any edit, which +// made every motion and delete O(file size). RowsView fetches rows on demand +// and caches them for the current buffer version. Strings fetched for an +// older version are retired, not freed, so a reference obtained before an +// edit stays valid (if stale) for the lifetime of the view, as with Rows(). +class RowsView { +public: + explicit RowsView(const Buffer &buf) : buf_(&buf) {} + + + [[nodiscard]] std::size_t size() const + { + return buf_->Nrows(); + } + + + [[nodiscard]] bool empty() const + { + return size() == 0; + } + + + const std::string &operator[](std::size_t row) const + { + if (buf_->Version() != version_) { + if (!cache_.empty()) + retired_.push_back(std::move(cache_)); + cache_.clear(); + version_ = buf_->Version(); + } + auto it = cache_.find(row); + if (it == cache_.end()) + it = cache_.emplace(row, buf_->GetLineString(row)).first; + return it->second; + } + +private: + const Buffer *buf_; + mutable std::uint64_t version_ = ~std::uint64_t{0}; + mutable std::map cache_; + mutable std::vector > retired_; +}; + + +static RowsView +rows_of(const Buffer &buf) +{ + return RowsView(buf); +} + + // Helper: extract text from [sx,sy) to [ex,ey) without modifying buffer. Newlines inserted between lines. static std::string extract_region_text(const Buffer &buf, std::size_t sx, std::size_t sy, std::size_t ex, std::size_t ey) { - const auto &rows = buf.Rows(); + const auto &rows = rows_of(buf); if (sy >= rows.size()) return std::string(); if (ey >= rows.size()) @@ -414,7 +527,7 @@ delete_region(Buffer &buf, std::size_t sx, std::size_t sy, std::size_t ex, std:: ey = nrows - 1; if (sy == ey) { // Single line: delete text from xs to xe - const auto &rows = buf.Rows(); + const auto &rows = rows_of(buf); const auto &line = rows[sy]; std::size_t xs = std::min(sx, line.size()); std::size_t xe = std::min(ex, line.size()); @@ -519,7 +632,7 @@ insert_text_at_cursor(Buffer &buf, const std::string &text) if (cur_y >= nrows) { buf.insert_row(static_cast(nrows), ""); } - const auto &rows = buf.Rows(); + const auto &rows = rows_of(buf); if (cur_x > rows[cur_y].size()) cur_x = rows[cur_y].size(); buf.insert_text(static_cast(cur_y), static_cast(cur_x), remain); @@ -529,7 +642,7 @@ insert_text_at_cursor(Buffer &buf, const std::string &text) // insert segment before newline std::string seg = remain.substr(0, pos); { - const auto &rows = buf.Rows(); + const auto &rows = rows_of(buf); if (cur_x > rows[cur_y].size()) cur_x = rows[cur_y].size(); } @@ -616,7 +729,7 @@ cmd_move_cursor_to(CommandContext &ctx) std::size_t by = bro + vy; // Clamp by to existing lines later ensure_at_least_one_line(*buf); - const auto &lines2 = buf->Rows(); + const auto &lines2 = rows_of(*buf); if (by >= lines2.size()) by = lines2.size() - 1; std::string line2 = static_cast(lines2[by]); @@ -634,7 +747,7 @@ cmd_move_cursor_to(CommandContext &ctx) } } ensure_at_least_one_line(*buf); - const auto &lines = buf->Rows(); + const auto &lines = rows_of(*buf); if (row >= lines.size()) row = lines.size() - 1; std::string line = static_cast(lines[row]); @@ -2198,7 +2311,7 @@ cmd_insert_text(CommandContext &ctx) ctx.editor.SetSearchQuery(q); // Recompute matches and move cursor to current index - const auto &rows = buf->Rows(); + const auto &rows = rows_of(*buf); std::vector > matches; if (!q.empty()) { for (std::size_t y = 0; y < rows.size(); ++y) { @@ -2250,7 +2363,7 @@ cmd_insert_text(CommandContext &ctx) if (buf->VisualLineActive()) { const std::size_t sy = buf->VisualLineStartY(); const std::size_t ey = buf->VisualLineEndY(); - const auto &rows = buf->Rows(); + const auto &rows = rows_of(*buf); UndoSystem *u = buf->Undo(); std::uint64_t gid = 0; if (u) @@ -3344,7 +3457,7 @@ cmd_backspace(CommandContext &ctx) if (buf->VisualLineActive()) { const std::size_t sy = buf->VisualLineStartY(); const std::size_t ey = buf->VisualLineEndY(); - const auto &rows = buf->Rows(); + const auto &rows = rows_of(*buf); std::uint64_t gid = 0; if (u) gid = u->BeginGroup(); @@ -3360,7 +3473,7 @@ cmd_backspace(CommandContext &ctx) for (int i = 0; i < repeat; ++i) { if (xx == 0) break; - const auto &rows_view = buf->Rows(); + const auto &rows_view = rows_of(*buf); if (yy >= rows_view.size()) break; const std::string_view lv = line_view(rows_view[yy]); @@ -3387,7 +3500,7 @@ cmd_backspace(CommandContext &ctx) } for (int i = 0; i < repeat; ++i) { // Refresh a read-only view of lines for char capture/lengths - const auto &rows_view = buf->Rows(); + const auto &rows_view = rows_of(*buf); if (x > 0) { std::string deleted; std::size_t n = 1; @@ -3455,7 +3568,7 @@ cmd_delete_char(CommandContext &ctx) if (buf->VisualLineActive()) { const std::size_t sy = buf->VisualLineStartY(); const std::size_t ey = buf->VisualLineEndY(); - const auto &rows = buf->Rows(); + const auto &rows = rows_of(*buf); std::uint64_t gid = 0; if (u) gid = u->BeginGroup(); @@ -3468,7 +3581,7 @@ cmd_delete_char(CommandContext &ctx) xx = rows[yy].size(); std::string deleted; for (int i = 0; i < repeat; ++i) { - const auto &rows_view = buf->Rows(); + const auto &rows_view = rows_of(*buf); if (yy >= rows_view.size() || xx >= rows_view[yy].size()) break; const std::string_view lv = line_view(rows_view[yy]); @@ -3490,7 +3603,7 @@ cmd_delete_char(CommandContext &ctx) return true; } for (int i = 0; i < repeat; ++i) { - const auto &rows_view = buf->Rows(); + const auto &rows_view = rows_of(*buf); if (y >= rows_view.size()) break; if (x < rows_view[y].size()) { @@ -3583,7 +3696,7 @@ cmd_kill_to_eol(CommandContext &ctx) UndoSystem *u = buf->Undo(); UndoGroupGuard guard(u); for (int i = 0; i < repeat; ++i) { - const auto &rows_view = buf->Rows(); + const auto &rows_view = rows_of(*buf); if (y >= rows_view.size()) break; if (x < rows_view[y].size()) { @@ -3642,7 +3755,7 @@ cmd_kill_line(CommandContext &ctx) UndoSystem *u = buf->Undo(); UndoGroupGuard guard(u); for (int i = 0; i < repeat; ++i) { - const auto &rows_view = buf->Rows(); + const auto &rows_view = rows_of(*buf); if (rows_view.empty()) break; if (rows_view.size() == 1) { @@ -3677,7 +3790,9 @@ cmd_kill_line(CommandContext &ctx) u->Append(std::string_view(deleted)); u->commit(); } + // End of buffer reached: a repeat count must not continue upward. y -= 1; + break; } else if (y < rows_view.size()) { // erase current line; keep y pointing at the next line std::string content = static_cast(rows_view[y]); @@ -3690,14 +3805,14 @@ cmd_kill_line(CommandContext &ctx) u->commit(); } buf->delete_row(static_cast(y)); - const auto &rows_after = buf->Rows(); + const auto &rows_after = rows_of(*buf); if (y >= rows_after.size()) { // deleted last line; move to previous y = rows_after.empty() ? 0 : rows_after.size() - 1; } } else { // out of range - const auto &rows2 = buf->Rows(); + const auto &rows2 = rows_of(*buf); y = rows2.empty() ? 0 : rows2.size() - 1; } } @@ -3826,7 +3941,7 @@ cmd_move_file_end(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - const auto &rows = buf->Rows(); + const auto &rows = rows_of(*buf); std::size_t y = rows.empty() ? 0 : rows.size() - 1; std::size_t x = rows.empty() ? 0 : rows[y].size(); buf->SetCursor(x, y); @@ -4021,7 +4136,7 @@ cmd_move_left(CommandContext &ctx) return true; } ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); std::size_t y = buf->Cury(); std::size_t x = buf->Curx(); int repeat = ctx.count > 0 ? ctx.count : 1; @@ -4106,7 +4221,7 @@ cmd_move_right(CommandContext &ctx) return true; } ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); std::size_t y = buf->Cury(); std::size_t x = buf->Curx(); int repeat = ctx.count > 0 ? ctx.count : 1; @@ -4151,7 +4266,7 @@ cmd_move_up(CommandContext &ctx) return true; } ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); std::size_t y = buf->Cury(); std::size_t x = buf->Curx(); int repeat = ctx.count > 0 ? ctx.count : 1; @@ -4200,7 +4315,7 @@ cmd_move_down(CommandContext &ctx) return true; } ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); std::size_t y = buf->Cury(); std::size_t x = buf->Curx(); int repeat = ctx.count > 0 ? ctx.count : 1; @@ -4250,7 +4365,7 @@ cmd_move_end(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); std::size_t y = buf->Cury(); std::size_t x = (y < rows.size()) ? rows[y].size() : 0; buf->SetCursor(x, y); @@ -4268,7 +4383,7 @@ cmd_page_up(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); int repeat = ctx.count > 0 ? ctx.count : 1; std::size_t content_rows = std::max(1, ctx.editor.ContentRows()); @@ -4307,7 +4422,7 @@ cmd_page_down(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); int repeat = ctx.count > 0 ? ctx.count : 1; std::size_t content_rows = std::max(1, ctx.editor.ContentRows()); @@ -4343,7 +4458,7 @@ cmd_scroll_up(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - const auto &rows = buf->Rows(); + const auto &rows = rows_of(*buf); std::size_t content_rows = std::max(1, ctx.editor.ContentRows()); std::size_t rowoffs = buf->Rowoffs(); @@ -4379,7 +4494,7 @@ cmd_scroll_down(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - const auto &rows = buf->Rows(); + const auto &rows = rows_of(*buf); std::size_t content_rows = std::max(1, ctx.editor.ContentRows()); std::size_t rowoffs = buf->Rowoffs(); @@ -4411,7 +4526,10 @@ cmd_scroll_down(CommandContext &ctx) static inline bool is_word_char(unsigned char c) { - return std::isalnum(c) || c == '_'; + // Bytes >= 0x80 belong to multibyte UTF-8 characters: count them as word + // characters, so word motion and deletion never stop inside a character + // (and letters outside ASCII are part of words). + return std::isalnum(c) || c == '_' || c >= 0x80; } @@ -4424,7 +4542,7 @@ cmd_word_prev(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); std::size_t y = buf->Cury(); std::size_t x = buf->Curx(); int repeat = ctx.count > 0 ? ctx.count : 1; @@ -4481,7 +4599,7 @@ cmd_word_next(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); std::size_t y = buf->Cury(); std::size_t x = buf->Curx(); int repeat = ctx.count > 0 ? ctx.count : 1; @@ -4536,7 +4654,7 @@ cmd_delete_word_prev(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); std::size_t y = buf->Cury(); std::size_t x = buf->Curx(); int repeat = ctx.count > 0 ? ctx.count : 1; @@ -4610,7 +4728,7 @@ cmd_delete_word_next(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - const auto &rows = buf->Rows(); + const auto &rows = rows_of(*buf); std::size_t y = buf->Cury(); std::size_t x = buf->Curx(); int repeat = ctx.count > 0 ? ctx.count : 1; @@ -4691,7 +4809,7 @@ cmd_indent_region(CommandContext &ctx) } UndoSystem *u = buf->Undo(); UndoGroupGuard guard(u); - for (std::size_t y = sy; y <= ey && y < buf->Rows().size(); ++y) { + for (std::size_t y = sy; y <= ey && y < rows_of(*buf).size(); ++y) { buf->insert_text(static_cast(y), 0, std::string_view("\t")); if (u) { buf->SetCursor(0, y); @@ -4724,8 +4842,8 @@ cmd_unindent_region(CommandContext &ctx) } UndoSystem *u = buf->Undo(); UndoGroupGuard guard(u); - for (std::size_t y = sy; y <= ey && y < buf->Rows().size(); ++y) { - const auto &rows_view = buf->Rows(); + for (std::size_t y = sy; y <= ey && y < rows_of(*buf).size(); ++y) { + const auto &rows_view = rows_of(*buf); if (y >= rows_view.size()) break; const std::string line = static_cast(rows_view[y]); @@ -4773,7 +4891,7 @@ cmd_reflow_paragraph(CommandContext &ctx) if (auto *u = buf->Undo()) u->commit(); ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); std::size_t y = buf->Cury(); // Treat a universal-argument count of 1 as "no width specified". // Editor::UArgGet() returns 1 when no explicit count was provided. @@ -5112,7 +5230,7 @@ cmd_reload_buffer(CommandContext &ctx) } // Try to restore the cursor to its previous position if still valid; otherwise clamp { - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); const std::size_t nrows = rows.size(); if (nrows == 0) { buf->SetCursor(0, 0); @@ -5137,7 +5255,7 @@ cmd_mark_all_and_jump_end(CommandContext &ctx) return false; ensure_at_least_one_line(*buf); buf->SetMark(0, 0); - auto &rows = buf->Rows(); + auto rows = rows_of(*buf); std::size_t last_y = rows.empty() ? 0 : rows.size() - 1; std::size_t last_x = last_y < rows.size() ? rows[last_y].size() : 0; buf->SetCursor(last_x, last_y); @@ -5412,6 +5530,15 @@ Execute(Editor &ed, CommandId id, const std::string &arg, int count) CommandId::CopyRegion && id != CommandId::DeleteWordPrev && id != CommandId::DeleteWordNext) { ed.SetKillChain(false); } + // While a prompt is open, keys edit the prompt; only commands that know + // about prompts (or only change the view) may run. Anything else would + // edit the buffer behind the prompt (skipping the read-only check below) + // or switch/close buffers under a pending confirmation, which then acts + // on the wrong buffer. + if (ed.PromptActive() && !allowed_during_prompt(id)) { + ed.SetStatus("Finish or cancel the prompt first (C-g)"); + return true; + } // If buffer is read-only, block mutating commands outside of prompts if (!ed.PromptActive()) { Buffer *b = ed.CurrentBuffer(); @@ -5447,6 +5574,13 @@ Execute(Editor &ed, const std::string &name, const std::string &arg, int count) const Command *cmd = CommandRegistry::FindByName(name); if (!cmd) return false; + if (!ed.PromptActive()) { + Buffer *b = ed.CurrentBuffer(); + if (b && b->IsReadOnly() && is_mutating_command(cmd->id)) { + ed.SetStatus("Read-only buffer"); + return true; + } + } CommandContext ctx{ed, arg, count}; return cmd->handler ? cmd->handler(ctx) : false; } \ No newline at end of file diff --git a/Editor.cc b/Editor.cc index 81b9db3..3615242 100644 --- a/Editor.cc +++ b/Editor.cc @@ -9,6 +9,12 @@ #include "syntax/NullHighlighter.h" +namespace { +// Universal-argument repeat counts are capped here (well below INT_MAX). +constexpr int kMaxUArgCount = 1000000; +} // namespace + + namespace { static std::string buffer_bytes_via_views(const Buffer &b) @@ -594,7 +600,7 @@ Editor::UArgStart() if (ucount_ == 0) { ucount_ = 1; } - ucount_ *= 4; + ucount_ = std::min(ucount_ * 4, kMaxUArgCount); // no int overflow } uarg_ = 1; char buf[64]; @@ -614,7 +620,7 @@ Editor::UArgDigit(int d) uarg_ = 1; ucount_ = 0; } - ucount_ = ucount_ * 10 + d; + ucount_ = (ucount_ > (kMaxUArgCount - d) / 10) ? kMaxUArgCount : ucount_ * 10 + d; char buf[64]; std::snprintf(buf, sizeof(buf), "C-u %d", ucount_); SetStatus(buf); diff --git a/ImGuiInputHandler.cc b/ImGuiInputHandler.cc index fd81382..47de96c 100644 --- a/ImGuiInputHandler.cc +++ b/ImGuiInputHandler.cc @@ -394,8 +394,11 @@ ImGuiInputHandler::ProcessSDLEvent(const SDL_Event &e) // Turn line breaks (\n, \r\n, or bare \r) into Newline // commands and the rest into InsertText; InsertText itself // rejects any embedded '\r'/'\n'. + // Into a prompt, line breaks must not become Newline: that + // would accept the prompt and send the rest to the buffer. + const bool to_prompt = ed_ && ed_->PromptActive(); std::lock_guard lk(mu_); - for (const auto &cmd : SplitPasteIntoCommands(text)) + for (const auto &cmd: to_prompt ? SplitPasteForPrompt(text) : SplitPasteIntoCommands(text)) q_.push(cmd); // Suppress the corresponding TEXTINPUT that may follow suppress_text_input_once_ = true; diff --git a/PasteSplit.h b/PasteSplit.h index 8c90cc4..06046ad 100644 --- a/PasteSplit.h +++ b/PasteSplit.h @@ -46,3 +46,27 @@ inline std::vector SplitPasteIntoCommands(const std::string &text) return out; } + + +// Pasted text destined for a prompt (search, file name, command). Newline +// would accept the prompt, so each line break (\n, \r\n or \r) becomes a +// single space and the whole paste is one InsertText. +inline std::vector SplitPasteForPrompt(const std::string &text) +{ + std::string flat; + flat.reserve(text.size()); + for (std::size_t i = 0; i < text.size(); ++i) { + const char c = text[i]; + if (c == '\n' || c == '\r') { + flat.push_back(' '); + if (c == '\r' && i + 1 < text.size() && text[i + 1] == '\n') + ++i; + } else { + flat.push_back(c); + } + } + std::vector out; + if (!flat.empty()) + out.push_back(MappedInput{true, CommandId::InsertText, flat, 0}); + return out; +} diff --git a/UndoSystem.cc b/UndoSystem.cc index aad60c7..ba1cc95 100644 --- a/UndoSystem.cc +++ b/UndoSystem.cc @@ -16,6 +16,9 @@ UndoSystem::BeginGroup() commit(); if (active_group_id_ == 0) active_group_id_ = next_group_id_++; + // Groups nest (e.g. SmartNewline wraps Newline, which groups its own + // visual-line splits); only the outermost EndGroup closes the group. + ++group_depth_; return active_group_id_; } @@ -24,7 +27,10 @@ void UndoSystem::EndGroup() { commit(); - active_group_id_ = 0; + if (group_depth_ > 0) + --group_depth_; + if (group_depth_ == 0) + active_group_id_ = 0; } @@ -267,6 +273,7 @@ UndoSystem::clear() tree_.current = nullptr; tree_.saved = nullptr; active_group_id_ = 0; + group_depth_ = 0; next_group_id_ = 1; update_dirty_flag(); } diff --git a/UndoSystem.h b/UndoSystem.h index df6cbd5..4102d69 100644 --- a/UndoSystem.h +++ b/UndoSystem.h @@ -114,6 +114,7 @@ private: PendingAppendMode pending_mode_ = PendingAppendMode::Append; std::uint64_t active_group_id_ = 0; + int group_depth_ = 0; // nesting depth of BeginGroup/EndGroup std::uint64_t next_group_id_ = 1; Buffer *buf_; diff --git a/tests/test_audit_regressions.cc b/tests/test_audit_regressions.cc index dc94142..d515e83 100644 --- a/tests/test_audit_regressions.cc +++ b/tests/test_audit_regressions.cc @@ -578,3 +578,120 @@ TEST(Audit_PieceTable_MovedFrom_LineIndexReset) ASSERT_EQ(a.LineCount(), (std::size_t) 1); ASSERT_EQ(b.LineCount(), (std::size_t) 4); } + + +// Kill-line with a count stops at the end of the buffer instead of killing +// lines above the starting line. +TEST(Audit_KillLine_Count_StopsAtEndOfBuffer) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, "a\nb\nc"); + b.SetCursor(0, 1); + ASSERT_TRUE(h.Exec(CommandId::KillLine, std::string(), 3)); + ASSERT_EQ(h.Text(), std::string("a")); +} + + +// A mark left past the end of the buffer by edits is clamped before use: +// kill-region deletes the text between the clamped mark and the cursor. +TEST(Audit_KillRegion_StaleMarkIsClamped) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, "abcdefgh"); + b.SetMark(1, 3); // no such row + b.SetCursor(5, 0); + ASSERT_TRUE(h.Exec(CommandId::KillRegion)); + ASSERT_EQ(h.Text(), std::string("abcde")); + ASSERT_EQ(h.EditorRef().KillRingHead(), std::string("fgh")); +} + + +// After kill-region the cursor is on a real column, so typing then undoing +// removes exactly what was typed. +TEST(Audit_KillRegion_CursorClampedForUndo) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, "worldond\nsecond\n"); + b.SetMark(11, 0); // past end of line 0 (the line was shortened) + b.SetCursor(3, 1); + ASSERT_TRUE(h.Exec(CommandId::KillRegion)); + ASSERT_EQ(h.Text(), std::string("worldondond\n")); + ASSERT_EQ(b.Curx(), (std::size_t) 8); + ASSERT_TRUE(h.Exec(CommandId::InsertText, "Z")); + ASSERT_TRUE(h.Undo()); + ASSERT_EQ(h.Text(), std::string("worldondond\n")); +} + + +// Undo groups nest: SmartNewline in visual-line mode (which groups the +// visual newline inside its own group) is a single undo step. +TEST(Audit_UndoGroups_Nest_SmartNewlineVisual) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, " ab\n cd"); + b.SetCursor(3, 0); + b.VisualLineStart(); + b.VisualLineSetActiveY(1); + b.SetCursor(3, 1); + ASSERT_TRUE(h.Exec(CommandId::SmartNewline)); + b.VisualLineClear(); + ASSERT_TRUE(h.Undo()); + ASSERT_EQ(h.Text(), std::string(" ab\n cd")); +} + + +// Word motion and word deletion never stop inside a UTF-8 character. +TEST(Audit_WordMotion_Utf8) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, "ab \xC3\xA9"); + b.SetCursor(5, 0); + ASSERT_TRUE(h.Exec(CommandId::WordPrev)); + ASSERT_EQ(b.Curx(), (std::size_t) 3); + b.SetCursor(5, 0); + ASSERT_TRUE(h.Exec(CommandId::DeleteWordPrev)); + ASSERT_EQ(h.Text(), std::string("ab ")); +} + + +// With a prompt open, buffer-editing commands do not run (they used to edit +// the buffer behind the prompt, skipping the read-only check). +TEST(Audit_Prompt_BlocksBufferCommands) +{ + TestHarness h; + Editor &ed = h.EditorRef(); + Buffer &b = h.Buf(); + b.insert_text(0, 0, "one\ntwo\n"); + b.SetCursor(0, 0); + ASSERT_TRUE(h.Exec(CommandId::FindStart)); + ASSERT_TRUE(ed.PromptActive()); + ASSERT_TRUE(h.Exec(CommandId::KillLine)); + ASSERT_TRUE(h.Exec(CommandId::DeleteChar)); + ASSERT_TRUE(h.Exec(CommandId::BufferNext)); + ASSERT_TRUE(ed.PromptActive()); + ASSERT_TRUE(ed.CurrentBuffer() == &b); + ASSERT_TRUE(h.Exec(CommandId::Refresh)); // C-g still cancels + ASSERT_TRUE(!ed.PromptActive()); + ASSERT_EQ(h.Text(), std::string("one\ntwo\n")); +} + + +// The universal-argument count saturates instead of overflowing int. +TEST(Audit_UArg_DoesNotOverflow) +{ + TestHarness h; + Editor &ed = h.EditorRef(); + ed.UArgStart(); + for (int i = 0; i < 12; ++i) + ed.UArgDigit(9); + ASSERT_EQ(ed.UArgGet(), 1000000); + ed.UArgStart(); + for (int i = 0; i < 40; ++i) + ed.UArgStart(); + ASSERT_EQ(ed.UArgGet(), 1000000); +} diff --git a/tests/test_paste_split.cc b/tests/test_paste_split.cc index 3ea9280..d850d39 100644 --- a/tests/test_paste_split.cc +++ b/tests/test_paste_split.cc @@ -102,3 +102,15 @@ TEST(PasteSplit_MultilineCRLF) ASSERT_TRUE(cmds[3].id == CommandId::Newline); ASSERT_TRUE(cmds[4].arg == "line3"); } + + +// Pasting into a prompt must not produce Newline (which accepts the prompt): +// line breaks become spaces in a single InsertText. +TEST(PasteSplit_ForPrompt_FlattensLineBreaks) +{ + const auto cmds = SplitPasteForPrompt("foo\nbar\r\nbaz\rqux"); + ASSERT_EQ(cmds.size(), (std::size_t) 1); + ASSERT_TRUE(cmds[0].id == CommandId::InsertText); + ASSERT_EQ(cmds[0].arg, std::string("foo bar baz qux")); + ASSERT_EQ(SplitPasteForPrompt("").size(), (std::size_t) 0); +}