Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/Surface.zig
Original file line number Diff line number Diff line change
Expand Up @@ -5656,6 +5656,11 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !bool
v,
),

.write_viewport_file => |v| try self.writeScreenFile(
.viewport,
v,
),

.write_scrollback_file => |v| try self.writeScreenFile(
.history,
v,
Expand Down Expand Up @@ -6048,6 +6053,7 @@ fn closingAction(action: input.Binding.Action) bool {
/// The portion of the screen to write for writeScreenFile.
const WriteScreenLoc = enum {
screen, // Full screen
viewport, // Visible viewport
history, // History (scrollback)
selection, // Selected text
};
Expand Down Expand Up @@ -6099,6 +6105,15 @@ fn writeScreenFile(
// command always works on the primary screen.
const pages = &self.io.terminal.screens.active.pages;
const sel_: ?terminal.Selection = switch (loc) {
.viewport => viewport: {
break :viewport terminal.Selection.init(
pages.getTopLeft(.viewport),
pages.getBottomRight(.viewport) orelse
break :viewport null,
false,
);
},
Comment on lines +6108 to +6115
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

orelse branch is dead code for .viewport

getBottomRight(.viewport) is guaranteed to return a non-null Pin — its implementation unconditionally computes a pin via br.down(self.rows - 1).? (which itself asserts non-null internally) and then returns it. This means orelse break :viewport null will never be taken.

This is consistent with how .screen is handled (also using orelse defensively), so it's not a correctness issue, but it is worth noting that unlike .history — where the null return is possible when the history region is empty — the viewport region always exists in a live terminal. A comment clarifying this behavior (similar to the one on .history about alternate screens) would improve readability.


.history => history: {
// We do not support this for alternate screens
// because they don't have scrollback anyways.
Expand Down
7 changes: 7 additions & 0 deletions src/input/Binding.zig
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,12 @@ pub const Action = union(enum) {
/// See `write_scrollback_file` for possible actions.
write_screen_file: WriteScreen,

/// Write the visible viewport into a temporary file with the
/// specified action.
///
/// See `write_scrollback_file` for possible actions.
write_viewport_file: WriteScreen,

/// Write the currently selected text into a temporary file with the
/// specified action.
///
Expand Down Expand Up @@ -1338,6 +1344,7 @@ pub const Action = union(enum) {
.jump_to_prompt,
.write_scrollback_file,
.write_screen_file,
.write_viewport_file,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No parsing test added for write_viewport_file

The sibling actions write_screen_file and write_scrollback_file both have dedicated parsing tests (around line 4503) that exercise both the shorthand form (e.g. write_screen_file:copy) and the extended form (e.g. write_screen_file:copy,html). There are no corresponding tests for write_viewport_file.

Since parsing is shared via the WriteScreen type, the behaviour is almost certainly correct, but adding test coverage would be consistent with the existing test patterns and would guard against future regressions, e.g.:

{
    const binding = try parseSingle("a=write_viewport_file:copy");
    try testing.expect(binding.action == .write_viewport_file);
    try testing.expectEqual(Action.WriteScreen.copy, binding.action.write_viewport_file);
}

{
    const binding = try parseSingle("a=write_viewport_file:copy,vt");
    try testing.expect(binding.action == .write_viewport_file);
    try testing.expectEqual(Action.WriteScreen{
        .action = .copy,
        .emit = .vt,
    }, binding.action.write_viewport_file);
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

.write_selection_file,
.close_surface,
.close_tab,
Expand Down
68 changes: 68 additions & 0 deletions src/input/command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,74 @@ fn actionCommands(action: Action.Key) []const Command {
},
},

.write_viewport_file => comptime &.{
.{
.action = .{ .write_viewport_file = .copy },
.title = "Copy Viewport to Temporary File and Copy Path",
.description = "Copy the visible viewport contents to a temporary file and copy the path to the clipboard.",
},
.{
.action = .{ .write_viewport_file = .paste },
.title = "Copy Viewport to Temporary File and Paste Path",
.description = "Copy the visible viewport contents to a temporary file and paste the path to the file.",
},
.{
.action = .{ .write_viewport_file = .open },
.title = "Copy Viewport to Temporary File and Open",
.description = "Copy the visible viewport contents to a temporary file and open it.",
},

.{
.action = .{ .write_viewport_file = .{
.action = .copy,
.emit = .html,
} },
.title = "Copy Viewport as HTML to Temporary File and Copy Path",
.description = "Copy the visible viewport contents as HTML to a temporary file and copy the path to the clipboard.",
},
.{
.action = .{ .write_viewport_file = .{
.action = .paste,
.emit = .html,
} },
.title = "Copy Viewport as HTML to Temporary File and Paste Path",
.description = "Copy the visible viewport contents as HTML to a temporary file and paste the path to the file.",
},
.{
.action = .{ .write_viewport_file = .{
.action = .open,
.emit = .html,
} },
.title = "Copy Viewport as HTML to Temporary File and Open",
.description = "Copy the visible viewport contents as HTML to a temporary file and open it.",
},

.{
.action = .{ .write_viewport_file = .{
.action = .copy,
.emit = .vt,
} },
.title = "Copy Viewport as ANSI Sequences to Temporary File and Copy Path",
.description = "Copy the visible viewport contents as ANSI escape sequences to a temporary file and copy the path to the clipboard.",
},
.{
.action = .{ .write_viewport_file = .{
.action = .paste,
.emit = .vt,
} },
.title = "Copy Viewport as ANSI Sequences to Temporary File and Paste Path",
.description = "Copy the visible viewport contents as ANSI escape sequences to a temporary file and paste the path to the file.",
},
.{
.action = .{ .write_viewport_file = .{
.action = .open,
.emit = .vt,
} },
.title = "Copy Viewport as ANSI Sequences to Temporary File and Open",
.description = "Copy the visible viewport contents as ANSI escape sequences to a temporary file and open it.",
},
},

.write_selection_file => comptime &.{
.{
.action = .{ .write_selection_file = .copy },
Expand Down
Loading