terminal: fix crash when reflowing grapheme with a spacer head (#7537)
Fixes #7536 When we're reflowing a row and we need to insert a spacer head, we must move to the next row to insert it. Previously, we were setting a spacer head and then copying data into that spacer head, which could lead to corrupt data and an eventual crash. In debug builds this triggers assertion failures but in release builds this would lead to silent corruption and a crash later on. The unit test shows the issue clearly but effectively you need a multi-codepoint grapheme such as `👨👨👦👦` to wrap across a row by changing the columns.pull/7541/head
commit
1c7623db81
|
|
@ -908,16 +908,6 @@ const ReflowCursor = struct {
|
||||||
const cell = &cells[x];
|
const cell = &cells[x];
|
||||||
x += 1;
|
x += 1;
|
||||||
|
|
||||||
// std.log.warn("\nsrc_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={} wide={}", .{
|
|
||||||
// src_y,
|
|
||||||
// x,
|
|
||||||
// self.y,
|
|
||||||
// self.x,
|
|
||||||
// self.page.size.cols,
|
|
||||||
// cell.content.codepoint,
|
|
||||||
// cell.wide,
|
|
||||||
// });
|
|
||||||
|
|
||||||
// Copy cell contents.
|
// Copy cell contents.
|
||||||
switch (cell.content_tag) {
|
switch (cell.content_tag) {
|
||||||
.codepoint,
|
.codepoint,
|
||||||
|
|
@ -937,8 +927,15 @@ const ReflowCursor = struct {
|
||||||
};
|
};
|
||||||
|
|
||||||
// Decrement the source position so that when we
|
// Decrement the source position so that when we
|
||||||
// loop we'll process this source cell again.
|
// loop we'll process this source cell again,
|
||||||
|
// since we can't copy it into a spacer head.
|
||||||
x -= 1;
|
x -= 1;
|
||||||
|
|
||||||
|
// Move to the next row (this sets pending wrap
|
||||||
|
// which will cause us to wrap on the next
|
||||||
|
// iteration).
|
||||||
|
self.cursorForward();
|
||||||
|
continue;
|
||||||
} else {
|
} else {
|
||||||
self.page_cell.* = cell.*;
|
self.page_cell.* = cell.*;
|
||||||
}
|
}
|
||||||
|
|
@ -990,6 +987,17 @@ const ReflowCursor = struct {
|
||||||
self.page_cell.hyperlink = false;
|
self.page_cell.hyperlink = false;
|
||||||
self.page_cell.style_id = stylepkg.default_id;
|
self.page_cell.style_id = stylepkg.default_id;
|
||||||
|
|
||||||
|
// std.log.warn("\nsrc_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={X} wide={} page_cell_wide={}", .{
|
||||||
|
// src_y,
|
||||||
|
// x,
|
||||||
|
// self.y,
|
||||||
|
// self.x,
|
||||||
|
// self.page.size.cols,
|
||||||
|
// cell.content.codepoint,
|
||||||
|
// cell.wide,
|
||||||
|
// self.page_cell.wide,
|
||||||
|
// });
|
||||||
|
|
||||||
// Copy grapheme data.
|
// Copy grapheme data.
|
||||||
if (cell.content_tag == .codepoint_grapheme) {
|
if (cell.content_tag == .codepoint_grapheme) {
|
||||||
// Copy the graphemes
|
// Copy the graphemes
|
||||||
|
|
@ -8375,6 +8383,125 @@ test "PageList resize reflow less cols to wrap a wide char" {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
test "PageList resize reflow less cols to wrap a multi-codepoint grapheme with a spacer head" {
|
||||||
|
const testing = std.testing;
|
||||||
|
const alloc = testing.allocator;
|
||||||
|
|
||||||
|
var s = try init(alloc, 4, 2, 0);
|
||||||
|
defer s.deinit();
|
||||||
|
{
|
||||||
|
try testing.expect(s.pages.first == s.pages.last);
|
||||||
|
const page = &s.pages.first.?.data;
|
||||||
|
|
||||||
|
// We want to make the screen look like this:
|
||||||
|
//
|
||||||
|
// 👨👨👦👦👨👨👦👦
|
||||||
|
|
||||||
|
// First family emoji at (0, 0)
|
||||||
|
{
|
||||||
|
const rac = page.getRowAndCell(0, 0);
|
||||||
|
rac.cell.* = .{
|
||||||
|
.content_tag = .codepoint,
|
||||||
|
.content = .{ .codepoint = 0x1F468 }, // First codepoint of the grapheme
|
||||||
|
.wide = .wide,
|
||||||
|
};
|
||||||
|
try page.setGraphemes(rac.row, rac.cell, &.{
|
||||||
|
0x200D, 0x1F468,
|
||||||
|
0x200D, 0x1F466,
|
||||||
|
0x200D, 0x1F466,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
{
|
||||||
|
const rac = page.getRowAndCell(1, 0);
|
||||||
|
rac.cell.* = .{
|
||||||
|
.content_tag = .codepoint,
|
||||||
|
.content = .{ .codepoint = 0 },
|
||||||
|
.wide = .spacer_tail,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
// Second family emoji at (2, 0)
|
||||||
|
{
|
||||||
|
const rac = page.getRowAndCell(2, 0);
|
||||||
|
rac.cell.* = .{
|
||||||
|
.content_tag = .codepoint,
|
||||||
|
.content = .{ .codepoint = 0x1F468 }, // First codepoint of the grapheme
|
||||||
|
.wide = .wide,
|
||||||
|
};
|
||||||
|
try page.setGraphemes(rac.row, rac.cell, &.{
|
||||||
|
0x200D, 0x1F468,
|
||||||
|
0x200D, 0x1F466,
|
||||||
|
0x200D, 0x1F466,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
{
|
||||||
|
const rac = page.getRowAndCell(3, 0);
|
||||||
|
rac.cell.* = .{
|
||||||
|
.content_tag = .codepoint,
|
||||||
|
.content = .{ .codepoint = 0 },
|
||||||
|
.wide = .spacer_tail,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Resize
|
||||||
|
try s.resize(.{ .cols = 3, .reflow = true });
|
||||||
|
try testing.expectEqual(@as(usize, 3), s.cols);
|
||||||
|
try testing.expectEqual(@as(usize, 2), s.totalRows());
|
||||||
|
|
||||||
|
{
|
||||||
|
try testing.expect(s.pages.first == s.pages.last);
|
||||||
|
const page = &s.pages.first.?.data;
|
||||||
|
|
||||||
|
{
|
||||||
|
const rac = page.getRowAndCell(0, 0);
|
||||||
|
try testing.expectEqual(@as(u21, 0x1F468), rac.cell.content.codepoint);
|
||||||
|
try testing.expectEqual(pagepkg.Cell.Wide.wide, rac.cell.wide);
|
||||||
|
|
||||||
|
const cps = page.lookupGrapheme(rac.cell).?;
|
||||||
|
try testing.expectEqual(@as(usize, 6), cps.len);
|
||||||
|
try testing.expectEqual(@as(u21, 0x200D), cps[0]);
|
||||||
|
try testing.expectEqual(@as(u21, 0x1F468), cps[1]);
|
||||||
|
try testing.expectEqual(@as(u21, 0x200D), cps[2]);
|
||||||
|
try testing.expectEqual(@as(u21, 0x1F466), cps[3]);
|
||||||
|
try testing.expectEqual(@as(u21, 0x200D), cps[4]);
|
||||||
|
try testing.expectEqual(@as(u21, 0x1F466), cps[5]);
|
||||||
|
|
||||||
|
// Row should be wrapped
|
||||||
|
try testing.expect(rac.row.wrap);
|
||||||
|
}
|
||||||
|
{
|
||||||
|
const rac = page.getRowAndCell(1, 0);
|
||||||
|
try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint);
|
||||||
|
try testing.expectEqual(pagepkg.Cell.Wide.spacer_tail, rac.cell.wide);
|
||||||
|
}
|
||||||
|
{
|
||||||
|
const rac = page.getRowAndCell(2, 0);
|
||||||
|
try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint);
|
||||||
|
try testing.expectEqual(pagepkg.Cell.Wide.spacer_head, rac.cell.wide);
|
||||||
|
}
|
||||||
|
|
||||||
|
{
|
||||||
|
const rac = page.getRowAndCell(0, 0);
|
||||||
|
try testing.expectEqual(@as(u21, 0x1F468), rac.cell.content.codepoint);
|
||||||
|
try testing.expectEqual(pagepkg.Cell.Wide.wide, rac.cell.wide);
|
||||||
|
|
||||||
|
const cps = page.lookupGrapheme(rac.cell).?;
|
||||||
|
try testing.expectEqual(@as(usize, 6), cps.len);
|
||||||
|
try testing.expectEqual(@as(u21, 0x200D), cps[0]);
|
||||||
|
try testing.expectEqual(@as(u21, 0x1F468), cps[1]);
|
||||||
|
try testing.expectEqual(@as(u21, 0x200D), cps[2]);
|
||||||
|
try testing.expectEqual(@as(u21, 0x1F466), cps[3]);
|
||||||
|
try testing.expectEqual(@as(u21, 0x200D), cps[4]);
|
||||||
|
try testing.expectEqual(@as(u21, 0x1F466), cps[5]);
|
||||||
|
}
|
||||||
|
{
|
||||||
|
const rac = page.getRowAndCell(1, 1);
|
||||||
|
try testing.expectEqual(@as(u21, 0), rac.cell.content.codepoint);
|
||||||
|
try testing.expectEqual(pagepkg.Cell.Wide.spacer_tail, rac.cell.wide);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
test "PageList resize reflow less cols copy kitty placeholder" {
|
test "PageList resize reflow less cols copy kitty placeholder" {
|
||||||
const testing = std.testing;
|
const testing = std.testing;
|
||||||
const alloc = testing.allocator;
|
const alloc = testing.allocator;
|
||||||
|
|
|
||||||
|
|
@ -1316,7 +1316,12 @@ pub const Page = struct {
|
||||||
|
|
||||||
/// Set the graphemes for the given cell. This asserts that the cell
|
/// Set the graphemes for the given cell. This asserts that the cell
|
||||||
/// has no graphemes set, and only contains a single codepoint.
|
/// has no graphemes set, and only contains a single codepoint.
|
||||||
pub fn setGraphemes(self: *Page, row: *Row, cell: *Cell, cps: []u21) GraphemeError!void {
|
pub fn setGraphemes(
|
||||||
|
self: *Page,
|
||||||
|
row: *Row,
|
||||||
|
cell: *Cell,
|
||||||
|
cps: []const u21,
|
||||||
|
) GraphemeError!void {
|
||||||
defer self.assertIntegrity();
|
defer self.assertIntegrity();
|
||||||
|
|
||||||
assert(cell.codepoint() > 0);
|
assert(cell.codepoint() > 0);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue