Skip to content

Commit

Permalink
fix: cmd+triple-click select all command output when first line wraps (
Browse files Browse the repository at this point in the history
…#5373)

I found this bug was easily reproduced with any command that wrapped to
multiple rows on the first line of its output. The cause is that we stop
searching for rows once we reach the first one where
`row.semantic_prompt = .command`, which means that we reach the bottom
line of wrapped output and stop there.

This PR makes it so that we continue iterating until we reach a row
where `semantic_prompt != .command` and then return the previous one (or
the last one if we run out of rows).

I also updated the test cases to include this.

I considered that this bug would also be avoided if we didn't propagate
the `command` semantic prompt to additional rows on wrapped lines, but I
don't know enough about the shell integration to make a call on that.

Closes #4693
  • Loading branch information
mitchellh authored Jan 29, 2025
2 parents d31e6c8 + 4b8010a commit 76fd4fa
Showing 1 changed file with 55 additions and 19 deletions.
74 changes: 55 additions & 19 deletions src/terminal/Screen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2591,13 +2591,39 @@ pub fn selectOutput(self: *Screen, pin: Pin) ?Selection {
const start: Pin = boundary: {
var it = pin.rowIterator(.left_up, null);
var it_prev = pin;

// First, iterate until we find the first line of command output
while (it.next()) |p| {
it_prev = p;
const row = p.rowAndCell().row;
switch (row.semantic_prompt) {
.command => break :boundary p,
else => {},
.command => break,

.unknown,
.prompt,
.prompt_continuation,
.input,
=> {},
}
if (row.semantic_prompt == .command) {
break;
}
}

// Because the first line of command output may span multiple visual rows we must now
// iterate until we find the first row of anything other than command output and then
// yield the previous row.
while (it.next()) |p| {
const row = p.rowAndCell().row;
switch (row.semantic_prompt) {
.command => {},

.unknown,
.prompt,
.prompt_continuation,
.input,
=> break :boundary it_prev,
}
it_prev = p;
}

Expand Down Expand Up @@ -7641,17 +7667,17 @@ test "Screen: selectOutput" {

// zig fmt: off
{
// line number:
try s.testWriteString("output1\n"); // 0
try s.testWriteString("output1\n"); // 1
try s.testWriteString("prompt2\n"); // 2
try s.testWriteString("input2\n"); // 3
try s.testWriteString("output2\n"); // 4
try s.testWriteString("output2\n"); // 5
try s.testWriteString("prompt3$ input3\n"); // 6
try s.testWriteString("output3\n"); // 7
try s.testWriteString("output3\n"); // 8
try s.testWriteString("output3"); // 9
// line number:
try s.testWriteString("output1\n"); // 0
try s.testWriteString("output1\n"); // 1
try s.testWriteString("prompt2\n"); // 2
try s.testWriteString("input2\n"); // 3
try s.testWriteString("output2output2output2output2\n"); // 4, 5, 6 due to overflow
try s.testWriteString("output2\n"); // 7
try s.testWriteString("prompt3$ input3\n"); // 8
try s.testWriteString("output3\n"); // 9
try s.testWriteString("output3\n"); // 10
try s.testWriteString("output3"); // 11
}
// zig fmt: on

Expand All @@ -7670,13 +7696,23 @@ test "Screen: selectOutput" {
const row = pin.rowAndCell().row;
row.semantic_prompt = .command;
}
{
const pin = s.pages.pin(.{ .screen = .{ .y = 5 } }).?;
const row = pin.rowAndCell().row;
row.semantic_prompt = .command;
}
{
const pin = s.pages.pin(.{ .screen = .{ .y = 6 } }).?;
const row = pin.rowAndCell().row;
row.semantic_prompt = .command;
}
{
const pin = s.pages.pin(.{ .screen = .{ .y = 8 } }).?;
const row = pin.rowAndCell().row;
row.semantic_prompt = .input;
}
{
const pin = s.pages.pin(.{ .screen = .{ .y = 7 } }).?;
const pin = s.pages.pin(.{ .screen = .{ .y = 9 } }).?;
const row = pin.rowAndCell().row;
row.semantic_prompt = .command;
}
Expand All @@ -7701,7 +7737,7 @@ test "Screen: selectOutput" {
{
var sel = s.selectOutput(s.pages.pin(.{ .active = .{
.x = 3,
.y = 5,
.y = 7,
} }).?).?;
defer sel.deinit(&s);
try testing.expectEqual(point.Point{ .active = .{
Expand All @@ -7710,23 +7746,23 @@ test "Screen: selectOutput" {
} }, s.pages.pointFromPin(.active, sel.start()).?);
try testing.expectEqual(point.Point{ .active = .{
.x = 9,
.y = 5,
.y = 7,
} }, s.pages.pointFromPin(.active, sel.end()).?);
}
// No end marker, should select till the end
{
var sel = s.selectOutput(s.pages.pin(.{ .active = .{
.x = 2,
.y = 7,
.y = 10,
} }).?).?;
defer sel.deinit(&s);
try testing.expectEqual(point.Point{ .active = .{
.x = 0,
.y = 7,
.y = 9,
} }, s.pages.pointFromPin(.active, sel.start()).?);
try testing.expectEqual(point.Point{ .active = .{
.x = 9,
.y = 10,
.y = 12,
} }, s.pages.pointFromPin(.active, sel.end()).?);
}
// input / prompt at y = 0, pt.y = 0
Expand Down

0 comments on commit 76fd4fa

Please sign in to comment.