Skip to content
Open
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
44 changes: 33 additions & 11 deletions src/Surface.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2363,7 +2363,13 @@ fn copySelectionToClipboards(
///
/// This must be called with the renderer mutex held.
fn setSelection(self: *Surface, sel_: ?terminal.Selection) !void {
// Evaluate equality BEFORE Screen.select() frees the old tracked
// pins. Screen.select() calls old.deinit() which returns tracked
// pin memory to the pool; reading prev after that is a
// use-after-free that usually (but not always) compares equal,
// silently skipping the clipboard copy.
const prev_ = self.io.terminal.screens.active.selection;
const same = if (prev_) |prev| if (sel_) |sel| sel.eql(prev) else false else false;
try self.io.terminal.screens.active.select(sel_);

// If copy on select is false then exit early.
Expand All @@ -2374,7 +2380,7 @@ fn setSelection(self: *Surface, sel_: ?terminal.Selection) !void {
// again if it changed, since setting the clipboard can be an expensive
// operation.
const sel = sel_ orelse return;
if (prev_) |prev| if (sel.eql(prev)) return;
if (same) return;

switch (self.config.copy_on_select) {
.false => unreachable, // handled above with an early exit
Expand Down Expand Up @@ -3834,19 +3840,35 @@ pub fn mouseButtonCallback(
);
}

// The selection clipboard is only updated for left-click drag when
// the left button is released. This is to avoid the clipboard
// being updated on every mouse move which would be noisy.
// Copy the final selection to the clipboard on mouse-up.
// During a drag, mouseDrag → setSelection is called on every
// move, but the eql() optimisation skips redundant clipboard
// writes while the selection end-point matches the previous
// one. On release we do a direct copy of the tracked
// selection to guarantee the clipboard is up-to-date.
if (self.config.copy_on_select != .false) {
self.renderer_state.mutex.lock();
defer self.renderer_state.mutex.unlock();
const prev_ = self.io.terminal.screens.active.selection;
if (prev_) |prev| {
try self.setSelection(terminal.Selection.init(
prev.start(),
prev.end(),
prev.rectangle,
));
if (self.io.terminal.screens.active.selection) |sel| {
switch (self.config.copy_on_select) {
.false => unreachable,
.clipboard => try self.copySelectionToClipboards(
sel,
&.{ .standard, .selection },
.mixed,
),
.true => {
const clipboard: apprt.Clipboard = if (self.rt_surface.supportsClipboard(.selection))
.selection
else
.standard;
try self.copySelectionToClipboards(
sel,
&.{clipboard},
.mixed,
);
},
}
}
}
Comment on lines 3849 to 3873
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Clipboard dispatch logic duplicated from setSelection()

The switch (self.config.copy_on_select) block here (lines 3852–3870) is identical to the one in setSelection() (lines 2379–2401). Extracting this into a private helper (e.g., copySelectionToConfiguredClipboards) would eliminate the duplication and make future copy_on_select variant changes a single-site edit.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Valid observation. The two switch blocks are identical. We intentionally kept them duplicated to minimize the fork diff against upstream Ghostty. Extracting a helper would be a clean improvement, but it increases the surface area for merge conflicts when syncing with upstream. If this fix lands upstream, the refactor would be better done there as a follow-up.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The two switch blocks are identical, yeah. We kept them duplicated on purpose to minimize the fork diff against upstream Ghostty. Extracting a helper would be cleaner but increases the surface area for merge conflicts when syncing with upstream. If this fix lands upstream, the refactor would be better done there.


Expand Down