From 360124ded029c9c150a858a0808b008506230f85 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 25 Jun 2025 23:16:43 -0600 Subject: [PATCH 1/2] terminal/Screen: account for rectangle selection in clone Fixes an issue where rectangle selections would appear visually wrong if their start or end were out of the viewport area, because when cloning them the restored pins were defaulting to the start and end of the row instead of the appropriate column. --- src/terminal/Screen.zig | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 2688b03a7..deff4a394 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -402,8 +402,8 @@ pub fn clonePool( }; const start_pin = pin_remap.get(ordered.tl) orelse start: { - // No start means it is outside the cloned area. We change it - // to the top-left. + // No start means it is outside the cloned area. + // We move it back in bounds to the top row. // If we have no end pin then either // (1) our whole selection is outside the cloned area or @@ -417,14 +417,17 @@ pub fn clonePool( if (!sel.contains(self, clone_top)) break :sel null; } - break :start try pages.trackPin(.{ .node = pages.pages.first.? }); + break :start try pages.trackPin(.{ + .node = pages.pages.first.?, + .x = if (sel.rectangle) ordered.tl.x else 0, + }); }; const end_pin = pin_remap.get(ordered.br) orelse end: { - // No end means it is outside the cloned area. We change it - // to the bottom-right. + // No end means it is outside the cloned area. + // We move it back in bounds to the bottom row. break :end try pages.trackPin(pages.pin(.{ .active = .{ - .x = pages.cols - 1, + .x = if (sel.rectangle) ordered.br.x else pages.cols - 1, .y = pages.rows - 1, } }) orelse break :sel null); }; From f7ee6b3bdacc64a4973b121dc5f06fdeffb4761f Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Thu, 26 Jun 2025 11:26:03 -0600 Subject: [PATCH 2/2] terminal/Screen: clean up selection remap in clone Cleans up the logic, checks for out of bounds using rows instead of sel.contains because that excludes cases where a rectangle selection doesn't include the leftmost column. Also adds test for clipping behavior of rectangular selections. --- src/terminal/Screen.zig | 79 +++++++++++++++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 14 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index deff4a394..8cdaf3fa2 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -403,34 +403,46 @@ pub fn clonePool( const start_pin = pin_remap.get(ordered.tl) orelse start: { // No start means it is outside the cloned area. - // We move it back in bounds to the top row. // If we have no end pin then either // (1) our whole selection is outside the cloned area or // (2) our cloned area is within the selection if (pin_remap.get(ordered.br) == null) { - // If our tl is before the cloned area and br is after - // the cloned area then the whole screen is selected. - // This detection is somewhat more expensive so we try - // to avoid it if possible so its nested in this if. + // We check if the selection bottom right pin is above + // the cloned area or if the top left pin is below the + // cloned area, in either of these cases it means that + // the selection is fully out of bounds, so we have no + // selection in the cloned area and break out now. const clone_top = self.pages.pin(top) orelse break :sel null; - if (!sel.contains(self, clone_top)) break :sel null; + const clone_top_y = self.pages.pointFromPin( + .screen, + clone_top, + ).?.screen.y; + if (self.pages.pointFromPin( + .screen, + ordered.br.*, + ).?.screen.y < clone_top_y) break :sel null; + if (self.pages.pointFromPin( + .screen, + ordered.tl.*, + ).?.screen.y > clone_top_y) break :sel null; } + // We move the top pin back in bounds to the top row. break :start try pages.trackPin(.{ .node = pages.pages.first.?, .x = if (sel.rectangle) ordered.tl.x else 0, }); }; - const end_pin = pin_remap.get(ordered.br) orelse end: { - // No end means it is outside the cloned area. - // We move it back in bounds to the bottom row. - break :end try pages.trackPin(pages.pin(.{ .active = .{ - .x = if (sel.rectangle) ordered.br.x else pages.cols - 1, - .y = pages.rows - 1, - } }) orelse break :sel null); - }; + // If we got to this point it means that the selection is not + // fully out of bounds, so we move the bottom right pin back + // in bounds if it isn't already. + const end_pin = pin_remap.get(ordered.br) orelse try pages.trackPin(.{ + .node = pages.pages.last.?, + .x = if (sel.rectangle) ordered.br.x else pages.cols - 1, + .y = pages.pages.last.?.data.size.rows - 1, + }); break :sel .{ .bounds = .{ .tracked = .{ @@ -5290,6 +5302,45 @@ test "Screen: clone contains subset of selection" { } } +test "Screen: clone contains subset of rectangle selection" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 5, 4, 1); + defer s.deinit(); + try s.testWriteString("1ABCD\n2EFGH\n3IJKL\n4ABCD"); + + // Select the full screen from x=1 to x=3 + try s.select(Selection.init( + s.pages.pin(.{ .active = .{ .x = 1, .y = 0 } }).?, + s.pages.pin(.{ .active = .{ .x = 3, .y = 3 } }).?, + true, + )); + + // Clone + var s2 = try s.clone( + alloc, + .{ .active = .{ .y = 1 } }, + .{ .active = .{ .y = 2 } }, + ); + defer s2.deinit(); + + // Our selection should remain valid and be properly clipped + // preserving the columns of the start and end points of the + // selection. + { + const sel = s2.selection.?; + try testing.expectEqual(point.Point{ .active = .{ + .x = 1, + .y = 0, + } }, s2.pages.pointFromPin(.active, sel.start()).?); + try testing.expectEqual(point.Point{ .active = .{ + .x = 3, + .y = 3, + } }, s2.pages.pointFromPin(.active, sel.end()).?); + } +} + test "Screen: clone basic" { const testing = std.testing; const alloc = testing.allocator;