From 6c6b42d40c9330f5cac50e003012e2b4d0749133 Mon Sep 17 00:00:00 2001 From: Matt Robenolt Date: Fri, 26 Jan 2024 20:23:40 -0800 Subject: [PATCH 1/7] Open links with Super+click A few people, including myself, many times accidentally click links by either clicking around aimlessly or getting focus back to Ghostty that happens to be hovering over a link. In iTerm2, if you want links enabled, it's always Cmd+Click. --- src/Surface.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Surface.zig b/src/Surface.zig index c0670c8b8..ecce0a109 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -2008,7 +2008,7 @@ pub fn mouseButtonCallback( // Handle link clicking. We want to do this before we do mouse // reporting or any other mouse handling because a successfully // clicked link will swallow the event. - if (button == .left and action == .release and self.mouse.over_link) { + if (button == .left and action == .release and mods.super and self.mouse.over_link) { const pos = try self.rt_surface.getCursorPos(); if (self.processLinks(pos)) |processed| { if (processed) return; From e70ec5d5f4bbd788e13b4dfb6548a981fd855d2d Mon Sep 17 00:00:00 2001 From: Matt Robenolt Date: Fri, 26 Jan 2024 20:52:00 -0800 Subject: [PATCH 2/7] Only detect links when Super is held down This stops underlining and changing to a pointer unless Cmd or Ctrl is held down already. --- src/Surface.zig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Surface.zig b/src/Surface.zig index ecce0a109..c8fb2448e 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -2008,7 +2008,7 @@ pub fn mouseButtonCallback( // Handle link clicking. We want to do this before we do mouse // reporting or any other mouse handling because a successfully // clicked link will swallow the event. - if (button == .left and action == .release and mods.super and self.mouse.over_link) { + if (button == .left and action == .release and mods.ctrlOrSuper() and self.mouse.over_link) { const pos = try self.rt_surface.getCursorPos(); if (self.processLinks(pos)) |processed| { if (processed) return; @@ -2238,6 +2238,9 @@ fn linkAtPos( // If we have no configured links we can save a lot of work if (self.config.links.len == 0) return null; + // Require super to be held down, so bail early + if (!self.mouse.mods.ctrlOrSuper()) return null; + // Convert our cursor position to a screen point. const mouse_pt = mouse_pt: { const viewport_point = self.posToViewport(pos.x, pos.y); From 58134886915b1fe460da75a4eebc69f8edacfed4 Mon Sep 17 00:00:00 2001 From: Matt Robenolt Date: Fri, 26 Jan 2024 20:56:34 -0800 Subject: [PATCH 3/7] Update Config docs to suggest Super + hover is required for link activation --- src/config/Config.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 0c0d3325c..e9198827d 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -432,8 +432,8 @@ command: ?[]const u8 = null, /// TODO: This can't currently be set! link: RepeatableLink = .{}, -/// Enable URL matching. URLs are matched on hover and open using the default -/// system application for the linked URL. +/// Enable URL matching. URLs are matched on `super` + hover and open using the +/// default system application for the linked URL. /// /// The URL matcher is always lowest priority of any configured links (see /// `link`). If you want to customize URL matching, use `link` and disable this. From ae11cc9042f14de484082b0bee35e8c767ab4e19 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 27 Jan 2024 19:03:03 -0800 Subject: [PATCH 4/7] add a new highlight state that requires modifiers --- src/Surface.zig | 49 ++++++++++++++++++++++++++++++----- src/config/Config.zig | 2 +- src/input/Link.zig | 5 ++++ src/renderer/Metal.zig | 1 + src/renderer/State.zig | 6 +++++ src/renderer/link.zig | 58 +++++++++++++++++++++++++++++++++++++----- 6 files changed, 107 insertions(+), 14 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index c8fb2448e..7b2e21be6 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -198,6 +198,7 @@ const DerivedConfig = struct { const Link = struct { regex: oni.Regex, action: input.Link.Action, + highlight: input.Link.Highlight, }; pub fn init(alloc_gpa: Allocator, config: *const configpkg.Config) !DerivedConfig { @@ -215,6 +216,7 @@ const DerivedConfig = struct { try links.append(.{ .regex = regex, .action = link.action, + .highlight = link.highlight, }); } @@ -814,6 +816,35 @@ pub fn handleMessage(self: *Surface, msg: Message) !void { } } +/// Call this when modifiers change. This is safe to call even if modifiers +/// match the previous state. +/// +/// This is not publicly exported because modifier changes happen implicitly +/// on mouse callbacks, key callbacks, etc. +/// +/// The renderer state mutex MUST NOT be held. +fn modsChanged(self: *Surface, mods: input.Mods) void { + // The only place we keep track of mods currently is on the mouse. + if (!self.mouse.mods.equal(mods)) { + // The mouse mods only contain binding modifiers since we don't + // want caps/num lock or sided modifiers to affect the mouse. + self.mouse.mods = mods.binding(); + + // We also need to update the renderer so it knows if it + // should highlight links. + { + self.renderer_state.mutex.lock(); + defer self.renderer_state.mutex.unlock(); + self.renderer_state.mouse.mods = mods; + } + + self.queueRender() catch |err| { + // Not a big deal if this fails. + log.warn("failed to notify renderer of mods change err={}", .{err}); + }; + } +} + /// Called when our renderer health state changes. fn updateRendererHealth(self: *Surface, health: renderer.Health) void { log.warn("renderer health status change status={}", .{health}); @@ -1352,10 +1383,12 @@ pub fn keyCallback( // to hide it again if it was hidden. const rehide = self.mouse.hidden; + // Update our modifiers, this will update mouse mods too + self.modsChanged(event.mods); + // We set this to null to force link reprocessing since // mod changes can affect link highlighting. self.mouse.link_point = null; - self.mouse.mods = event.mods; const pos = self.rt_surface.getCursorPos() catch break :mouse_mods; self.cursorPosCallback(pos) catch {}; if (rehide) self.hideMouse(); @@ -1969,11 +2002,13 @@ pub fn mouseButtonCallback( // Always record our latest mouse state self.mouse.click_state[@intCast(@intFromEnum(button))] = action; - self.mouse.mods = @bitCast(mods); // Always show the mouse again if it is hidden if (self.mouse.hidden) self.showMouse(); + // Update our modifiers if they changed + self.modsChanged(mods); + // This is set to true if the terminal is allowed to capture the shift // modifer. Note we can do this more efficiently probably with less // locking/unlocking but clicking isn't that frequent enough to be a @@ -2008,7 +2043,7 @@ pub fn mouseButtonCallback( // Handle link clicking. We want to do this before we do mouse // reporting or any other mouse handling because a successfully // clicked link will swallow the event. - if (button == .left and action == .release and mods.ctrlOrSuper() and self.mouse.over_link) { + if (button == .left and action == .release and self.mouse.over_link) { const pos = try self.rt_surface.getCursorPos(); if (self.processLinks(pos)) |processed| { if (processed) return; @@ -2238,9 +2273,6 @@ fn linkAtPos( // If we have no configured links we can save a lot of work if (self.config.links.len == 0) return null; - // Require super to be held down, so bail early - if (!self.mouse.mods.ctrlOrSuper()) return null; - // Convert our cursor position to a screen point. const mouse_pt = mouse_pt: { const viewport_point = self.posToViewport(pos.x, pos.y); @@ -2255,6 +2287,11 @@ fn linkAtPos( // Go through each link and see if we clicked it for (self.config.links) |link| { + switch (link.highlight) { + .always, .hover => {}, + .mods => |v| if (!v.equal(self.mouse.mods)) continue, + } + var it = strmap.searchIterator(link.regex); while (true) { var match = (try it.next()) orelse break; diff --git a/src/config/Config.zig b/src/config/Config.zig index e9198827d..3c5c377c3 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1437,7 +1437,7 @@ pub fn default(alloc_gpa: Allocator) Allocator.Error!Config { try result.link.links.append(alloc, .{ .regex = url.regex, .action = .{ .open = {} }, - .highlight = .{ .hover = {} }, + .highlight = .{ .mods = inputpkg.ctrlOrSuper(.{}) }, }); return result; diff --git a/src/input/Link.zig b/src/input/Link.zig index 55390d4be..bc0aa2265 100644 --- a/src/input/Link.zig +++ b/src/input/Link.zig @@ -5,6 +5,7 @@ const Link = @This(); const oni = @import("oniguruma"); +const Mods = @import("key.zig").Mods; /// The regular expression that will be used to match the link. Ownership /// of this memory is up to the caller. The link will never free this memory. @@ -30,6 +31,10 @@ pub const Highlight = union(enum) { /// Only highlight the link when the mouse is hovering over it. hover: void, + + /// Highlight anytime the given mods are pressed, regardless of + /// hover state. + mods: Mods, }; /// Returns a new oni.Regex that can be used to match the link. diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index 96d5b11d8..3ef7d7869 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -1555,6 +1555,7 @@ fn rebuildCells( arena_alloc, screen, mouse.point orelse .{}, + mouse.mods, ); // Determine our x/y range for preedit. We don't want to render anything diff --git a/src/renderer/State.zig b/src/renderer/State.zig index 9fd7fa5b2..6c69d30a9 100644 --- a/src/renderer/State.zig +++ b/src/renderer/State.zig @@ -4,6 +4,7 @@ const std = @import("std"); const Allocator = std.mem.Allocator; const Inspector = @import("../inspector/main.zig").Inspector; const terminal = @import("../terminal/main.zig"); +const inputpkg = @import("../input.zig"); const renderer = @import("../renderer.zig"); /// The mutex that must be held while reading any of the data in the @@ -34,6 +35,11 @@ pub const Mouse = struct { /// viewport points to avoid the complexity of mapping the mouse to /// the renderer state. point: ?terminal.point.Viewport = null, + + /// The mods that are currently active for the last mouse event. + /// This could really just be mods in general and we probably will + /// move it out of mouse state at some point. + mods: inputpkg.Mods = .{}, }; /// The pre-edit state. See Surface.preeditCallback for more information. diff --git a/src/renderer/link.zig b/src/renderer/link.zig index 0537aef6d..27d396bd2 100644 --- a/src/renderer/link.zig +++ b/src/renderer/link.zig @@ -65,6 +65,7 @@ pub const Set = struct { alloc: Allocator, screen: *Screen, mouse_vp_pt: point.Viewport, + mouse_mods: inputpkg.Mods, ) !MatchSet { // Convert the viewport point to a screen point. const mouse_pt = mouse_vp_pt.toScreen(screen); @@ -90,10 +91,13 @@ pub const Set = struct { // Go through each link and see if we have any matches. for (self.links) |link| { - // If this is a hover link and our mouse point isn't within - // this line at all, we can skip it. - if (link.highlight == .hover) { - if (!line.selection().contains(mouse_pt)) continue; + // Determine if our highlight conditions are met. We use a + // switch here instead of an if so that we can get a compile + // error if any other conditions are added. + switch (link.highlight) { + .always => {}, + .hover => if (!line.selection().contains(mouse_pt)) continue, + .mods => |v| if (!mouse_mods.equal(v)) continue, } var it = strmap.searchIterator(link.regex); @@ -186,7 +190,7 @@ test "matchset" { defer set.deinit(alloc); // Get our matches - var match = try set.matchSet(alloc, &s, .{}); + var match = try set.matchSet(alloc, &s, .{}, .{}); defer match.deinit(alloc); try testing.expectEqual(@as(usize, 2), match.matches.len); @@ -227,7 +231,7 @@ test "matchset hover links" { // Not hovering over the first link { - var match = try set.matchSet(alloc, &s, .{}); + var match = try set.matchSet(alloc, &s, .{}, .{}); defer match.deinit(alloc); try testing.expectEqual(@as(usize, 1), match.matches.len); @@ -242,7 +246,7 @@ test "matchset hover links" { // Hovering over the first link { - var match = try set.matchSet(alloc, &s, .{ .x = 1, .y = 0 }); + var match = try set.matchSet(alloc, &s, .{ .x = 1, .y = 0 }, .{}); defer match.deinit(alloc); try testing.expectEqual(@as(usize, 2), match.matches.len); @@ -255,3 +259,43 @@ test "matchset hover links" { try testing.expect(!match.orderedContains(.{ .x = 1, .y = 2 })); } } + +test "matchset mods no match" { + const testing = std.testing; + const alloc = testing.allocator; + + // Initialize our screen + var s = try Screen.init(alloc, 5, 5, 0); + defer s.deinit(); + const str = "1ABCD2EFGH\n3IJKL"; + try s.testWriteString(str); + + // Get a set + var set = try Set.fromConfig(alloc, &.{ + .{ + .regex = "AB", + .action = .{ .open = {} }, + .highlight = .{ .always = {} }, + }, + + .{ + .regex = "EF", + .action = .{ .open = {} }, + .highlight = .{ .mods = .{ .ctrl = true } }, + }, + }); + defer set.deinit(alloc); + + // Get our matches + var match = try set.matchSet(alloc, &s, .{}, .{}); + defer match.deinit(alloc); + try testing.expectEqual(@as(usize, 1), match.matches.len); + + // Test our matches + try testing.expect(!match.orderedContains(.{ .x = 0, .y = 0 })); + try testing.expect(match.orderedContains(.{ .x = 1, .y = 0 })); + try testing.expect(match.orderedContains(.{ .x = 2, .y = 0 })); + try testing.expect(!match.orderedContains(.{ .x = 3, .y = 0 })); + try testing.expect(!match.orderedContains(.{ .x = 1, .y = 1 })); + try testing.expect(!match.orderedContains(.{ .x = 1, .y = 2 })); +} From 3efe88c85c63177af9c1df7a16289efff18d5340 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 27 Jan 2024 19:07:49 -0800 Subject: [PATCH 5/7] input: add link highlight always/hover w/ mods --- src/Surface.zig | 2 +- src/config/Config.zig | 2 +- src/input/Link.zig | 9 ++++++--- src/renderer/OpenGL.zig | 1 + src/renderer/link.zig | 9 +++++++-- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 7b2e21be6..c3e6c592c 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -2289,7 +2289,7 @@ fn linkAtPos( for (self.config.links) |link| { switch (link.highlight) { .always, .hover => {}, - .mods => |v| if (!v.equal(self.mouse.mods)) continue, + .always_mods, .hover_mods => |v| if (!v.equal(self.mouse.mods)) continue, } var it = strmap.searchIterator(link.regex); diff --git a/src/config/Config.zig b/src/config/Config.zig index 3c5c377c3..5e09bf247 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1437,7 +1437,7 @@ pub fn default(alloc_gpa: Allocator) Allocator.Error!Config { try result.link.links.append(alloc, .{ .regex = url.regex, .action = .{ .open = {} }, - .highlight = .{ .mods = inputpkg.ctrlOrSuper(.{}) }, + .highlight = .{ .hover_mods = inputpkg.ctrlOrSuper(.{}) }, }); return result; diff --git a/src/input/Link.zig b/src/input/Link.zig index bc0aa2265..cdd87ef02 100644 --- a/src/input/Link.zig +++ b/src/input/Link.zig @@ -32,9 +32,12 @@ pub const Highlight = union(enum) { /// Only highlight the link when the mouse is hovering over it. hover: void, - /// Highlight anytime the given mods are pressed, regardless of - /// hover state. - mods: Mods, + /// Highlight anytime the given mods are pressed, either when + /// hovering or always. For always, all links will be highlighted + /// when the mods are pressed regardless of if the mouse is hovering + /// over them. + always_mods: Mods, + hover_mods: Mods, }; /// Returns a new oni.Regex that can be used to match the link. diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index 643b64bbf..a3e0721ec 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -991,6 +991,7 @@ pub fn rebuildCells( arena_alloc, screen, mouse.point orelse .{}, + mouse.mods, ); // Determine our x/y range for preedit. We don't want to render anything diff --git a/src/renderer/link.zig b/src/renderer/link.zig index 27d396bd2..dda3e726f 100644 --- a/src/renderer/link.zig +++ b/src/renderer/link.zig @@ -96,8 +96,13 @@ pub const Set = struct { // error if any other conditions are added. switch (link.highlight) { .always => {}, - .hover => if (!line.selection().contains(mouse_pt)) continue, - .mods => |v| if (!mouse_mods.equal(v)) continue, + .always_mods => |v| if (!mouse_mods.equal(v)) continue, + inline .hover, .hover_mods => |v, tag| { + if (!line.selection().contains(mouse_pt)) continue; + if (comptime tag == .hover_mods) { + if (!mouse_mods.equal(v)) continue; + } + }, } var it = strmap.searchIterator(link.regex); From 9beb395b121fbf77efe6bf4a3bd2f473638c6557 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 27 Jan 2024 19:09:03 -0800 Subject: [PATCH 6/7] config: update docs --- src/config/Config.zig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 5e09bf247..49ca25d49 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -432,8 +432,9 @@ command: ?[]const u8 = null, /// TODO: This can't currently be set! link: RepeatableLink = .{}, -/// Enable URL matching. URLs are matched on `super` + hover and open using the -/// default system application for the linked URL. +/// Enable URL matching. URLs are matched on hover with control (Linux) or +/// super (macOS) pressed and open using the default system application for +/// the linked URL. /// /// The URL matcher is always lowest priority of any configured links (see /// `link`). If you want to customize URL matching, use `link` and disable this. From 75ca29da59241da1f2cb50f6667ab9a8b931de04 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 27 Jan 2024 19:34:05 -0800 Subject: [PATCH 7/7] renderer/link: fix test --- src/renderer/link.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/link.zig b/src/renderer/link.zig index dda3e726f..574d18e56 100644 --- a/src/renderer/link.zig +++ b/src/renderer/link.zig @@ -286,7 +286,7 @@ test "matchset mods no match" { .{ .regex = "EF", .action = .{ .open = {} }, - .highlight = .{ .mods = .{ .ctrl = true } }, + .highlight = .{ .always_mods = .{ .ctrl = true } }, }, }); defer set.deinit(alloc);