From 57deadce9765b78fdf7fddb0ecdca8efd216fd65 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 4 Mar 2024 14:08:52 -0800 Subject: [PATCH] terminal/new: more reflow tests with wide chars --- src/terminal/Screen.zig | 2 + src/terminal/new/PageList.zig | 85 ++++++++++++++---------- src/terminal/new/Screen.zig | 119 ++++++++++++++++++++++++++++++++-- src/terminal/new/page.zig | 4 +- 4 files changed, 167 insertions(+), 43 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 0202c2144..58eb8832e 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -7546,6 +7546,7 @@ test "Screen: resize more rows then shrink again" { } } +// X test "Screen: resize less cols to eliminate wide char" { const testing = std.testing; const alloc = testing.allocator; @@ -7580,6 +7581,7 @@ test "Screen: resize less cols to eliminate wide char" { try testing.expect(!cell.attrs.wide_spacer_head); } +// X test "Screen: resize less cols to wrap wide char" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/new/PageList.zig b/src/terminal/new/PageList.zig index 9cfbc22c9..a49681535 100644 --- a/src/terminal/new/PageList.zig +++ b/src/terminal/new/PageList.zig @@ -727,12 +727,13 @@ fn reflowPage( src_cursor.page_cell.wide == .wide and dst_cursor.x == cap.cols - 1) { + reflowUpdateCursor(cursor, &src_cursor, &dst_cursor, dst_node); + dst_cursor.page_cell.* = .{ .content_tag = .codepoint, .content = .{ .codepoint = 0 }, .wide = .spacer_head, }; - // TODO: update cursor dst_cursor.cursorForward(); } @@ -742,6 +743,7 @@ fn reflowPage( src_cursor.page_cell.wide == .spacer_head and dst_cursor.x != cap.cols - 1) { + reflowUpdateCursor(cursor, &src_cursor, &dst_cursor, dst_node); src_cursor.cursorForward(); continue; } @@ -829,40 +831,7 @@ fn reflowPage( // If our original cursor was on this page, this x/y then // we need to update to the new location. - if (cursor) |c| cursor: { - const offset = c.offset orelse break :cursor; - if (&offset.page.data == src_cursor.page and - offset.row_offset == src_cursor.y and - c.x == src_cursor.x) - { - // std.log.warn("c.x={} c.y={} dst_x={} dst_y={} src_y={}", .{ - // c.x, - // c.y, - // dst_cursor.x, - // dst_cursor.y, - // src_cursor.y, - // }); - - // Column always matches our dst x - c.x = dst_cursor.x; - - // Our y is more complicated. The cursor y is the active - // area y, not the row offset. Our cursors are row offsets. - // Instead of calculating the active area coord, we can - // better calculate the CHANGE in coordinate by subtracting - // our dst from src which will calculate how many rows - // we unwrapped to get here. - // - // Note this doesn't handle when we pull down scrollback. - // See the cursor updates in resizeGrowCols for that. - //c.y -|= src_cursor.y - dst_cursor.y; - - c.offset = .{ - .page = dst_node, - .row_offset = dst_cursor.y, - }; - } - } + reflowUpdateCursor(cursor, &src_cursor, &dst_cursor, dst_node); // Move both our cursors forward src_cursor.cursorForward(); @@ -902,6 +871,52 @@ fn reflowPage( self.destroyPage(node); } +/// This updates the cursor offset if the cursor is exactly on the cell +/// we're currently reflowing. This can then be fixed up later to an exact +/// x/y (see resizeCols). +fn reflowUpdateCursor( + cursor: ?*Resize.Cursor, + src_cursor: *const ReflowCursor, + dst_cursor: *const ReflowCursor, + dst_node: *List.Node, +) void { + const c = cursor orelse return; + + // If our original cursor was on this page, this x/y then + // we need to update to the new location. + const offset = c.offset orelse return; + if (&offset.page.data != src_cursor.page or + offset.row_offset != src_cursor.y or + c.x != src_cursor.x) return; + + // std.log.warn("c.x={} c.y={} dst_x={} dst_y={} src_y={}", .{ + // c.x, + // c.y, + // dst_cursor.x, + // dst_cursor.y, + // src_cursor.y, + // }); + + // Column always matches our dst x + c.x = dst_cursor.x; + + // Our y is more complicated. The cursor y is the active + // area y, not the row offset. Our cursors are row offsets. + // Instead of calculating the active area coord, we can + // better calculate the CHANGE in coordinate by subtracting + // our dst from src which will calculate how many rows + // we unwrapped to get here. + // + // Note this doesn't handle when we pull down scrollback. + // See the cursor updates in resizeGrowCols for that. + //c.y -|= src_cursor.y - dst_cursor.y; + + c.offset = .{ + .page = dst_node, + .row_offset = dst_cursor.y, + }; +} + fn resizeWithoutReflow(self: *PageList, opts: Resize) !void { if (opts.rows) |rows| { switch (std.math.order(rows, self.rows)) { diff --git a/src/terminal/new/Screen.zig b/src/terminal/new/Screen.zig index 09992dba1..bef04b786 100644 --- a/src/terminal/new/Screen.zig +++ b/src/terminal/new/Screen.zig @@ -931,17 +931,48 @@ fn testWriteString(self: *Screen, text: []const u8) !void { ref.* += 1; self.cursor.page_row.styled = true; } - - if (self.cursor.x + 1 < self.pages.cols) { - self.cursorRight(1); - } else { - self.cursor.pending_wrap = true; - } }, - 2 => @panic("todo double-width"), + 2 => { + // Need a wide spacer head + if (self.cursor.x == self.pages.cols - 1) { + self.cursor.page_cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 0 }, + .wide = .spacer_head, + }; + + self.cursor.page_row.wrap = true; + try self.cursorDownOrScroll(); + self.cursorHorizontalAbsolute(0); + self.cursor.page_row.wrap_continuation = true; + } + + // Write our wide char + self.cursor.page_cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = c }, + .style_id = self.cursor.style_id, + .wide = .wide, + }; + + // Write our tail + self.cursorRight(1); + self.cursor.page_cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 0 }, + .wide = .spacer_tail, + }; + }, + else => unreachable, } + + if (self.cursor.x + 1 < self.pages.cols) { + self.cursorRight(1); + } else { + self.cursor.pending_wrap = true; + } } } @@ -3120,3 +3151,77 @@ test "Screen: resize more rows then shrink again" { try testing.expectEqualStrings(str, contents); } } + +test "Screen: resize less cols to eliminate wide char" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 1, 0); + defer s.deinit(); + const str = "šŸ˜€"; + try s.testWriteString(str); + { + const contents = try s.dumpStringAlloc(alloc, .{ .screen = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + const list_cell = s.pages.getCell(.{ .screen = .{ .x = 0, .y = 0 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(Cell.Wide.wide, cell.wide); + try testing.expectEqual(@as(u21, 'šŸ˜€'), cell.content.codepoint); + } + + // Resize to 1 column can't fit a wide char. So it should be deleted. + try s.resize(1, 1); + { + const contents = try s.dumpStringAlloc(alloc, .{ .screen = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings("", contents); + } + { + const list_cell = s.pages.getCell(.{ .screen = .{ .x = 0, .y = 0 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(@as(u21, 0), cell.content.codepoint); + try testing.expectEqual(Cell.Wide.narrow, cell.wide); + } +} + +test "Screen: resize less cols to wrap wide char" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 3, 0); + defer s.deinit(); + const str = "xšŸ˜€"; + try s.testWriteString(str); + { + const contents = try s.dumpStringAlloc(alloc, .{ .screen = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + const list_cell = s.pages.getCell(.{ .screen = .{ .x = 1, .y = 0 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(Cell.Wide.wide, cell.wide); + try testing.expectEqual(@as(u21, 'šŸ˜€'), cell.content.codepoint); + } + { + const list_cell = s.pages.getCell(.{ .screen = .{ .x = 2, .y = 0 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(Cell.Wide.spacer_tail, cell.wide); + } + + try s.resize(2, 3); + { + const contents = try s.dumpStringAlloc(alloc, .{ .screen = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings("x\nšŸ˜€", contents); + } + { + const list_cell = s.pages.getCell(.{ .screen = .{ .x = 1, .y = 0 } }).?; + const cell = list_cell.cell; + try testing.expectEqual(Cell.Wide.spacer_head, cell.wide); + try testing.expect(list_cell.row.wrap); + } +} diff --git a/src/terminal/new/page.zig b/src/terminal/new/page.zig index 28c740212..045ad29af 100644 --- a/src/terminal/new/page.zig +++ b/src/terminal/new/page.zig @@ -774,9 +774,11 @@ pub const Cell = packed struct(u64) { /// Returns true if the cell has no text or styling. pub fn isEmpty(self: Cell) bool { return switch (self.content_tag) { + // Textual cells are empty if they have no text and are narrow. + // The "narrow" requirement is because wide spacers are meaningful. .codepoint, .codepoint_grapheme, - => !self.hasText(), + => !self.hasText() and self.wide == .narrow, .bg_color_palette, .bg_color_rgb,