From 3d72178ef4b359a14c910083c16a88df1597b73d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 11 Aug 2023 16:22:00 -0700 Subject: [PATCH 1/7] terminal: delete wide char if it wraps and we delete the row above --- src/terminal/Screen.zig | 48 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 9f75325bc..6a912cfa7 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2297,6 +2297,9 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { var cur_trimmed_row = trimmed_row; while (true) { for (cur_trimmed_row, 0..) |cell, old_x| { + // Set to true to write an empty cell + var clear_cell: bool = false; + // Soft wrap if we have to. if (x == self.cols) { row.setWrapped(true); @@ -2308,6 +2311,9 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { if (y >= self.rows) { try self.scroll(.{ .screen = 1 }); y -= 1; + + // Clear if our current cell is a wide spacer tail + clear_cell = cell.cell.attrs.wide_spacer_tail; } row = self.getRow(.{ .active = y }); @@ -2322,7 +2328,7 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { // Write the cell var new_cell = row.getCellPtr(x); - new_cell.* = cell.cell; + new_cell.* = if (clear_cell) .{} else cell.cell; x += 1; } @@ -2396,8 +2402,12 @@ fn trimRowForResizeLessCols(self: *Screen, old: *Screen, row: Row) []StorageCell if (!cell.empty()) { // If we are beyond our new width and this is just // an empty-character stylized cell, then we trim it. + // We also have to ignore wide spacers because they form + // a critical part of a wide character. if (i > self.cols) { - if (cell.char == 0 or cell.char == ' ') continue; + if ((cell.char == 0 or cell.char == ' ') and + !cell.attrs.wide_spacer_tail and + !cell.attrs.wide_spacer_head) continue; } break; @@ -5722,6 +5732,40 @@ test "Screen: resize more rows then shrink again" { } } +test "Screen: resize less cols to eliminate wide char" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 1, 2, 0); + defer s.deinit(); + const str = "😀"; + try s.testWriteString(str); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + const cell = s.getCell(.screen, 0, 0); + try testing.expectEqual(@as(u32, '😀'), cell.char); + try testing.expect(cell.attrs.wide); + } + + // Resize to 1 column can't fit a wide char. So it should be deleted. + try s.resize(1, 1); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings("", contents); + } + + const cell = s.getCell(.screen, 0, 0); + try testing.expectEqual(@as(u32, 0), cell.char); + try testing.expect(!cell.attrs.wide); + try testing.expect(!cell.attrs.wide_spacer_tail); + try testing.expect(!cell.attrs.wide_spacer_head); +} + test "Screen: jump zero" { const testing = std.testing; const alloc = testing.allocator; From a6af75aee42c2baa95aa62bf76d386de9840a769 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 12 Aug 2023 08:20:10 -0700 Subject: [PATCH 2/7] terminal: resize more cols with wide spacer head deletes the spacer --- src/terminal/Screen.zig | 49 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 6a912cfa7..7716e5bcf 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2119,8 +2119,14 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { // Mark the last element as not wrapped new_row.setWrapped(false); - // We maintain an x coord so that we can set cursors properly + // x is the offset where we start copying into new_row. Its also + // used for cursor tracking. var x: usize = old.cols; + + // Edge case: if the end of our old row is a wide spacer head, + // we want to overwrite it. + if (old_row.getCellPtr(x - 1).attrs.wide_spacer_head) x -= 1; + wrapping: while (iter.next()) |wrapped_row| { // Trim the row from the right so that we ignore all trailing // empty chars and don't wrap them. We only do this if the @@ -5766,6 +5772,47 @@ test "Screen: resize less cols to eliminate wide char" { try testing.expect(!cell.attrs.wide_spacer_head); } +test "Screen: resize more cols with wide spacer head" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 3, 0); + defer s.deinit(); + const str = " 😀"; + try s.testWriteString(str); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings(" \n😀", contents); + } + + // So this is the key point: we end up with a wide spacer head at + // the end of row 1, then the emoji, then a wide spacer tail on row 2. + // We should expect that if we resize to more cols, the wide spacer + // head is replaced with the emoji. + { + const cell = s.getCell(.screen, 0, 2); + try testing.expectEqual(@as(u32, ' '), cell.char); + try testing.expect(cell.attrs.wide_spacer_head); + try testing.expect(s.getCell(.screen, 1, 0).attrs.wide); + try testing.expect(s.getCell(.screen, 1, 1).attrs.wide_spacer_tail); + } + + try s.resize(2, 4); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + const cell = s.getCell(.screen, 0, 2); + try testing.expectEqual(@as(u32, '😀'), cell.char); + try testing.expect(!cell.attrs.wide_spacer_head); + try testing.expect(cell.attrs.wide); + try testing.expect(s.getCell(.screen, 0, 3).attrs.wide_spacer_tail); + } +} + test "Screen: jump zero" { const testing = std.testing; const alloc = testing.allocator; From 3d5eda62fe024d501f8e1b641740dc81f6845a5f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 12 Aug 2023 08:26:13 -0700 Subject: [PATCH 3/7] terminal: resize more cols with wide spacer head across multiple lines --- src/terminal/Screen.zig | 60 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 7716e5bcf..220a23c84 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2128,15 +2128,26 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { if (old_row.getCellPtr(x - 1).attrs.wide_spacer_head) x -= 1; wrapping: while (iter.next()) |wrapped_row| { - // Trim the row from the right so that we ignore all trailing - // empty chars and don't wrap them. We only do this if the - // row is NOT wrapped again because the whitespace would be - // meaningful. const wrapped_cells = trim: { var i: usize = old.cols; + + // Trim the row from the right so that we ignore all trailing + // empty chars and don't wrap them. We only do this if the + // row is NOT wrapped again because the whitespace would be + // meaningful. if (!wrapped_row.header().flags.wrap) { - while (i > 0) : (i -= 1) if (!wrapped_row.getCell(i - 1).empty()) break; + while (i > 0) : (i -= 1) { + if (!wrapped_row.getCell(i - 1).empty()) break; + } + } else { + // If we are wrapped, then similar to above "edge case" + // we want to overwrite the wide spacer head if we end + // in one. + if (wrapped_row.getCellPtr(i - 1).attrs.wide_spacer_head) { + i -= 1; + } } + break :trim wrapped_row.storage[1 .. i + 1]; }; @@ -5813,6 +5824,45 @@ test "Screen: resize more cols with wide spacer head" { } } +test "Screen: resize more cols with wide spacer head multiple lines" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 3, 0); + defer s.deinit(); + const str = "xxxyy😀"; + try s.testWriteString(str); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings("xxx\nyy\n😀", contents); + } + + // Similar to the "wide spacer head" test, but this time we'er going + // to increase our columns such that multiple rows are unwrapped. + { + const cell = s.getCell(.screen, 1, 2); + try testing.expectEqual(@as(u32, ' '), cell.char); + try testing.expect(cell.attrs.wide_spacer_head); + try testing.expect(s.getCell(.screen, 2, 0).attrs.wide); + try testing.expect(s.getCell(.screen, 2, 1).attrs.wide_spacer_tail); + } + + try s.resize(2, 8); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + const cell = s.getCell(.screen, 0, 5); + try testing.expect(!cell.attrs.wide_spacer_head); + try testing.expectEqual(@as(u32, '😀'), cell.char); + try testing.expect(cell.attrs.wide); + try testing.expect(s.getCell(.screen, 0, 6).attrs.wide_spacer_tail); + } +} + test "Screen: jump zero" { const testing = std.testing; const alloc = testing.allocator; From eb10e9642c4df21a6291ef0170fcebe352e212f8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 12 Aug 2023 09:49:26 -0700 Subject: [PATCH 4/7] terminal: introduce wide spacer head if reflowing wide char w/ more cols --- src/terminal/Screen.zig | 63 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 220a23c84..55c250aa7 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2160,7 +2160,26 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { const wrapped_cells_rem = wrapped_cells.len - wrapped_i; // We copy as much as we can into our new row - const copy_len = @min(new_row_rem, wrapped_cells_rem); + const copy_len = if (new_row_rem <= wrapped_cells_rem) copy_len: { + // We are going to end up filling our new row. We need + // to check if the end of the row is a wide char and + // if so, we need to insert a wide char header and wrap + // there. + var proposed: usize = new_row_rem; + + // If the end of our copy is wide, we copy one less and + // set the wide spacer header now since we're not going + // to write over it anyways. + if (wrapped_cells[wrapped_i + proposed - 1].cell.attrs.wide) { + proposed -= 1; + new_row.getCellPtr(x + proposed).* = .{ + .char = ' ', + .attrs = .{ .wide_spacer_head = true }, + }; + } + + break :copy_len proposed; + } else wrapped_cells_rem; // The row doesn't fit, meaning we have to soft-wrap the // new row but probably at a diff boundary. @@ -5863,6 +5882,48 @@ test "Screen: resize more cols with wide spacer head multiple lines" { } } +test "Screen: resize more cols requiring a wide spacer head" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + const str = "xx😀"; + try s.testWriteString(str); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings("xx\n😀", contents); + } + { + try testing.expect(s.getCell(.screen, 1, 0).attrs.wide); + try testing.expect(s.getCell(.screen, 1, 1).attrs.wide_spacer_tail); + } + + // This resizes to 3 columns, which isn't enough space for our wide + // char to enter row 1. But we need to mark the wide spacer head on the + // end of the first row since we're wrapping to the next row. + try s.resize(2, 3); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings("xx\n😀", contents); + } + { + const cell = s.getCell(.screen, 0, 2); + try testing.expectEqual(@as(u32, ' '), cell.char); + try testing.expect(cell.attrs.wide_spacer_head); + try testing.expect(s.getCell(.screen, 1, 0).attrs.wide); + try testing.expect(s.getCell(.screen, 1, 1).attrs.wide_spacer_tail); + } + { + const cell = s.getCell(.screen, 1, 0); + try testing.expectEqual(@as(u32, '😀'), cell.char); + try testing.expect(cell.attrs.wide); + try testing.expect(s.getCell(.screen, 1, 1).attrs.wide_spacer_tail); + } +} + test "Screen: jump zero" { const testing = std.testing; const alloc = testing.allocator; From 44dd51a5b9d4c72465a2937d85d29cbc4d90e050 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 12 Aug 2023 10:07:13 -0700 Subject: [PATCH 5/7] terminal: less col resizing with wide char needs to insert head spacer --- src/terminal/Screen.zig | 44 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 55c250aa7..fdd6ec839 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2336,6 +2336,15 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { // Set to true to write an empty cell var clear_cell: bool = false; + // We need to wrap wide chars with a spacer head. + if (cell.cell.attrs.wide and x == self.cols - 1) { + row.getCellPtr(x).* = .{ + .char = ' ', + .attrs = .{ .wide_spacer_head = true }, + }; + x += 1; + } + // Soft wrap if we have to. if (x == self.cols) { row.setWrapped(true); @@ -5802,6 +5811,41 @@ test "Screen: resize less cols to eliminate wide char" { try testing.expect(!cell.attrs.wide_spacer_head); } +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); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + const cell = s.getCell(.screen, 0, 1); + try testing.expectEqual(@as(u32, '😀'), cell.char); + try testing.expect(cell.attrs.wide); + try testing.expect(s.getCell(.screen, 0, 2).attrs.wide_spacer_tail); + } + + try s.resize(3, 2); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings("x\n😀", contents); + } + { + const cell = s.getCell(.screen, 0, 1); + try testing.expectEqual(@as(u32, ' '), cell.char); + try testing.expect(!cell.attrs.wide); + try testing.expect(!cell.attrs.wide_spacer_tail); + try testing.expect(cell.attrs.wide_spacer_head); + } +} + test "Screen: resize more cols with wide spacer head" { const testing = std.testing; const alloc = testing.allocator; From 71c8d5fc8e68594a910bff87bc8bc6d9e3bd8bc7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 12 Aug 2023 10:23:06 -0700 Subject: [PATCH 6/7] terminal: resizing to 1 col wide with wide chars --- src/terminal/Screen.zig | 61 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index fdd6ec839..a5ccb862b 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2332,9 +2332,19 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { var cur_old_row_wrapped = old_row_wrapped; var cur_trimmed_row = trimmed_row; while (true) { - for (cur_trimmed_row, 0..) |cell, old_x| { - // Set to true to write an empty cell - var clear_cell: bool = false; + for (cur_trimmed_row, 0..) |old_cell, old_x| { + var cell: StorageCell = old_cell; + + // This is a really wild edge case if we're resizing down + // to 1 column. In reality this is pretty broken for end + // users so downstream should prevent this. + if (self.cols == 1 and + (cell.cell.attrs.wide or + cell.cell.attrs.wide_spacer_head or + cell.cell.attrs.wide_spacer_tail)) + { + cell = .{ .cell = .{ .char = ' ' } }; + } // We need to wrap wide chars with a spacer head. if (cell.cell.attrs.wide and x == self.cols - 1) { @@ -2358,7 +2368,9 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { y -= 1; // Clear if our current cell is a wide spacer tail - clear_cell = cell.cell.attrs.wide_spacer_tail; + if (cell.cell.attrs.wide_spacer_tail) { + cell = .{ .cell = .{} }; + } } row = self.getRow(.{ .active = y }); @@ -2373,7 +2385,7 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { // Write the cell var new_cell = row.getCellPtr(x); - new_cell.* = if (clear_cell) .{} else cell.cell; + new_cell.* = cell.cell; x += 1; } @@ -5801,11 +5813,11 @@ test "Screen: resize less cols to eliminate wide char" { { var contents = try s.testString(alloc, .screen); defer alloc.free(contents); - try testing.expectEqualStrings("", contents); + try testing.expectEqualStrings(" ", contents); } const cell = s.getCell(.screen, 0, 0); - try testing.expectEqual(@as(u32, 0), cell.char); + try testing.expectEqual(@as(u32, ' '), cell.char); try testing.expect(!cell.attrs.wide); try testing.expect(!cell.attrs.wide_spacer_tail); try testing.expect(!cell.attrs.wide_spacer_head); @@ -5846,6 +5858,41 @@ test "Screen: resize less cols to wrap wide char" { } } +test "Screen: resize less cols to eliminate wide char with row space" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + const str = "😀"; + try s.testWriteString(str); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + const cell = s.getCell(.screen, 0, 0); + try testing.expectEqual(@as(u32, '😀'), cell.char); + try testing.expect(cell.attrs.wide); + try testing.expect(s.getCell(.screen, 0, 1).attrs.wide_spacer_tail); + } + + try s.resize(2, 1); + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings(" \n ", contents); + } + { + const cell = s.getCell(.screen, 0, 0); + try testing.expectEqual(@as(u32, ' '), cell.char); + try testing.expect(!cell.attrs.wide); + try testing.expect(!cell.attrs.wide_spacer_tail); + try testing.expect(!cell.attrs.wide_spacer_head); + } +} + test "Screen: resize more cols with wide spacer head" { const testing = std.testing; const alloc = testing.allocator; From 9b5fd4b2ee70f76360057ac06b8e90aea25bcf53 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 12 Aug 2023 10:27:18 -0700 Subject: [PATCH 7/7] terminal: print wide char with 1-col width --- src/terminal/Terminal.zig | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index f1bd0f517..15fcc23da 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -703,7 +703,7 @@ pub fn print(self: *Terminal, c: u21) !void { // using two cells: the first is flagged "wide" and has the // wide char. The second is guaranteed to be a spacer if // we're not at the end of the line. - 2 => { + 2 => if (self.cols > 1) { // If we don't have space for the wide char, we need // to insert spacers and wrap. Then we just print the wide // char as normal. @@ -720,6 +720,10 @@ pub fn print(self: *Terminal, c: u21) !void { self.screen.cursor.x += 1; const spacer = self.printCell(' '); spacer.attrs.wide_spacer_tail = true; + } else { + // This is pretty broken, terminals should never be only 1-wide. + // We sould prevent this downstream. + _ = self.printCell(' '); }, else => unreachable, @@ -2451,3 +2455,26 @@ test "Terminal: cursorIsAtPrompt alternate screen" { t.markSemanticPrompt(.prompt); try testing.expect(!t.cursorIsAtPrompt()); } + +test "Terminal: print wide char with 1-column width" { + const alloc = testing.allocator; + var t = try init(alloc, 1, 2); + defer t.deinit(alloc); + + try t.print('😀'); // 0x1F600 +} + +// https://github.com/mitchellh/ghostty/issues/272 +// This is also tested in depth in screen resize tests but I want to keep +// this test around to ensure we don't regress at multiple layers. +test "Terminal: resize less cols with wide char then print" { + const alloc = testing.allocator; + var t = try init(alloc, 3, 3); + defer t.deinit(alloc); + + try t.print('x'); + try t.print('😀'); // 0x1F600 + try t.resize(alloc, 2, 3); + t.setCursorPos(1, 2); + try t.print('😀'); // 0x1F600 +}