From 66f2d75ddd6709d6c4ce9a5527f59506d0b2550d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 1 Oct 2024 16:30:01 -0700 Subject: [PATCH] Change copy-on-select behavior to be more idiomatic for Linux Fixes #2345 The new docs for `copy-on-select`: Whether to automatically copy selected text to the clipboard. `true` will prefer to copy to the selection clipboard if supported by the OS, otherwise it will copy to the system clipboard. The value `clipboard` will always copy text to the system clipboard (for supported systems) as well as the system clipboard. This is sometimes a preferred behavior on Linux. Middle-click paste will always use the selection clipboard on Linux and the system clipboard on macOS. Middle-click paste is always enabled even if this is `false`. The default value is true on Linux and false on macOS. macOS copy on select behavior is not typical for applications so it is disabled by default. On Linux, this is a standard behavior so it is enabled by default. --- src/Surface.zig | 75 ++++++++++++++++++++++++++----------------- src/config/Config.zig | 33 +++++++++++++------ 2 files changed, 69 insertions(+), 39 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index e8bbb885ff..f952a88dfc 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1176,12 +1176,8 @@ fn setSelection(self: *Surface, sel_: ?terminal.Selection) !void { const prev_ = self.io.terminal.screen.selection; try self.io.terminal.screen.select(sel_); - // Determine the clipboard we want to copy selection to, if it is enabled. - const clipboard: apprt.Clipboard = switch (self.config.copy_on_select) { - .false => return, - .true => .selection, - .clipboard => .standard, - }; + // If copy on select is false then exit early. + if (self.config.copy_on_select == .false) return; // Set our selection clipboard. If the selection is cleared we do not // clear the clipboard. If the selection is set, we only set the clipboard @@ -1190,12 +1186,6 @@ fn setSelection(self: *Surface, sel_: ?terminal.Selection) !void { const sel = sel_ orelse return; if (prev_) |prev| if (sel.eql(prev)) return; - // Check if our runtime supports the selection clipboard at all. - // We can save a lot of work if it doesn't. - if (!self.rt_surface.supportsClipboard(clipboard)) { - return; - } - const buf = self.io.terminal.screen.selectionString(self.alloc, .{ .sel = sel, .trim = self.config.clipboard_trim_trailing_spaces, @@ -1205,10 +1195,45 @@ fn setSelection(self: *Surface, sel_: ?terminal.Selection) !void { }; defer self.alloc.free(buf); - self.rt_surface.setClipboardString(buf, clipboard, false) catch |err| { - log.err("error setting clipboard string err={}", .{err}); - return; - }; + // Set the clipboard. This is not super DRY but it is clear what + // we're doing for each setting without being clever. + switch (self.config.copy_on_select) { + .false => unreachable, // handled above with an early exit + + // Both standard and selection clipboards are set. + .clipboard => { + const clipboards: []const apprt.Clipboard = &.{ .standard, .selection }; + for (clipboards) |clipboard| self.rt_surface.setClipboardString( + buf, + clipboard, + false, + ) catch |err| { + log.err( + "error setting clipboard string clipboard={} err={}", + .{ clipboard, err }, + ); + }; + }, + + // The selection clipboard is set if supported, otherwise the standard. + .true => { + const clipboard: apprt.Clipboard = if (self.rt_surface.supportsClipboard(.selection)) + .selection + else + .standard; + + self.rt_surface.setClipboardString( + buf, + clipboard, + false, + ) catch |err| { + log.err( + "error setting clipboard string clipboard={} err={}", + .{ clipboard, err }, + ); + }; + }, + } } /// Change the cell size for the terminal grid. This can happen as @@ -2680,19 +2705,11 @@ pub fn mouseButtonCallback( // Middle-click pastes from our selection clipboard if (button == .middle and action == .press) { - if (comptime builtin.target.isDarwin()) { - // Fast-path for MacOS - always paste from clipboard on - // middle-click. - try self.startClipboardRequest(.standard, .{ .paste = {} }); - } else if (self.config.copy_on_select != .false) { - const clipboard: apprt.Clipboard = switch (self.config.copy_on_select) { - .true => .selection, - .clipboard => .standard, - .false => unreachable, - }; - - try self.startClipboardRequest(clipboard, .{ .paste = {} }); - } + const clipboard: apprt.Clipboard = if (self.rt_surface.supportsClipboard(.selection)) + .selection + else + .standard; + try self.startClipboardRequest(clipboard, .{ .paste = {} }); } // Right-click down selects word for context menus. If the apprt diff --git a/src/config/Config.zig b/src/config/Config.zig index 35156dc186..eefd3703f8 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1114,15 +1114,27 @@ keybind: Keybinds = .{}, /// limit per surface is double. @"image-storage-limit": u32 = 320 * 1000 * 1000, -/// Whether to automatically copy selected text to the clipboard. `true` will -/// only copy on systems that support a selection clipboard. -/// -/// The value `clipboard` will copy to the system clipboard, making this work on -/// macOS. Note that middle-click will also paste from the system clipboard in -/// this case. -/// -/// Note that if this is disabled, middle-click paste will also be disabled. -@"copy-on-select": CopyOnSelect = .true, +/// Whether to automatically copy selected text to the clipboard. `true` +/// will prefer to copy to the selection clipboard if supported by the +/// OS, otherwise it will copy to the system clipboard. +/// +/// The value `clipboard` will always copy text to the selection clipboard +/// (for supported systems) as well as the system clipboard. This is sometimes +/// a preferred behavior on Linux. +/// +/// Middle-click paste will always use the selection clipboard on Linux +/// and the system clipboard on macOS. Middle-click paste is always enabled +/// even if this is `false`. +/// +/// The default value is true on Linux and false on macOS. macOS copy on +/// select behavior is not typical for applications so it is disabled by +/// default. On Linux, this is a standard behavior so it is enabled by +/// default. +@"copy-on-select": CopyOnSelect = switch (builtin.os.tag) { + .linux => .true, + .macos => .false, + else => .false, +}, /// The time in milliseconds between clicks to consider a click a repeat /// (double, triple, etc.) or an entirely new single click. A value of zero will @@ -4326,7 +4338,8 @@ pub const CopyOnSelect = enum { /// This is not supported on platforms such as macOS. This is the default. true, - /// Copy on select is enabled and goes to the system clipboard. + /// Copy on select is enabled and goes to both the system clipboard + /// and the selection clipboard (for Linux). clipboard, };