From 7dac9e02b343c8ed514c94339bf299d9ca2956bc Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Feb 2025 10:16:33 -0800 Subject: [PATCH] terminal: reflow the saved cursor if we have one Fixes #5718 When a terminal is resized with text reflow (i.e. soft-wrapped text), the cursor is generally reflowed with it. For example, imagine a terminal window 5-columns wide and you type the following without pressing enter. The cursor is on the X. ``` OOOOO OOX ``` If you resize the window now to 8 or more columns, this happens, as expected: ``` OOOOOOOX ``` As expected, the cursor remains on the "X". This behaves like any other text input... Terminals also provide an escape sequence to [save the cursor (ESC 7 aka DECSC)](https://ghostty.org/docs/vt/esc/decsc). This includes, amongst other things, the cursor position. The cursor can be restored with [DECRC](https://ghostty.org/docs/vt/esc/decrc). The behavior of the position of the _saved cursor_ in the context of text reflow is unspecified and varies wildly between terminals Ghostty does this right now (as do many other terminals): ``` OOOOOOOO X ``` This commit changes the behavior so that we reflow the saved cursor. --- src/terminal/Screen.zig | 42 ++++++++++++++++++++ src/terminal/Terminal.zig | 81 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 339599728..273e1aebe 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1590,6 +1590,18 @@ fn resizeInternal( self.cursor.hyperlink = null; } + // We need to insert a tracked pin for our saved cursor so we can + // modify its X/Y for reflow. + const saved_cursor_pin: ?*Pin = saved_cursor: { + const sc = self.saved_cursor orelse break :saved_cursor null; + const pin = self.pages.pin(.{ .active = .{ + .x = sc.x, + .y = sc.y, + } }) orelse break :saved_cursor null; + break :saved_cursor try self.pages.trackPin(pin); + }; + defer if (saved_cursor_pin) |p| self.pages.untrackPin(p); + // Perform the resize operation. try self.pages.resize(.{ .rows = rows, @@ -1609,6 +1621,36 @@ fn resizeInternal( // state is correct. self.cursorReload(); + // If we reflowed a saved cursor, update it. + if (saved_cursor_pin) |p| { + // This should never fail because a non-null saved_cursor_pin + // implies a non-null saved_cursor. + const sc = &self.saved_cursor.?; + if (self.pages.pointFromPin(.active, p.*)) |pt| { + sc.x = @intCast(pt.active.x); + sc.y = @intCast(pt.active.y); + + // If we had pending wrap set and we're no longer at the end of + // the line, we unset the pending wrap and move the cursor to + // reflect the correct next position. + if (sc.pending_wrap and sc.x != cols - 1) { + sc.pending_wrap = false; + sc.x += 1; + } + } else { + // I think this can happen if the screen is resized to be + // less rows or less cols and our saved cursor moves outside + // the active area. In this case, there isn't anything really + // reasonable we can do so we just move the cursor to the + // top-left. It may be reasonable to also move the cursor to + // match the primary cursor. Any behavior is fine since this is + // totally unspecified. + sc.x = 0; + sc.y = 0; + sc.pending_wrap = false; + } + } + // Fix up our hyperlink if we had one. if (hyperlink_) |link| { self.startHyperlink(link.uri, switch (link.id) { diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 65476108d..bec0a24a2 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -10708,6 +10708,87 @@ test "Terminal: resize with high unique style per cell with wrapping" { try t.resize(alloc, 60, 30); } +test "Terminal: resize with reflow and saved cursor" { + const alloc = testing.allocator; + var t = try init(alloc, .{ .cols = 2, .rows = 3 }); + defer t.deinit(alloc); + try t.printString("1A2B"); + t.setCursorPos(2, 2); + { + const list_cell = t.screen.pages.getCell(.{ .active = .{ + .x = t.screen.cursor.x, + .y = t.screen.cursor.y, + } }).?; + const cell = list_cell.cell; + try testing.expectEqual(@as(u32, 'B'), cell.content.codepoint); + } + + { + const str = try t.plainString(testing.allocator); + defer testing.allocator.free(str); + try testing.expectEqualStrings("1A\n2B", str); + } + + t.saveCursor(); + try t.resize(alloc, 5, 3); + try t.restoreCursor(); + + { + const str = try t.plainString(testing.allocator); + defer testing.allocator.free(str); + try testing.expectEqualStrings("1A2B", str); + } + + // Verify our cursor is still in the same place + { + const list_cell = t.screen.pages.getCell(.{ .active = .{ + .x = t.screen.cursor.x, + .y = t.screen.cursor.y, + } }).?; + const cell = list_cell.cell; + try testing.expectEqual(@as(u32, 'B'), cell.content.codepoint); + } +} + +test "Terminal: resize with reflow and saved cursor pending wrap" { + const alloc = testing.allocator; + var t = try init(alloc, .{ .cols = 2, .rows = 3 }); + defer t.deinit(alloc); + try t.printString("1A2B"); + { + const list_cell = t.screen.pages.getCell(.{ .active = .{ + .x = t.screen.cursor.x, + .y = t.screen.cursor.y, + } }).?; + const cell = list_cell.cell; + try testing.expectEqual(@as(u32, 'B'), cell.content.codepoint); + } + + { + const str = try t.plainString(testing.allocator); + defer testing.allocator.free(str); + try testing.expectEqualStrings("1A\n2B", str); + } + + t.saveCursor(); + try t.resize(alloc, 5, 3); + try t.restoreCursor(); + + { + const str = try t.plainString(testing.allocator); + defer testing.allocator.free(str); + try testing.expectEqualStrings("1A2B", str); + } + + // Pending wrap should be reset + try t.print('X'); + { + const str = try t.plainString(testing.allocator); + defer testing.allocator.free(str); + try testing.expectEqualStrings("1A2BX", str); + } +} + test "Terminal: DECCOLM without DEC mode 40" { const alloc = testing.allocator; var t = try init(alloc, .{ .rows = 5, .cols = 5 });