mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-09-14 18:01:58 +00:00
terminal: mark the previous row dirty when clearing its spacer head (#14054)
Erasing a wrapped wide character at the start of a row (`ECH` or `DCH`) also clears the spacer head it left at the end of the previous row, but that row was never marked dirty. With both rows visible, an incremental render kept the stale spacer head on screen until something unrelated redrew that row. The clearing happens in the row-start branch of `splitCellBoundary`. `clearCells` doesn't do dirty tracking, and both callers only mark the cursor row, so mark the previous row at the point it's mutated. The added dirty assertions fail without the fix. ## AI Disclaimer Claude did the heavy lifting - identifying the root cause, generating code and description. I reviewed and iterated on it to move around and tweak tests, comments and reduce verboseness. Verified the end user visible behaviour improvement with a script that coloured the wide character, which made the stale rendering visible until a switch to the alt screen and back cleared it it.
This commit is contained in:
@@ -1949,6 +1949,11 @@ pub fn splitCellBoundary(
|
||||
p_rac.row,
|
||||
p_cells[p_row.node.cols() - 1 ..][0..1],
|
||||
);
|
||||
|
||||
// `clearCells` does not mark rows dirty, and our
|
||||
// callers only mark the cursor row, so mark the
|
||||
// previous row here.
|
||||
p_row.markDirty();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9922,6 +9922,38 @@ test "Terminal: eraseChars wide char wrap boundary conditions" {
|
||||
}
|
||||
}
|
||||
|
||||
test "Terminal: eraseChars clearing wrapped wide char marks spacer head row dirty" {
|
||||
const alloc = testing.allocator;
|
||||
const io_impl = testing.io;
|
||||
var t = try init(io_impl, alloc, .{ .rows = 3, .cols = 5 });
|
||||
defer t.deinit(alloc);
|
||||
|
||||
// The wide char doesn't fit so it wraps, leaving a spacer head at
|
||||
// the end of the first row.
|
||||
try t.printString("ABCD字");
|
||||
{
|
||||
const list_cell = t.screens.active.pages.getCell(.{ .screen = .{ .x = 4, .y = 0 } }).?;
|
||||
try testing.expectEqual(Cell.Wide.spacer_head, list_cell.cell.wide);
|
||||
try testing.expect(list_cell.row.wrap);
|
||||
}
|
||||
|
||||
t.setCursorPos(2, 1);
|
||||
t.clearDirty();
|
||||
t.eraseChars(1);
|
||||
t.screens.active.cursor.page_pin.node.page().assertIntegrity();
|
||||
|
||||
// Erasing the wide char also clears the spacer head on the previous
|
||||
// row, so that row must be dirty too.
|
||||
try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } }));
|
||||
try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 1 } }));
|
||||
try testing.expect(!t.isDirty(.{ .screen = .{ .x = 0, .y = 2 } }));
|
||||
|
||||
{
|
||||
const list_cell = t.screens.active.pages.getCell(.{ .screen = .{ .x = 4, .y = 0 } }).?;
|
||||
try testing.expectEqual(Cell.Wide.narrow, list_cell.cell.wide);
|
||||
}
|
||||
}
|
||||
|
||||
test "Terminal: reverseIndex" {
|
||||
const alloc = testing.allocator;
|
||||
const io_impl = testing.io;
|
||||
@@ -13356,9 +13388,16 @@ test "Terminal: deleteChars wide char wrap boundary conditions" {
|
||||
}
|
||||
|
||||
t.setCursorPos(2, 2);
|
||||
t.clearDirty();
|
||||
t.deleteChars(3);
|
||||
t.screens.active.cursor.page_pin.node.page().assertIntegrity();
|
||||
|
||||
// Deleting the wide char also clears the spacer head on the previous
|
||||
// row, so that row must be dirty too.
|
||||
try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } }));
|
||||
try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 1 } }));
|
||||
try testing.expect(!t.isDirty(.{ .screen = .{ .x = 0, .y = 2 } }));
|
||||
|
||||
{
|
||||
const str = try t.plainString(alloc);
|
||||
defer testing.allocator.free(str);
|
||||
|
||||
Reference in New Issue
Block a user