Skip to content

feat: strip shift modifier for link detection and highlighting #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dortort wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: strip shift modifier for link detection and highlighting #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 12 additions & 2 deletions src/Surface.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,10 @@ fn searchCallback_(
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)) {
// Invalidate link point cache so that mouseRefreshLinks
// re-evaluates on the next cursor callback with new mods.
self.mouse.link_point = null;

// 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();
Expand Down Expand Up @@ -4492,8 +4496,14 @@ fn linkAtPos(
// Get our comparison mods
const mouse_mods = self.mouseModsWithCapture(self.mouse.mods);

// For link detection, ignore shift so that Cmd+Shift+Click can also
// activate links (the apprt reads the actual shift state separately
// to choose between built-in and external browser).
var link_mods = mouse_mods;
link_mods.shift = false;

// If we have the proper modifiers set then we can check for OSC8 links.
if (mouse_mods.equal(input.ctrlOrSuper(.{}))) hyperlink: {
if (link_mods.equal(input.ctrlOrSuper(.{}))) hyperlink: {
const rac = mouse_pin.rowAndCell();
const cell = rac.cell;
if (!cell.hyperlink) break :hyperlink;
Expand All @@ -4502,7 +4512,7 @@ fn linkAtPos(
}

// Fall back to configured links
return try self.linkAtPin(mouse_pin, mouse_mods);
return try self.linkAtPin(mouse_pin, link_mods);
}

/// Detects if a link is present at the given pin.
Expand Down
15 changes: 12 additions & 3 deletions src/renderer/generic.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1259,14 +1259,19 @@ pub fn Renderer(comptime GraphicsAPI: type) type {
);
}

// Strip shift for link highlighting — shift toggles browser
// destination in the apprt, not link detection.
var link_mods = state.mouse.mods;
link_mods.shift = false;

// Get our OSC8 links we're hovering if we have a mouse.
// This requires terminal state because of URLs.
const links: terminal.RenderState.CellSet = osc8: {
// If our mouse isn't hovering, we have no links.
const vp = state.mouse.point orelse break :osc8 .empty;

// If the right mods aren't pressed, then we can't match.
if (!state.mouse.mods.equal(inputpkg.ctrlOrSuper(.{})))
if (!link_mods.equal(inputpkg.ctrlOrSuper(.{})))
break :osc8 .empty;

break :osc8 self.terminal_state.linkCells(
Expand Down Expand Up @@ -1296,13 +1301,17 @@ pub fn Renderer(comptime GraphicsAPI: type) type {
};

// Outside the critical area we can update our links to contain
// our regex results.
// our regex results. Strip shift — it toggles browser destination,
// not link detection.
var render_link_mods = state.mouse.mods;
render_link_mods.shift = false;
Comment on lines +1304 to +1307
Copy link

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use the same mouse snapshot as critical.links to avoid mixed-frame link state.

Line 1306 and Line 1313 currently read state.mouse.* after leaving the mutex-protected section. That can combine OSC8 links from one mouse state with regex links from another in the same frame.

Proposed fix 
-            var render_link_mods = state.mouse.mods;
+            var render_link_mods = critical.mouse.mods;
             render_link_mods.shift = false;
@@
-                state.mouse.point,
+                critical.mouse.point,
                 render_link_mods,

Also applies to: 1313-1314

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/generic.zig` around lines 1304 - 1307, The code reads
state.mouse outside the mutex-protected section, causing mixed-frame link state
between OSC8 (critical.links) and regex link detection; fix by taking a single
mouse snapshot while still holding the mutex (the same snapshot used to populate
critical.links) and use that snapshot for render_link_mods and any subsequent
mouse-mod reads instead of re-reading state.mouse (referencing the variables
critical.links and render_link_mods where state.mouse is currently accessed).


self.config.links.renderCellMap(
arena_alloc,
&critical.links,
&self.terminal_state,
state.mouse.point,
state.mouse.mods,
render_link_mods,
) catch |err| {
log.warn("error searching for regex links err={}", .{err});
};
Expand Down