kyle/kte
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Commands: keep line structure intact at end of buffer; undo visual newline

Browse Source 
- Regex replace-all, reflow and kill-region replaced or deleted whole
  rows, which added or left a stray newline when the text ended on a
  last line without one. They now edit the byte range in place.
- Regex replace-all no longer matches the empty position after a final
  newline. Before, a zero-width pattern such as ^ matched it, added a
  line, and matched the new empty last row again: the editor hung.
- Kill-line on the last row kills an unterminated line with the newline
  before it, and does nothing on the empty row after a final newline
  (it used to record an undo step that added a line).
- The visual-line newline records its splits as one undo step, and
  places the cursor correctly for a repeat count.
- Buffer::delete_text computes its byte range directly instead of
  materializing every line it spans.

Audit findings D3, D4, D5.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB
This commit is contained in:
Claude
2026-09-24 09:16:04 -07:00
committed by Kyle Isom
parent aa53d75f96
commit a69664037d
3 changed files with 255 additions and 151 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Buffer.cc
+10 -43
View File
@@ -699,51 +699,18 @@ Buffer::delete_text(int row, int col, std::size_t len)
if (col < 0)
col = 0;
// `len` counts each newline as one character, so the deletion is simply
// the next `len` bytes from the (clamped) start, capped at end of buffer.
const std::size_t start = content_.LineColToByteOffset(static_cast<std::size_t>(row),
static_cast<std::size_t>(col));
std::size_t r = static_cast<std::size_t>(row);
std::size_t c = static_cast<std::size_t>(col);
std::size_t remaining = len;
const std::size_t lc = content_.LineCount();
while (remaining > 0 && r < lc) {
const std::string line = content_.GetLine(r); // logical line (without trailing '\n')
const std::size_t L = line.size();
if (c < L) {
const std::size_t take = std::min(remaining, L - c);
c += take;
remaining -= take;
}
if (remaining == 0)
break;
// Consume newline between lines as one char, if there is a next line
if (r + 1 < lc) {
remaining -= 1; // the newline
r += 1;
c = 0;
} else {
// At last line and still remaining: delete to EOF
const std::size_t total = content_.Size();
const std::size_t actual = (total > start) ? (total - start) : 0;
if (actual == 0)
return;
content_.Delete(start, actual);
rows_cache_dirty_ = true;
if (swap_rec_)
swap_rec_->OnDelete(row, col, actual);
return;
}
}
// Compute end offset at (r,c)
std::size_t end = content_.LineColToByteOffset(r, c);
if (end > start) {
const std::size_t actual = end - start;
content_.Delete(start, actual);
rows_cache_dirty_ = true;
if (swap_rec_)
swap_rec_->OnDelete(row, col, actual);
}
const std::size_t total = content_.Size();
const std::size_t actual = (total > start) ? std::min(len, total - start) : 0;
if (actual == 0)
return;
content_.Delete(start, actual);
rows_cache_dirty_ = true;
if (swap_rec_)
swap_rec_->OnDelete(row, col, actual);
}
Command.cc
+122 -108
View File
@@ -337,59 +337,30 @@ delete_region(Buffer &buf, std::size_t sx, std::size_t sy, std::size_t ex, std::
u->commit();
}
} else {
// Multi-line: delete from (sx,sy) to (ex,ey)
// Strategy:
// 1. Save suffix of last line (from ex to end)
// 2. Delete tail of first line (from sx to end)
// 3. Delete all lines from sy+1 to ey (inclusive)
// 4. Insert saved suffix at end of first line
// 5. Join if needed (no, suffix is appended directly)
const auto &rows = buf.Rows();
std::size_t first_line_len = rows[sy].size();
std::size_t last_line_len = rows[ey].size();
std::size_t xs = std::min(sx, first_line_len);
std::size_t xe = std::min(ex, last_line_len);
// Save suffix of last line before any modifications
std::string suffix = rows[ey].substr(xe);
// Delete tail of first line (from xs to end)
if (xs < first_line_len) {
std::string tail = std::string(rows[sy].substr(xs));
buf.delete_text(static_cast<int>(sy), static_cast<int>(xs), first_line_len - xs);
if (u && !tail.empty()) {
buf.SetCursor(xs, sy);
u->Begin(UndoType::Delete);
u->Append(std::string_view(tail));
u->commit();
// Multi-line: delete from (sx,sy) to (ex,ey) as one contiguous range.
// Deleting whole rows instead would leave a stray newline when the
// region ends on a last line that has no trailing newline.
std::string deleted;
std::size_t xs = 0;
for (std::size_t y = sy; y <= ey; ++y) {
const std::string line = buf.GetLineString(y);
if (y == sy) {
xs = std::min(sx, line.size());
deleted.append(line, xs, std::string::npos);
deleted.push_back('\n');
} else if (y < ey) {
deleted.append(line);
deleted.push_back('\n');
} else {
deleted.append(line, 0, std::min(ex, line.size()));
}
}
// Delete lines from ey down to sy+1 (reverse order to preserve indices)
for (std::size_t i = ey; i > sy; --i) {
if (u) {
std::string row_text = static_cast<std::string>(buf.Rows()[i]);
buf.SetCursor(0, i);
u->Begin(UndoType::DeleteRow);
u->Append(std::string_view(row_text));
u->commit();
}
buf.delete_row(static_cast<int>(i));
}
// Append saved suffix to first line
if (!suffix.empty()) {
// Get current length of line sy after deletions
const auto &rows_after = buf.Rows();
std::size_t line_len = rows_after[sy].size();
buf.insert_text(static_cast<int>(sy), static_cast<int>(line_len), suffix);
if (u) {
buf.SetCursor(line_len, sy);
u->Begin(UndoType::Insert);
u->Append(std::string_view(suffix));
u->commit();
}
buf.delete_text(static_cast<int>(sy), static_cast<int>(xs), deleted.size());
if (u && !deleted.empty()) {
buf.SetCursor(xs, sy);
u->Begin(UndoType::Delete);
u->Append(std::string_view(deleted));
u->commit();
}
}
buf.SetCursor(sx, sy);
@@ -397,6 +368,35 @@ delete_region(Buffer &buf, std::size_t sx, std::size_t sy, std::size_t ex, std::
}
// Replace the text of rows [y, y + old_text lines) in place with new_text,
// where old_text is those rows joined by '\n' (no trailing newline). Unlike
// delete_row/insert_row this never adds or removes the newline after the
// last replaced row, so a file without a trailing newline keeps that shape.
static void
replace_rows_text(Buffer &buf, std::size_t y, const std::string &old_text, const std::string &new_text,
UndoSystem *u)
{
if (!old_text.empty()) {
buf.delete_text(static_cast<int>(y), 0, old_text.size());
if (u) {
buf.SetCursor(0, y);
u->Begin(UndoType::Delete);
u->Append(std::string_view(old_text));
u->commit();
}
}
if (!new_text.empty()) {
buf.insert_text(static_cast<int>(y), 0, new_text);
if (u) {
buf.SetCursor(0, y);
u->Begin(UndoType::Insert);
u->Append(std::string_view(new_text));
u->commit();
}
}
}
// Insert arbitrary text at cursor, supporting newlines. Updates cursor position.
static void
insert_text_at_cursor(Buffer &buf, const std::string &text)
@@ -3006,29 +3006,24 @@ cmd_newline(CommandContext &ctx)
std::size_t changed = 0;
UndoSystem *ru = buf->Undo();
UndoGroupGuard rguard(ru);
// Iterate by index to allow modifications via PieceTable helpers
for (std::size_t y = 0; y < buf->Rows().size(); ++y) {
std::string before = static_cast<std::string>(buf->Rows()[y]);
std::string after = std::regex_replace(before, rx, repl);
// When the buffer ends with '\n', the last row is the empty position
// after it, not a line; a zero-width pattern like ^ must not match it.
std::size_t nrows = buf->Nrows();
if (nrows > 1 && buf->GetLineString(nrows - 1).empty())
--nrows;
for (std::size_t y = 0; y < nrows; ++y) {
const std::string before = buf->GetLineString(y);
const std::string after = std::regex_replace(before, rx, repl);
if (after != before) {
// Replace entire line y with 'after' using PieceTable ops
if (ru) {
buf->SetCursor(0, y);
ru->Begin(UndoType::DeleteRow);
ru->Append(std::string_view(before));
ru->commit();
}
buf->delete_row(static_cast<int>(y));
buf->insert_row(static_cast<int>(y), std::string_view(after));
if (ru) {
buf->SetCursor(0, y);
ru->Begin(UndoType::InsertRow);
ru->Append(std::string_view(after));
ru->commit();
}
replace_rows_text(*buf, y, before, after, ru);
// A replacement containing newlines adds rows; skip past them.
const auto added = static_cast<std::size_t>(std::count(after.begin(), after.end(), '\n'));
y += added;
nrows += added;
++changed;
}
}
clamp_cursor_to_buffer(*buf);
buf->SetDirty(true);
ctx.editor.SetStatus("Regex replaced in " + std::to_string(changed) + " line(s)");
// Clear search UI state
@@ -3068,32 +3063,42 @@ cmd_newline(CommandContext &ctx)
if (buf->VisualLineActive()) {
const std::size_t sy = buf->VisualLineStartY();
const std::size_t ey = buf->VisualLineEndY();
const auto &rows = buf->Rows();
if (rows.empty())
if (buf->Nrows() == 0)
return true;
std::size_t splits_above = 0;
if (sy < y)
splits_above = std::min(ey, y - 1) - sy + 1;
// Iterate bottom-up to keep row indices stable while splitting.
for (std::size_t yy = ey + 1; yy-- > sy;) {
const auto &rows_view = buf->Rows();
if (yy >= rows_view.size())
continue;
std::size_t xx = x;
if (xx > rows_view[yy].size())
xx = rows_view[yy].size();
// First split at the cursor column; subsequent splits create blank lines.
buf->split_line(static_cast<int>(yy), static_cast<int>(xx));
for (int i = 1; i < repeat; ++i) {
buf->split_line(static_cast<int>(yy + i), 0);
// Every split is recorded, as one undo step. Begin() takes the node's
// position from the cursor, so place the cursor at each split first.
UndoSystem *vu = buf->Undo();
UndoGroupGuard vguard(vu);
auto split_at = [&](std::size_t row, std::size_t col) {
buf->SetCursor(col, row);
if (vu) {
vu->Begin(UndoType::Newline);
vu->commit();
}
buf->split_line(static_cast<int>(row), static_cast<int>(col));
};
// Iterate bottom-up to keep row indices stable while splitting.
const std::size_t nrows = buf->Nrows();
for (std::size_t yy = ey + 1; yy-- > sy;) {
if (yy >= nrows)
continue;
const std::size_t xx = std::min(x, buf->GetLineString(yy).size());
// First split at the cursor column; subsequent splits create blank lines.
split_at(yy, xx);
for (int i = 1; i < repeat; ++i)
split_at(yy + static_cast<std::size_t>(i), 0);
}
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<std::size_t>(repeat);
new_y += splits_above;
new_y += splits_above * static_cast<std::size_t>(repeat);
buf->SetCursor(0, new_y);
ensure_cursor_visible(ctx.editor, *buf);
return true;
@@ -3549,6 +3554,25 @@ cmd_kill_line(CommandContext &ctx)
}
}
y = 0;
} else if (y + 1 == rows_view.size()) {
// Last row. If it is empty, the buffer ends with '\n' and this row is
// just the position after it: there is no line to kill. Otherwise
// the line has no trailing newline, so kill it together with the
// newline before it (delete_row would leave that newline behind).
std::string content = static_cast<std::string>(rows_view[y]);
if (content.empty())
break;
const std::size_t prev_len = rows_view[y - 1].size();
killed_total += content;
const std::string deleted = "\n" + content;
buf->delete_text(static_cast<int>(y - 1), static_cast<int>(prev_len), deleted.size());
if (u) {
buf->SetCursor(prev_len, y - 1);
u->Begin(UndoType::Delete);
u->Append(std::string_view(deleted));
u->commit();
}
y -= 1;
} else if (y < rows_view.size()) {
// erase current line; keep y pointing at the next line
std::string content = static_cast<std::string>(rows_view[y]);
@@ -4916,31 +4940,21 @@ cmd_reflow_paragraph(CommandContext &ctx)
if (new_lines.empty())
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<std::string>(buf->Rows()[i]));
u->commit();
}
buf->delete_row(static_cast<int>(i));
if (i == 0)
break; // prevent wrap on size_t
// Replace the paragraph's text in place (see replace_rows_text).
std::string old_text;
for (std::size_t i = para_start; i <= para_end; ++i) {
if (i > para_start)
old_text.push_back('\n');
old_text += static_cast<std::string>(rows[i]);
}
// Insert new lines starting at para_start
std::size_t insert_y = para_start;
for (const auto &ln: new_lines) {
buf->insert_row(static_cast<int>(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;
std::string new_text;
for (std::size_t i = 0; i < new_lines.size(); ++i) {
if (i > 0)
new_text.push_back('\n');
new_text += new_lines[i];
}
if (new_text != old_text)
replace_rows_text(*buf, para_start, old_text, new_text, buf->Undo());
// Place cursor at the end of the paragraph
std::size_t new_last_y = para_start + (new_lines.empty() ? 0 : new_lines.size() - 1);
tests/test_audit_regressions.cc
+123
View File
@@ -248,3 +248,126 @@ TEST(Audit_M3_SharedBuffers_UseOwnersSwapManager)
b.insert_text(0, 0, "abc");
ASSERT_EQ(b.GetLineString(0), std::string("abc"));
}
// D3: killing a region that ends on a last line without a trailing newline
// removes it entirely (no stray newline), and undo restores the original.
TEST(Audit_D3_KillRegion_ToUnterminatedLastLine)
{
TestHarness h;
Buffer &b = h.Buf();
b.insert_text(0, 0, "foo\nbar");
b.SetMark(3, 0);
b.SetCursor(3, 1);
ASSERT_TRUE(h.Exec(CommandId::KillRegion));
ASSERT_EQ(h.Text(), std::string("foo"));
ASSERT_EQ(b.Nrows(), (std::size_t) 1);
ASSERT_TRUE(h.Undo());
ASSERT_EQ(h.Text(), std::string("foo\nbar"));
}
// D3: killing the unterminated last line removes it and the newline before
// it; undo restores it exactly.
TEST(Audit_D3_KillLine_UnterminatedLastLine)
{
TestHarness h;
Buffer &b = h.Buf();
b.insert_text(0, 0, "a\nb");
b.SetCursor(0, 1);
ASSERT_TRUE(h.Exec(CommandId::KillLine));
ASSERT_EQ(h.Text(), std::string("a"));
ASSERT_EQ(h.EditorRef().KillRingHead(), std::string("b"));
ASSERT_TRUE(h.Undo());
ASSERT_EQ(h.Text(), std::string("a\nb"));
}
// D4: the empty row after a trailing newline is not a line; killing it is a
// no-op, and undo must not add a line.
TEST(Audit_D4_KillLine_OnRowAfterTrailingNewline)
{
TestHarness h;
Buffer &b = h.Buf();
b.insert_text(0, 0, "a\n");
b.SetCursor(0, 1);
ASSERT_TRUE(h.Exec(CommandId::KillLine));
ASSERT_EQ(h.Text(), std::string("a\n"));
(void) h.Undo();
ASSERT_EQ(h.Text(), std::string("a\n"));
}
static void
regex_replace_all(TestHarness &h, const std::string &find, const std::string &with)
{
ASSERT_TRUE(h.Exec(CommandId::RegexpReplace));
ASSERT_TRUE(h.Exec(CommandId::InsertText, find));
ASSERT_TRUE(h.Exec(CommandId::Newline));
if (!with.empty())
ASSERT_TRUE(h.Exec(CommandId::InsertText, with));
ASSERT_TRUE(h.Exec(CommandId::Newline));
ASSERT_TRUE(!h.EditorRef().PromptActive());
}
// D4: a zero-width regex does not match the position after the final newline.
TEST(Audit_D4_RegexReplace_ZeroWidth_NoExtraLine)
{
TestHarness h;
h.Buf().insert_text(0, 0, "a\nb\n");
regex_replace_all(h, "^", "# ");
ASSERT_EQ(h.Text(), std::string("# a\n# b\n"));
}
// D3: regex replace on an unterminated last line does not add a newline.
TEST(Audit_D3_RegexReplace_KeepsMissingTrailingNewline)
{
TestHarness h;
h.Buf().insert_text(0, 0, "a\nfoo");
regex_replace_all(h, "foo", "bar");
ASSERT_EQ(h.Text(), std::string("a\nbar"));
ASSERT_TRUE(h.Undo());
ASSERT_EQ(h.Text(), std::string("a\nfoo"));
}
// D3: reflowing the last paragraph of a file without a trailing newline does
// not add one.
TEST(Audit_D3_Reflow_KeepsMissingTrailingNewline)
{
TestHarness h;
Buffer &b = h.Buf();
b.insert_text(0, 0, "one\ntwo");
b.SetCursor(0, 1);
ASSERT_TRUE(h.Exec(CommandId::ReflowParagraph));
ASSERT_EQ(h.Text(), std::string("one two"));
ASSERT_TRUE(h.Undo());
ASSERT_EQ(h.Text(), std::string("one\ntwo"));
}
// D5: the visual-line newline is undoable as one step.
TEST(Audit_D5_VisualLineNewline_Undo)
{
TestHarness h;
Buffer &b = h.Buf();
b.insert_text(0, 0, "ab\ncd\nef");
b.SetCursor(1, 0);
b.VisualLineStart();
b.VisualLineSetActiveY(1);
b.SetCursor(1, 1);
ASSERT_TRUE(h.Exec(CommandId::Newline));
ASSERT_EQ(h.Text(), std::string("a\nb\nc\nd\nef"));
b.VisualLineClear();
ASSERT_TRUE(h.Undo());
ASSERT_EQ(h.Text(), std::string("ab\ncd\nef"));
}