terminal: eraseChars was checking wide char split boundary on wrong cell

Fixes #2817

The test is pretty explanatory. I also renamed `end` to `count` since I
think this poor naming was the reason for the bug. In `eraseChars`, the
`count` (nee `end`) is the number of cells to erase, not the index of
the last cell to erase.
This commit is contained in:
Mitchell Hashimoto
2024-11-26 14:27:44 -08:00
parent 204c5796a7
commit e20b27de84

View File

@ -1955,13 +1955,9 @@ pub fn deleteChars(self: *Terminal, count_req: usize) void {
} }
pub fn eraseChars(self: *Terminal, count_req: usize) void { pub fn eraseChars(self: *Terminal, count_req: usize) void {
const count = @max(count_req, 1); const count = end: {
// Our last index is at most the end of the number of chars we have
// in the current line.
const end = end: {
const remaining = self.cols - self.screen.cursor.x; const remaining = self.cols - self.screen.cursor.x;
var end = @min(remaining, count); var end = @min(remaining, @max(count_req, 1));
// If our last cell is a wide char then we need to also clear the // If our last cell is a wide char then we need to also clear the
// cell beyond it since we can't just split a wide char. // cell beyond it since we can't just split a wide char.
@ -1979,7 +1975,7 @@ pub fn eraseChars(self: *Terminal, count_req: usize) void {
// protected modes. We need to figure out how to make `clearCells` or at // protected modes. We need to figure out how to make `clearCells` or at
// least `clearUnprotectedCells` handle boundary conditions... // least `clearUnprotectedCells` handle boundary conditions...
self.screen.splitCellBoundary(self.screen.cursor.x); self.screen.splitCellBoundary(self.screen.cursor.x);
self.screen.splitCellBoundary(end); self.screen.splitCellBoundary(self.screen.cursor.x + count);
// Reset our row's soft-wrap. // Reset our row's soft-wrap.
self.screen.cursorResetWrap(); self.screen.cursorResetWrap();
@ -1997,7 +1993,7 @@ pub fn eraseChars(self: *Terminal, count_req: usize) void {
self.screen.clearCells( self.screen.clearCells(
&self.screen.cursor.page_pin.node.data, &self.screen.cursor.page_pin.node.data,
self.screen.cursor.page_row, self.screen.cursor.page_row,
cells[0..end], cells[0..count],
); );
return; return;
} }
@ -2005,7 +2001,7 @@ pub fn eraseChars(self: *Terminal, count_req: usize) void {
self.screen.clearUnprotectedCells( self.screen.clearUnprotectedCells(
&self.screen.cursor.page_pin.node.data, &self.screen.cursor.page_pin.node.data,
self.screen.cursor.page_row, self.screen.cursor.page_row,
cells[0..end], cells[0..count],
); );
} }
@ -6104,6 +6100,36 @@ test "Terminal: eraseChars wide char boundary conditions" {
} }
} }
test "Terminal: eraseChars wide char splits proper cell boundaries" {
const alloc = testing.allocator;
var t = try init(alloc, .{ .rows = 1, .cols = 30 });
defer t.deinit(alloc);
// This is a test for a bug: https://github.com/ghostty-org/ghostty/issues/2817
// To explain the setup:
// (1) We need our wide characters starting on an even (1-based) column.
// (2) We need our cursor to be in the middle somewhere.
// (3) We need our count to be less than our cursor X and on a split cell.
// The bug was that we split the wrong cell boundaries.
try t.printString("x食べて下さい");
{
const str = try t.plainString(alloc);
defer testing.allocator.free(str);
try testing.expectEqualStrings("x食べて下さい", str);
}
t.setCursorPos(1, 6); // At:
t.eraseChars(4); // Delete:
t.screen.cursor.page_pin.node.data.assertIntegrity();
{
const str = try t.plainString(alloc);
defer testing.allocator.free(str);
try testing.expectEqualStrings("x食べ さい", str);
}
}
test "Terminal: eraseChars wide char wrap boundary conditions" { test "Terminal: eraseChars wide char wrap boundary conditions" {
const alloc = testing.allocator; const alloc = testing.allocator;
var t = try init(alloc, .{ .rows = 3, .cols = 8 }); var t = try init(alloc, .{ .rows = 3, .cols = 8 });