From a6ad489c975d3b7aa862ba2d115215d692c96231 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 3 Mar 2024 22:10:08 -0800 Subject: [PATCH] terminal/new: fix issue with resizing when cursor is in blank trailing cell --- src/terminal/Screen.zig | 6 +++ src/terminal/new/PageList.zig | 85 +++++++++++++++++++++++++++++++++++ src/terminal/new/Screen.zig | 45 +++++++++++++++++++ 3 files changed, 136 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 07e81b0a0..5cce3f0db 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -6556,6 +6556,7 @@ test "Screen: resize (no reflow) more cols with scrollback scrolled up" { try testing.expectEqual(@as(usize, 2), s.cursor.y); } +// X // https://github.com/mitchellh/ghostty/issues/1159 test "Screen: resize (no reflow) less cols with scrollback scrolled up" { const testing = std.testing; @@ -6583,6 +6584,11 @@ test "Screen: resize (no reflow) less cols with scrollback scrolled up" { defer alloc.free(contents); try testing.expectEqualStrings(str, contents); } + { + const contents = try s.testString(alloc, .active); + defer alloc.free(contents); + try testing.expectEqualStrings("6\n7\n8", contents); + } // Cursor remains at bottom try testing.expectEqual(@as(usize, 1), s.cursor.x); diff --git a/src/terminal/new/PageList.zig b/src/terminal/new/PageList.zig index f6fe10224..acef3957a 100644 --- a/src/terminal/new/PageList.zig +++ b/src/terminal/new/PageList.zig @@ -760,6 +760,29 @@ fn reflowPage( // Move both our cursors forward src_cursor.cursorForward(); dst_cursor.cursorForward(); + } else cursor: { + // We made it through all our source columns. As a final edge + // case, if our cursor is in one of the blanks, we update it + // to the edge of this page. + + // If we have no trailing empty cells, it can't be in the blanks. + if (trailing_empty == 0) break :cursor; + + // If we have no cursor, nothing to update. + const c = cursor orelse break :cursor; + const offset = c.offset orelse break :cursor; + + // If our cursor is on this page, and our x is greater than + // our end, we update to the edge. + if (&offset.page.data == src_cursor.page and + offset.row_offset == src_cursor.y and + c.x >= cols_len) + { + c.offset = .{ + .page = dst_node, + .row_offset = dst_cursor.y, + }; + } } } else { // We made it through all our source rows, we're done. @@ -3297,6 +3320,68 @@ test "PageList resize reflow less cols cursor in unchanged row" { try testing.expectEqual(@as(size.CellCountInt, 0), cursor.y); } +test "PageList resize reflow less cols cursor in blank cell" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 6, 2, null); + defer s.deinit(); + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + for (0..s.rows) |y| { + for (0..2) |x| { + const rac = page.getRowAndCell(x, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(x) }, + }; + } + } + + // Set our cursor to be in a blank cell + var cursor: Resize.Cursor = .{ .x = 2, .y = 0 }; + + // Resize + try s.resize(.{ .cols = 4, .reflow = true, .cursor = &cursor }); + try testing.expectEqual(@as(usize, 4), s.cols); + try testing.expectEqual(@as(usize, 2), s.totalRows()); + + // Our cursor should move to the first row + try testing.expectEqual(@as(size.CellCountInt, 2), cursor.x); + try testing.expectEqual(@as(size.CellCountInt, 0), cursor.y); +} + +test "PageList resize reflow less cols cursor in final blank cell" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 6, 2, null); + defer s.deinit(); + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + for (0..s.rows) |y| { + for (0..2) |x| { + const rac = page.getRowAndCell(x, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(x) }, + }; + } + } + + // Set our cursor to be in the final cell of our resized + var cursor: Resize.Cursor = .{ .x = 3, .y = 0 }; + + // Resize + try s.resize(.{ .cols = 4, .reflow = true, .cursor = &cursor }); + try testing.expectEqual(@as(usize, 4), s.cols); + try testing.expectEqual(@as(usize, 2), s.totalRows()); + + // Our cursor should move to the first row + try testing.expectEqual(@as(size.CellCountInt, 3), cursor.x); + try testing.expectEqual(@as(size.CellCountInt, 0), cursor.y); +} + test "PageList resize reflow less cols blank lines" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/new/Screen.zig b/src/terminal/new/Screen.zig index a8b5bb740..af6ed0e27 100644 --- a/src/terminal/new/Screen.zig +++ b/src/terminal/new/Screen.zig @@ -2258,6 +2258,51 @@ test "Screen: resize (no reflow) more cols with scrollback scrolled up" { try testing.expectEqual(@as(size.CellCountInt, 2), s.cursor.y); } +// https://github.com/mitchellh/ghostty/issues/1159 +test "Screen: resize (no reflow) less cols with scrollback scrolled up" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 5, 3, 5); + defer s.deinit(); + const str = "1\n2\n3\n4\n5\n6\n7\n8"; + try s.testWriteString(str); + + // Cursor at bottom + try testing.expectEqual(@as(size.CellCountInt, 1), s.cursor.x); + try testing.expectEqual(@as(size.CellCountInt, 2), s.cursor.y); + + s.scroll(.{ .delta_row = -4 }); + { + const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings("2\n3\n4", contents); + } + + try s.resize(4, 3); + { + const contents = try s.dumpStringAlloc(alloc, .{ .screen = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings(str, contents); + } + { + const contents = try s.dumpStringAlloc(alloc, .{ .active = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings("6\n7\n8", contents); + } + + // Cursor remains at bottom + try testing.expectEqual(@as(size.CellCountInt, 1), s.cursor.x); + try testing.expectEqual(@as(size.CellCountInt, 2), s.cursor.y); + + // Old implementation doesn't do this but it makes sense to me: + // { + // const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); + // defer alloc.free(contents); + // try testing.expectEqualStrings("2\n3\n4", contents); + // } +} + test "Screen: resize more cols with reflow that fits full width" { const testing = std.testing; const alloc = testing.allocator;