terminal/Screen: account for rectangle selection in clone (#7692)

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.

This issue is shown in discussion #7687.
pull/7716/head
Mitchell Hashimoto 2025-06-26 13:15:48 -07:00 committed by GitHub
commit 3d01bb43cc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 70 additions and 16 deletions

View File

@ -402,32 +402,47 @@ 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.
// 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;
}
break :start try pages.trackPin(.{ .node = pages.pages.first.? });
// 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 change it
// to the bottom-right.
break :end try pages.trackPin(pages.pin(.{ .active = .{
.x = 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 = .{
@ -5287,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;