From 1970a849602d342b2d54c6a85488682f59149a3a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 28 Feb 2023 22:28:27 -0800 Subject: [PATCH 01/14] screen: when resizing and trimming scrollback, have to offset cursor Y When the scrollback is trimmed off the top, the y stops moving. This would cause an assertion failure because y could be greater than the row count! The test case tests this. --- src/terminal/Screen.zig | 46 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index c9f9b8c7b..ef4a8c7e2 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1731,6 +1731,13 @@ pub fn resizeWithoutReflow(self: *Screen, rows: usize, cols: usize) !void { while (row_it.next()) |old_row| { // If we're past the end, scroll if (y >= self.rows) { + // If we're shrinking rows then its possible we'll trim scrollback + // and we have to account for how much we actually trimmed and + // reflect that in the cursor. + if (self.storage.len() >= self.maxCapacity()) { + old.cursor.y -|= 1; + } + y -= 1; try self.scroll(.{ .delta = 1 }); } @@ -4165,6 +4172,45 @@ test "Screen: resize less rows with populated scrollback" { } } +test "Screen: resize less rows with full scrollback" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 5, 3); + defer s.deinit(); + const str = "00000\n1ABCD\n2EFGH\n3IJKL\n4ABCD\n5EFGH"; + try s.testWriteString(str); + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + const expected = "3IJKL\n4ABCD\n5EFGH"; + try testing.expectEqualStrings(expected, contents); + } + + const cursor = s.cursor; + try testing.expectEqual(Cursor{ .x = 4, .y = 2 }, cursor); + + // Resize + try s.resize(2, 5); + + // Cursor should stay in the same relative place (bottom of the + // screen, same character). + try testing.expectEqual(Cursor{ .x = 4, .y = 1 }, s.cursor); + + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + const expected = "1ABCD\n2EFGH\n3IJKL\n4ABCD\n5EFGH"; + try testing.expectEqualStrings(expected, contents); + } + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + const expected = "4ABCD\n5EFGH"; + try testing.expectEqualStrings(expected, contents); + } +} + test "Screen: resize less cols no reflow" { const testing = std.testing; const alloc = testing.allocator; From 5f9ab91466c402c6de6cbe47a9637e1d7f4d8f16 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 09:08:08 -0800 Subject: [PATCH 02/14] screen: fix issue with resizing w/ more cols, reflow, and scrollback --- src/terminal/Screen.zig | 50 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index ef4a8c7e2..6c9530e3a 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -819,6 +819,7 @@ pub fn clone(self: *Screen, alloc: Allocator, top: RowIndex, bottom: RowIndex) ! // copy. const real_y = @min(bot_y, max_y); const real_height = (real_y - top_y) + 1; + //log.warn("bot={} max={} top={} real={}", .{ bot_y, max_y, top_y, real_y }); // Init a new screen that exactly fits the height. The height is the // non-real value because we still want the requested height by the @@ -1899,7 +1900,9 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { // If we were able to copy the entire row then // we shortened the screen by one. We need to reflect // this in our viewport. - if (wrapped_i == 0 and old.viewport > 0) old.viewport -= 1; + if (wrapped_i == 0 and old.viewport > 0) { + old.viewport -= 1; + } y += 1; break :wrapping; @@ -1926,10 +1929,14 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { new_row = self.getRow(.{ .active = y }); } } - - self.viewport = old.viewport; } + // During the resize, we just keep the viewport at the bottom. But + // we want to restore the viewport to what the user had when they + // started the resize. old.viewport is maintained for removed lines + // in the for loop above. + self.viewport = old.viewport; + // If we have a new cursor, we need to convert that to a viewport // point and set it up. if (new_cursor) |pos| { @@ -4053,6 +4060,43 @@ test "Screen: resize more cols with populated scrollback" { } } +test "Screen: resize more cols with reflow" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 2, 5); + defer s.deinit(); + const str = "1ABC\n2DEF\n3ABC\n4DEF"; + try s.testWriteString(str); + + // Let's put our cursor on row 2, where the soft wrap is + s.cursor.x = 0; + s.cursor.y = 2; + try testing.expectEqual(@as(u32, 'E'), s.getCell(.active, s.cursor.y, s.cursor.x).char); + + // Verify we soft wrapped + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + const expected = "BC\n4D\nEF"; + try testing.expectEqualStrings(expected, contents); + } + + // Resize and verify we undid the soft wrap because we have space now + try s.resize(3, 7); + + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + const expected = "1ABC\n2DEF\n3ABC\n4DEF"; + try testing.expectEqualStrings(expected, contents); + } + + // Our cursor should've moved + try testing.expectEqual(@as(usize, 2), s.cursor.x); + try testing.expectEqual(@as(usize, 2), s.cursor.y); +} + test "Screen: resize less rows no scrollback" { const testing = std.testing; const alloc = testing.allocator; From d4057522ee33dad62b2f3a384186ab0992a182a6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 09:18:26 -0800 Subject: [PATCH 03/14] screen: resize more rows preserves soft wrapped flag --- src/terminal/Screen.zig | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 6c9530e3a..6d3170bca 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -402,6 +402,9 @@ pub const Row = struct { // If we have graphemes, clear first to unset them. if (self.storage[0].header.flags.grapheme) self.clear(.{}); + // Copy the flags + self.storage[0].header.flags = src.storage[0].header.flags; + // Always mark the row as dirty for this. self.storage[0].header.flags.dirty = true; @@ -3735,6 +3738,45 @@ test "Screen: resize (no reflow) grapheme copy" { } } +test "Screen: resize (no reflow) more rows with soft wrapping" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 2, 3); + defer s.deinit(); + const str = "1A2B\n3C4E\n5F6G"; + try s.testWriteString(str); + + // Every second row should be wrapped + { + var y: usize = 0; + while (y < 6) : (y += 1) { + const row = s.getRow(.{ .screen = y }); + const wrapped = (y % 2 == 0); + try testing.expectEqual(wrapped, row.header().flags.wrap); + } + } + + // Resize + try s.resizeWithoutReflow(10, 2); + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + const expected = "1A\n2B\n3C\n4E\n5F\n6G"; + try testing.expectEqualStrings(expected, contents); + } + + // Every second row should be wrapped + { + var y: usize = 0; + while (y < 6) : (y += 1) { + const row = s.getRow(.{ .screen = y }); + const wrapped = (y % 2 == 0); + try testing.expectEqual(wrapped, row.header().flags.wrap); + } + } +} + test "Screen: resize more rows no scrollback" { const testing = std.testing; const alloc = testing.allocator; From b4d8419feb8b0fd9d094fe14f0583d93fb67151e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 17:04:45 -0800 Subject: [PATCH 04/14] screen: trim trailing no-character cells when rows is changing This matches Terminal.app, and makes it so the `ESC [ J` doesn't generate scrollback on rows change. --- src/apprt/glfw.zig | 7 ++ src/terminal/Screen.zig | 164 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 168 insertions(+), 3 deletions(-) diff --git a/src/apprt/glfw.zig b/src/apprt/glfw.zig index 31c117c36..892c1ed05 100644 --- a/src/apprt/glfw.zig +++ b/src/apprt/glfw.zig @@ -59,6 +59,13 @@ pub const App = struct { while (true) { // Wait for any events from the app event loop. wakeup will post // an empty event so that this will return. + // + // Warning: a known issue on macOS is that this will block while + // a resize event is actively happening, which will prevent the + // app tick from happening. I don't know know a way around this + // but its not a big deal since we don't use glfw for the official + // mac app, but noting it in case anyone builds for macos using + // glfw. glfw.waitEvents(); // Tick the terminal app diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 6d3170bca..7707863fc 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -289,6 +289,17 @@ pub const Row = struct { return self.storage.len - 1; } + /// Returns true if the row only has empty characters. This ignores + /// styling (i.e. styling does not count as non-empty). + pub fn isEmpty(self: Row) bool { + const len = self.storage.len; + for (self.storage[1..len]) |cell| { + if (cell.cell.char != 0) return false; + } + + return true; + } + /// Clear the row, making all cells empty. pub fn clear(self: Row, pen: Cell) void { var empty_pen = pen; @@ -1698,6 +1709,36 @@ pub fn resizeWithoutReflow(self: *Screen, rows: usize, cols: usize) !void { // If we're resizing to the same size, do nothing. if (self.cols == cols and self.rows == rows) return; + // The number of no-character lines after our cursor. This is used + // to trim those lines on a resize first without generating history. + // This is only done if we don't have history yet. + // + // This matches macOS Terminal.app behavior. I chose to match that + // behavior because it seemed fine in an ocean of differing behavior + // between terminal apps. I'm completely open to changing it as long + // as resize behavior isn't regressed in a user-hostile way. + const trailing_blank_lines = blank: { + // If there is history, blank line counting is disabled and + // we generate scrollback. Why? Terminal.app does it, seems... fine. + if (self.history > 0) break :blank 0; + + // Start one line below our cursor and continue to the last line + // of the screen or however many rows we have written. + const start = self.cursor.y + 1; + const end = @min(self.rowsWritten(), self.rows); + if (start >= end) break :blank 0; + + var blank: usize = 0; + for (0..(end - start)) |i| { + const y = end - i - 1; + const row = self.getRow(.{ .active = y }); + if (!row.isEmpty()) break; + blank += 1; + } + + break :blank blank; + }; + // Make a copy so we can access the old indexes. var old = self.*; errdefer self.* = old; @@ -1706,10 +1747,14 @@ pub fn resizeWithoutReflow(self: *Screen, rows: usize, cols: usize) !void { self.rows = rows; self.cols = cols; + // The end of the screen is the rows we wrote minus any blank lines + // we're trimming. + const end_of_screen_y = old.rowsWritten() - trailing_blank_lines; + // Calculate our buffer size. This is going to be either the old data // with scrollback or the max capacity of our new size. We prefer the old // length so we can save all the data (ignoring col truncation). - const old_len = @max(old.rowsWritten(), rows) * (cols + 1); + const old_len = @max(end_of_screen_y, rows) * (cols + 1); const new_max_capacity = self.maxCapacity(); const buf_size = @min(old_len, new_max_capacity); @@ -1731,8 +1776,9 @@ pub fn resizeWithoutReflow(self: *Screen, rows: usize, cols: usize) !void { // Rewrite all our rows var y: usize = 0; - var row_it = old.rowIterator(.screen); - while (row_it.next()) |old_row| { + for (0..end_of_screen_y) |it_y| { + const old_row = old.getRow(.{ .screen = it_y }); + // If we're past the end, scroll if (y >= self.rows) { // If we're shrinking rows then its possible we'll trim scrollback @@ -2249,6 +2295,46 @@ pub fn testString(self: *Screen, alloc: Allocator, tag: RowIndexTag) ![]const u8 return try alloc.realloc(buf, str.len); } +test "Row: isEmpty with no data" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 5, 5, 0); + defer s.deinit(); + + const row = s.getRow(.{ .active = 0 }); + try testing.expect(row.isEmpty()); +} + +test "Row: isEmpty with a character at the end" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 5, 5, 0); + defer s.deinit(); + + const row = s.getRow(.{ .active = 0 }); + const cell = row.getCellPtr(4); + cell.*.char = 'A'; + try testing.expect(!row.isEmpty()); +} + +test "Row: isEmpty with only styled cells" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 5, 5, 0); + defer s.deinit(); + + const row = s.getRow(.{ .active = 0 }); + for (0..s.cols) |x| { + const cell = row.getCellPtr(x); + cell.*.bg = .{ .r = 0xAA, .g = 0xBB, .b = 0xCC }; + cell.*.attrs.has_bg = true; + } + try testing.expect(row.isEmpty()); +} + test "Row: clear with graphemes" { const testing = std.testing; const alloc = testing.allocator; @@ -3610,6 +3696,78 @@ test "Screen: resize (no reflow) less rows" { } } +test "Screen: resize (no reflow) less rows trims blank lines" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 5, 0); + defer s.deinit(); + const str = "1ABCD"; + try s.testWriteString(str); + + // Write only a background color into the remaining rows + for (1..s.rows) |y| { + const row = s.getRow(.{ .active = y }); + for (0..s.cols) |x| { + const cell = row.getCellPtr(x); + cell.*.bg = .{ .r = 0xFF, .g = 0, .b = 0 }; + cell.*.attrs.has_bg = true; + } + } + + // Make sure our cursor is at the end of the first line + s.cursor.x = 4; + s.cursor.y = 0; + const cursor = s.cursor; + + try s.resizeWithoutReflow(2, 5); + + // Cursor should not move + try testing.expectEqual(cursor, s.cursor); + + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + try testing.expectEqualStrings("1ABCD", contents); + } +} + +test "Screen: resize (no reflow) more rows trims blank lines" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 5, 0); + defer s.deinit(); + const str = "1ABCD"; + try s.testWriteString(str); + + // Write only a background color into the remaining rows + for (1..s.rows) |y| { + const row = s.getRow(.{ .active = y }); + for (0..s.cols) |x| { + const cell = row.getCellPtr(x); + cell.*.bg = .{ .r = 0xFF, .g = 0, .b = 0 }; + cell.*.attrs.has_bg = true; + } + } + + // Make sure our cursor is at the end of the first line + s.cursor.x = 4; + s.cursor.y = 0; + const cursor = s.cursor; + + try s.resizeWithoutReflow(7, 5); + + // Cursor should not move + try testing.expectEqual(cursor, s.cursor); + + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + try testing.expectEqualStrings("1ABCD", contents); + } +} + test "Screen: resize (no reflow) more cols" { const testing = std.testing; const alloc = testing.allocator; From 89138cf7e3f44524b38ddbab88d3d30f2a0dffce Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 17:09:57 -0800 Subject: [PATCH 05/14] screen: don't trim blank lines if rows aren't changing --- src/terminal/Screen.zig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 7707863fc..7165749f3 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1718,6 +1718,10 @@ pub fn resizeWithoutReflow(self: *Screen, rows: usize, cols: usize) !void { // between terminal apps. I'm completely open to changing it as long // as resize behavior isn't regressed in a user-hostile way. const trailing_blank_lines = blank: { + // If we aren't changing row length, then don't bother calculating + // because we aren't going to trim. + if (self.rows == rows) break :blank 0; + // If there is history, blank line counting is disabled and // we generate scrollback. Why? Terminal.app does it, seems... fine. if (self.history > 0) break :blank 0; From 31ac3ec7ba26eb62d7841471b598bac10e63a2c7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 17:24:24 -0800 Subject: [PATCH 06/14] screen: when expanding cols, broadcast empty styled cells --- src/terminal/Screen.zig | 66 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 7165749f3..757f84fd0 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1894,8 +1894,19 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { new_cursor = .{ .y = self.rowsWritten() - 1, .x = cursor_pos.x }; } - // If no reflow, just keep going + // If no reflow, just keep going. if (!old_row.header().flags.wrap) { + // If we have no reflow, we attempt to extend any stylized + // cells at the end of the line if there is one. + const len = old_row.lenCells(); + const end = new_row.getCell(len - 1); + if ((end.char == 0 or end.char == ' ') and !end.empty()) { + for (len..self.cols) |x| { + const cell = new_row.getCellPtr(x); + cell.* = end; + } + } + y += 1; continue; } @@ -4050,6 +4061,59 @@ test "Screen: resize more cols no reflow" { } } +test "Screen: resize more cols trailing background colors" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 5, 0); + defer s.deinit(); + const str = "1AB"; + try s.testWriteString(str); + const cursor = s.cursor; + + // Color our cells red + const pen: Cell = .{ .bg = .{ .r = 0xFF }, .attrs = .{ .has_bg = true } }; + for (s.cursor.x..s.cols) |x| { + const row = s.getRow(.{ .active = s.cursor.y }); + const cell = row.getCellPtr(x); + cell.* = pen; + } + for ((s.cursor.y + 1)..s.rows) |y| { + const row = s.getRow(.{ .active = y }); + row.fill(pen); + } + + try s.resize(3, 10); + + // Cursor should not move + try testing.expectEqual(cursor, s.cursor); + + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + + // Verify all our trailing cells have the color + for (s.cursor.x..s.cols) |x| { + const row = s.getRow(.{ .active = s.cursor.y }); + const cell = row.getCellPtr(x); + try testing.expectEqual(pen, cell.*); + } + for ((s.cursor.y + 1)..s.rows) |y| { + const row = s.getRow(.{ .active = y }); + for (0..s.cols) |x| { + const cell = row.getCellPtr(x); + try testing.expectEqual(pen, cell.*); + } + } +} + test "Screen: resize more cols grapheme map" { const testing = std.testing; const alloc = testing.allocator; From 9a4a138da09751f121065829a37bbc81b50707d9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 17:34:17 -0800 Subject: [PATCH 07/14] screen: don't wrap empty-char stylized cells on shrinking cols --- src/terminal/Screen.zig | 74 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 757f84fd0..ff24c15cf 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2058,7 +2058,26 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { // empty chars and don't wrap them. const trimmed_row = trim: { var i: usize = old.cols; - while (i > 0) : (i -= 1) if (!old_row.getCell(i - 1).empty()) break; + + // We only trim if this isn't a wrapped line. If its a wrapped + // line we need to keep all the empty cells because they are + // meaningful whitespace before our wrap. + if (!old_row.header().flags.wrap) { + while (i > 0) : (i -= 1) { + const cell = old_row.getCell(i - 1); + + if (!cell.empty()) { + // If we are beyond our new width and this is just + // an empty-character stylized cell, then we trim it. + if (i > self.cols) { + if (cell.char == 0 or cell.char == ' ') continue; + } + + break; + } + } + } + break :trim old_row.storage[1 .. i + 1]; }; @@ -4551,6 +4570,59 @@ test "Screen: resize less cols no reflow" { } } +test "Screen: resize less cols trailing background colors" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 10, 0); + defer s.deinit(); + const str = "1AB"; + try s.testWriteString(str); + const cursor = s.cursor; + + // Color our cells red + const pen: Cell = .{ .bg = .{ .r = 0xFF }, .attrs = .{ .has_bg = true } }; + for (s.cursor.x..s.cols) |x| { + const row = s.getRow(.{ .active = s.cursor.y }); + const cell = row.getCellPtr(x); + cell.* = pen; + } + for ((s.cursor.y + 1)..s.rows) |y| { + const row = s.getRow(.{ .active = y }); + row.fill(pen); + } + + try s.resize(3, 5); + + // Cursor should not move + try testing.expectEqual(cursor, s.cursor); + + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + + // Verify all our trailing cells have the color + for (s.cursor.x..s.cols) |x| { + const row = s.getRow(.{ .active = s.cursor.y }); + const cell = row.getCellPtr(x); + try testing.expectEqual(pen, cell.*); + } + for ((s.cursor.y + 1)..s.rows) |y| { + const row = s.getRow(.{ .active = y }); + for (0..s.cols) |x| { + const cell = row.getCellPtr(x); + try testing.expectEqual(pen, cell.*); + } + } +} + test "Screen: resize less cols with graphemes" { const testing = std.testing; const alloc = testing.allocator; From 74f0e38b57083f3ae2329017c322892e7a8e41e5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 17:42:46 -0800 Subject: [PATCH 08/14] screen: only trim if we're not wrapping on col growing --- src/terminal/Screen.zig | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index ff24c15cf..564439e45 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1922,10 +1922,14 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { var x: usize = old.cols; 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. + // 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; - while (i > 0) : (i -= 1) if (!wrapped_row.getCell(i - 1).empty()) break; + if (!wrapped_row.header().flags.wrap) { + while (i > 0) : (i -= 1) if (!wrapped_row.getCell(i - 1).empty()) break; + } break :trim wrapped_row.storage[1 .. i + 1]; }; From 3b586c39c546f419c824fcc7e689499e45f6fc26 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 19:45:39 -0800 Subject: [PATCH 09/14] screen: grow cols before rows to handle reflow (tested) --- src/terminal/Screen.zig | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 564439e45..7bdfaa46b 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1838,9 +1838,6 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { return; } - // We grow rows first so we can make space for more reflow - if (rows > self.rows) try self.resizeWithoutReflow(rows, cols); - // If our columns increased, we alloc space for the new column width // and go through each row and reflow if necessary. if (cols > self.cols) { @@ -2014,6 +2011,10 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { } } + // We grow rows after cols so that we can do our unwrapping/reflow + // before we do a no-reflow grow. + if (rows > self.rows) try self.resizeWithoutReflow(rows, cols); + // If our rows got smaller, we trim the scrollback. We do this after // handling cols growing so that we can save as many lines as we can. // We do it before cols shrinking so we can save compute on that operation. @@ -4058,6 +4059,39 @@ test "Screen: resize more rows with populated scrollback" { } } +test "Screen: resize more rows and cols with wrapping" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 4, 2, 0); + defer s.deinit(); + const str = "1A2B\n3C4D"; + try s.testWriteString(str); + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + const expected = "1A\n2B\n3C\n4D"; + try testing.expectEqualStrings(expected, contents); + } + + try s.resize(10, 5); + + // Cursor should move due to wrapping + try testing.expectEqual(@as(usize, 3), s.cursor.x); + try testing.expectEqual(@as(usize, 1), s.cursor.y); + + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } +} + test "Screen: resize more cols no reflow" { const testing = std.testing; const alloc = testing.allocator; From 979dc5a43990f40ae8f0110aeee56f4f212f52a8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 20:59:35 -0800 Subject: [PATCH 10/14] screen: redo resizing to less columns This is more performant (prefers fast copies if no wrapping) and keeps track of the cursor more accurately. --- src/terminal/Screen.zig | 235 +++++++++++++++++++++++++++------------- 1 file changed, 162 insertions(+), 73 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 7bdfaa46b..fe9acbcd7 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2056,88 +2056,87 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { // Iterate over the screen since we need to check for reflow. var iter = old.rowIterator(.screen); - var x: usize = 0; var y: usize = 0; while (iter.next()) |old_row| { - // Trim the row from the right so that we ignore all trailing - // empty chars and don't wrap them. - const trimmed_row = trim: { - var i: usize = old.cols; + const old_row_wrapped = old_row.header().flags.wrap; + const trimmed_row = self.trimRowForResizeLessCols(&old, old_row); - // We only trim if this isn't a wrapped line. If its a wrapped - // line we need to keep all the empty cells because they are - // meaningful whitespace before our wrap. - if (!old_row.header().flags.wrap) { - while (i > 0) : (i -= 1) { - const cell = old_row.getCell(i - 1); + // If our y is more than our rows, we need to scroll + if (y >= self.rows) { + try self.scroll(.{ .delta = 1 }); + y -= 1; + } - if (!cell.empty()) { - // If we are beyond our new width and this is just - // an empty-character stylized cell, then we trim it. - if (i > self.cols) { - if (cell.char == 0 or cell.char == ' ') continue; - } - - break; - } - } - } - - break :trim old_row.storage[1 .. i + 1]; - }; - - // Copy all the cells into our row. - for (trimmed_row, 0..) |cell, i| { - // Soft wrap if we have to - if (x == self.cols) { - var row = self.getRow(.{ .active = y }); - row.setWrapped(true); - x = 0; - y += 1; - } - - // If our y is more than our rows, we need to scroll - if (y >= self.rows) { - try self.scroll(.{ .delta = 1 }); - y = self.rows - 1; - x = 0; - } - - // If our cursor is on this point, we need to move it. - if (cursor_pos.y == iter.value - 1 and - cursor_pos.x == i) - { + // Fast path: our old row is not wrapped AND our old row fits + // into our new smaller size. In this case, we just do a fast + // copy and move on. + if (!old_row_wrapped and trimmed_row.len <= self.cols) { + // If our cursor is on this line, then set the new cursor. + if (cursor_pos.y == iter.value - 1) { assert(new_cursor == null); - new_cursor = .{ .x = x, .y = self.viewport + y }; + new_cursor = .{ .x = cursor_pos.x, .y = self.history + y }; } - // Copy the old cell, unset the old wrap state - // log.warn("y={} x={} rows={}", .{ y, x, self.rows }); - var new_cell = self.getCellPtr(.active, y, x); - new_cell.* = cell.cell; + const row = self.getRow(.{ .active = y }); + fastmem.copy( + StorageCell, + row.storage[1..], + trimmed_row, + ); - // Next - x += 1; - } - - // If our cursor is on this line but not in a content area, - // then we just set it to be at the end. - if (cursor_pos.y == iter.value - 1 and - cursor_pos.x >= trimmed_row.len) - { - assert(new_cursor == null); - new_cursor = .{ - .x = @min(cursor_pos.x, self.cols - 1), - .y = self.viewport + y, - }; - } - - // If we aren't wrapping, then move to the next row - if (trimmed_row.len == 0 or - !old_row.header().flags.wrap) - { y += 1; - x = 0; + continue; + } + + // Slow path: the row is wrapped or doesn't fit so we have to + // wrap ourselves. In this case, we basically just "print and wrap" + var row = self.getRow(.{ .active = y }); + var x: usize = 0; + var cur_old_row = old_row; + var cur_old_row_wrapped = old_row_wrapped; + var cur_trimmed_row = trimmed_row; + while (true) { + for (cur_trimmed_row, 0..) |cell, old_x| { + // Soft wrap if we have to. + if (x == self.cols) { + row.setWrapped(true); + x = 0; + y += 1; + + // Wrapping can cause us to overflow our visible area. + // If so, scroll. + if (y >= self.rows) { + try self.scroll(.{ .delta = 1 }); + y -= 1; + } + + row = self.getRow(.{ .active = y }); + } + + // If our cursor is on this char, then set the new cursor. + if (cursor_pos.y == iter.value - 1 and cursor_pos.x == old_x) { + assert(new_cursor == null); + new_cursor = .{ .x = x, .y = y }; + } + + // Write the cell + var new_cell = row.getCellPtr(x); + new_cell.* = cell.cell; + x += 1; + } + + // If we're done wrapping, we move on. + if (!cur_old_row_wrapped) { + y += 1; + break; + } + + // If the old row is wrapped we continue with the loop with + // the next row. + cur_old_row = iter.next() orelse + @panic("if the current row is wrapped, must have a follow-up row"); + cur_old_row_wrapped = cur_old_row.header().flags.wrap; + cur_trimmed_row = self.trimRowForResizeLessCols(&old, cur_old_row); } } @@ -2157,6 +2156,35 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { } } +/// When resizing to less columns, this trims the row from the right +/// so we don't unnecessarily wrap. This will freely throw away trailing +/// colored but empty (character) cells. This matches Terminal.app behavior, +/// which isn't strictly correct but seems nice. +fn trimRowForResizeLessCols(self: *Screen, old: *Screen, row: Row) []StorageCell { + assert(old.cols > self.cols); + + // We only trim if this isn't a wrapped line. If its a wrapped + // line we need to keep all the empty cells because they are + // meaningful whitespace before our wrap. + if (row.header().flags.wrap) return row.storage[1 .. old.cols + 1]; + + var i: usize = old.cols; + while (i > 0) : (i -= 1) { + const cell = row.getCell(i - 1); + if (!cell.empty()) { + // If we are beyond our new width and this is just + // an empty-character stylized cell, then we trim it. + if (i > self.cols) { + if (cell.char == 0 or cell.char == ' ') continue; + } + + break; + } + } + + return row.storage[1 .. i + 1]; +} + /// Writes a basic string into the screen for testing. Newlines (\n) separate /// each row. If a line is longer than the available columns, soft-wrapping /// will occur. This will automatically handle basic wide chars. @@ -4782,6 +4810,67 @@ test "Screen: resize less cols with reflow with trimmed rows and scrollback" { } } +test "Screen: resize less cols with reflow previously wrapped" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 5, 0); + defer s.deinit(); + const str = "3IJKL4ABCD5EFGH"; + try s.testWriteString(str); + + // Check + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + const expected = "3IJKL\n4ABCD\n5EFGH"; + try testing.expectEqualStrings(expected, contents); + } + + try s.resize(3, 3); + + // { + // var contents = try s.testString(alloc, .viewport); + // defer alloc.free(contents); + // const expected = "CD\n5EF\nGH"; + // try testing.expectEqualStrings(expected, contents); + // } + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + const expected = "ABC\nD5E\nFGH"; + try testing.expectEqualStrings(expected, contents); + } +} + +test "Screen: resize less cols with reflow and scrollback" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 5, 5); + defer s.deinit(); + const str = "1A\n2B\n3C\n4D\n5E"; + try s.testWriteString(str); + + // Put our cursor on the end + s.cursor.x = 1; + s.cursor.y = s.rows - 1; + try testing.expectEqual(@as(u32, 'E'), s.getCell(.active, s.cursor.y, s.cursor.x).char); + + try s.resize(3, 3); + + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + const expected = "3C\n4D\n5E"; + try testing.expectEqualStrings(expected, contents); + } + + // Cursor should be on the last line + try testing.expectEqual(@as(usize, 1), s.cursor.x); + try testing.expectEqual(@as(usize, 2), s.cursor.y); +} + // This seems like it should work fine but for some reason in practice // in the initial implementation I found this bug! This is a regression // test for that. From 56cb1dd55b6437638fd864427bc39da36056b30c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 21:06:33 -0800 Subject: [PATCH 11/14] screen: correct cursor position with scrollback and less cols --- src/terminal/Screen.zig | 45 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index fe9acbcd7..72090bf8c 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2116,7 +2116,7 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { // If our cursor is on this char, then set the new cursor. if (cursor_pos.y == iter.value - 1 and cursor_pos.x == old_x) { assert(new_cursor == null); - new_cursor = .{ .x = x, .y = y }; + new_cursor = .{ .x = x, .y = self.history + y }; } // Write the cell @@ -4871,6 +4871,49 @@ test "Screen: resize less cols with reflow and scrollback" { try testing.expectEqual(@as(usize, 2), s.cursor.y); } +test "Screen: resize less cols with reflow previously wrapped and scrollback" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 3, 5, 2); + defer s.deinit(); + const str = "1ABCD2EFGH3IJKL4ABCD5EFGH"; + try s.testWriteString(str); + + // Check + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + const expected = "3IJKL\n4ABCD\n5EFGH"; + try testing.expectEqualStrings(expected, contents); + } + + // Put our cursor on the end + s.cursor.x = s.cols - 1; + s.cursor.y = s.rows - 1; + try testing.expectEqual(@as(u32, 'H'), s.getCell(.active, s.cursor.y, s.cursor.x).char); + + try s.resize(3, 3); + + { + var contents = try s.testString(alloc, .viewport); + defer alloc.free(contents); + const expected = "CD5\nEFG\nH"; + try testing.expectEqualStrings(expected, contents); + } + { + var contents = try s.testString(alloc, .screen); + defer alloc.free(contents); + const expected = "JKL\n4AB\nCD5\nEFG\nH"; + try testing.expectEqualStrings(expected, contents); + } + + // Cursor should be on the last line + try testing.expectEqual(@as(u32, 'H'), s.getCell(.active, s.cursor.y, s.cursor.x).char); + try testing.expectEqual(@as(usize, 0), s.cursor.x); + try testing.expectEqual(@as(usize, 2), s.cursor.y); +} + // This seems like it should work fine but for some reason in practice // in the initial implementation I found this bug! This is a regression // test for that. From 05fe2a83b19d7a26e7207e7abe7b2a5d7321d469 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 21:30:31 -0800 Subject: [PATCH 12/14] terminal: erase display below should unwrap soft wrapped state --- src/terminal/Screen.zig | 15 +++++++-------- src/terminal/Terminal.zig | 27 +++++++++++++++++---------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 72090bf8c..3438cc745 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1876,22 +1876,21 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { while (iter.next()) |old_row| { // If we're past the end, scroll if (y >= self.rows) { - y -= 1; try self.scroll(.{ .delta = 1 }); + y -= 1; } - // Get this row - var new_row = self.getRow(.{ .active = y }); - try new_row.copyRow(old_row); - // We need to check if our cursor was on this line. If so, // we set the new cursor. if (cursor_pos.y == iter.value - 1) { assert(new_cursor == null); // should only happen once - new_cursor = .{ .y = self.rowsWritten() - 1, .x = cursor_pos.x }; + new_cursor = .{ .y = self.history + y, .x = cursor_pos.x }; } - // If no reflow, just keep going. + // At this point, we're always at x == 0 so we can just copy + // the row (we know old.cols < self.cols). + var new_row = self.getRow(.{ .active = y }); + try new_row.copyRow(old_row); if (!old_row.header().flags.wrap) { // If we have no reflow, we attempt to extend any stylized // cells at the end of the line if there is one. @@ -1955,7 +1954,7 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { cursor_pos.x < copy_len and new_cursor == null) { - new_cursor = .{ .y = self.rowsWritten() - 1, .x = x + cursor_pos.x }; + new_cursor = .{ .y = self.history + y, .x = x + cursor_pos.x }; } // We copied the full amount left in this wrapped row. diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index fb0189d03..b67ab58b7 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -912,19 +912,26 @@ pub fn eraseDisplay( .below => { // All lines to the right (including the cursor) - var x: usize = self.screen.cursor.x; - while (x < self.cols) : (x += 1) { - const cell = self.screen.getCellPtr(.active, self.screen.cursor.y, x); - cell.* = self.screen.cursor.pen; - cell.char = 0; + { + const row = self.screen.getRow(.{ .active = self.screen.cursor.y }); + row.setWrapped(false); + row.setDirty(true); + for (self.screen.cursor.x..self.cols) |x| { + if (row.header().flags.grapheme) row.clearGraphemes(x); + const cell = row.getCellPtr(x); + cell.* = self.screen.cursor.pen; + cell.char = 0; + } } // All lines below - var y: usize = self.screen.cursor.y + 1; - while (y < self.rows) : (y += 1) { - x = 0; - while (x < self.cols) : (x += 1) { - const cell = self.screen.getCellPtr(.active, y, x); + for ((self.screen.cursor.y + 1)..self.rows) |y| { + const row = self.screen.getRow(.{ .active = y }); + row.setWrapped(false); + row.setDirty(true); + for (0..self.cols) |x| { + if (row.header().flags.grapheme) row.clearGraphemes(x); + const cell = row.getCellPtr(x); cell.* = self.screen.cursor.pen; cell.char = 0; } From 28378a350dfd567214e85d1149044e9c52fab652 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 22:01:42 -0800 Subject: [PATCH 13/14] screen: shrinking cols trims trailing blank lines --- src/terminal/Screen.zig | 63 ++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 3438cc745..88ec89579 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1726,21 +1726,7 @@ pub fn resizeWithoutReflow(self: *Screen, rows: usize, cols: usize) !void { // we generate scrollback. Why? Terminal.app does it, seems... fine. if (self.history > 0) break :blank 0; - // Start one line below our cursor and continue to the last line - // of the screen or however many rows we have written. - const start = self.cursor.y + 1; - const end = @min(self.rowsWritten(), self.rows); - if (start >= end) break :blank 0; - - var blank: usize = 0; - for (0..(end - start)) |i| { - const y = end - i - 1; - const row = self.getRow(.{ .active = y }); - if (!row.isEmpty()) break; - blank += 1; - } - - break :blank blank; + break :blank self.trailingBlankLines(); }; // Make a copy so we can access the old indexes. @@ -2053,10 +2039,15 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { self.viewport = 0; self.history = 0; - // Iterate over the screen since we need to check for reflow. - var iter = old.rowIterator(.screen); + // Iterate over the screen since we need to check for reflow. We + // clear all the trailing blank lines so that shells like zsh and + // fish that often clear the display below don't force us to have + // scrollback. + var old_y: usize = 0; + const end_y = RowIndexTag.screen.maxLen(&old) - old.trailingBlankLines(); var y: usize = 0; - while (iter.next()) |old_row| { + while (old_y < end_y) : (old_y += 1) { + const old_row = old.getRow(.{ .screen = old_y }); const old_row_wrapped = old_row.header().flags.wrap; const trimmed_row = self.trimRowForResizeLessCols(&old, old_row); @@ -2071,7 +2062,7 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { // copy and move on. if (!old_row_wrapped and trimmed_row.len <= self.cols) { // If our cursor is on this line, then set the new cursor. - if (cursor_pos.y == iter.value - 1) { + if (cursor_pos.y == old_y) { assert(new_cursor == null); new_cursor = .{ .x = cursor_pos.x, .y = self.history + y }; } @@ -2113,7 +2104,7 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { } // If our cursor is on this char, then set the new cursor. - if (cursor_pos.y == iter.value - 1 and cursor_pos.x == old_x) { + if (cursor_pos.y == old_y and cursor_pos.x == old_x) { assert(new_cursor == null); new_cursor = .{ .x = x, .y = self.history + y }; } @@ -2132,8 +2123,8 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { // If the old row is wrapped we continue with the loop with // the next row. - cur_old_row = iter.next() orelse - @panic("if the current row is wrapped, must have a follow-up row"); + old_y += 1; + cur_old_row = old.getRow(.{ .screen = old_y }); cur_old_row_wrapped = cur_old_row.header().flags.wrap; cur_trimmed_row = self.trimRowForResizeLessCols(&old, cur_old_row); } @@ -2155,6 +2146,27 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { } } +/// Counts the number of trailing lines from the cursor that are blank. +/// This is specifically used for resizing and isn't meant to be a general +/// purpose tool. +fn trailingBlankLines(self: *Screen) usize { + // Start one line below our cursor and continue to the last line + // of the screen or however many rows we have written. + const start = self.cursor.y + 1; + const end = @min(self.rowsWritten(), self.rows); + if (start >= end) return 0; + + var blank: usize = 0; + for (0..(end - start)) |i| { + const y = end - i - 1; + const row = self.getRow(.{ .active = y }); + if (!row.isEmpty()) break; + blank += 1; + } + + return blank; +} + /// When resizing to less columns, this trims the row from the right /// so we don't unnecessarily wrap. This will freely throw away trailing /// colored but empty (character) cells. This matches Terminal.app behavior, @@ -4679,13 +4691,6 @@ test "Screen: resize less cols trailing background colors" { const cell = row.getCellPtr(x); try testing.expectEqual(pen, cell.*); } - for ((s.cursor.y + 1)..s.rows) |y| { - const row = s.getRow(.{ .active = y }); - for (0..s.cols) |x| { - const cell = row.getCellPtr(x); - try testing.expectEqual(pen, cell.*); - } - } } test "Screen: resize less cols with graphemes" { From ba96a2c023fec9fc3ea7a7a35f741b4fa89781fc Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 1 Mar 2023 22:14:09 -0800 Subject: [PATCH 14/14] screen: adding cols doesn't mess with the viewport This was untested anyways, and the result was bugs! --- src/terminal/Screen.zig | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 88ec89579..a8c798d93 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1947,13 +1947,6 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { if (copy_len == wrapped_cells_rem) { // If this row isn't also wrapped, we're done! if (!wrapped_row.header().flags.wrap) { - // If we were able to copy the entire row then - // we shortened the screen by one. We need to reflect - // this in our viewport. - if (wrapped_i == 0 and old.viewport > 0) { - old.viewport -= 1; - } - y += 1; break :wrapping; } @@ -1981,12 +1974,6 @@ pub fn resize(self: *Screen, rows: usize, cols: usize) !void { } } - // During the resize, we just keep the viewport at the bottom. But - // we want to restore the viewport to what the user had when they - // started the resize. old.viewport is maintained for removed lines - // in the for loop above. - self.viewport = old.viewport; - // If we have a new cursor, we need to convert that to a viewport // point and set it up. if (new_cursor) |pos| {