Skip to content

fix(desktop): restore cmd+click for v1 terminal file links#3457

Merged
Kitenite merged 1 commit into
mainfrom
garrulous-ricotta
Apr 14, 2026
Merged

fix(desktop): restore cmd+click for v1 terminal file links#3457
Kitenite merged 1 commit into
mainfrom
garrulous-ricotta

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Apr 14, 2026

Summary

  • v1 terminal file-path links reverted to opening on plain click after Port link detection v2 to v1 #3382 ported v1 to the shared TerminalLinkManager. The old FilePathLinkProvider required cmd/ctrl; the shared LinkDetectorAdapter (also used by v2) activates on plain click.
  • Gate onFileLinkClick behind metaKey/ctrlKey in v1's helpers.ts so v1 requires the modifier again without changing v2 behavior.
  • Web URLs were already correctly gated in UrlLinkProvider; this fix aligns file-path links with that.

Test plan

  • In v1 terminal, plain-click a file path in output — nothing happens.
  • Cmd/Ctrl+click a file path — opens in file viewer / external editor per setting.
  • Cmd/Ctrl+click a URL — still opens in browser.
  • v2 terminal file-link click behavior unchanged.

Summary by cubic

Restore Cmd/Ctrl+click requirement for file-path links in the v1 terminal; plain clicks no longer open files. URL clicks and v2 terminal behavior are unchanged.

Written for commit 7c67db1. Summary will update on new commits.

Summary by CodeRabbit

  • Terminal
    • File links in the Terminal now require pressing a modifier key (Ctrl or Cmd) to open.

Gate `onFileLinkClick` behind metaKey/ctrlKey in v1 helpers so file-path
links once again require cmd/ctrl+click, matching pre-#3382 behavior.
The shared `LinkDetectorAdapter` (used by v2) still activates on plain
click and is untouched.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 14, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3c7baafb-ca64-4fe0-9a8f-d7b4f53b691d

📥 Commits

Reviewing files that changed from the base of the PR and between 47efa73 and 7c67db1.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts

📝 Walkthrough

Walkthrough

Added a modifier key requirement to terminal file link click handling. The onFileLinkClick handler now returns early unless the mouse event includes either metaKey or ctrlKey, preventing unintended file link activation from regular clicks.

Changes

Cohort / File(s) Summary
Terminal Link Click Handler
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
Added early return condition to onFileLinkClick that short-circuits processing unless event.metaKey or event.ctrlKey is active, requiring modifier key combinations to trigger file link navigation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A click without a key won't do,
Mod+Click's the rabbit's trick so true,
Links stay put till fingers dance,
With Ctrl or Cmd's commanding glance! 🖱️✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch garrulous-ricotta

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Kitenite Kitenite merged commit a3e34bf into main Apr 14, 2026
6 of 7 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch
  • ⚠️ Electric Fly.io app

Thank you for your contribution! 🎉

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 14, 2026

Greptile Summary

This PR fixes a regression introduced in #3382, where v1 terminal file-path links began opening on plain click after being ported to the shared TerminalLinkManager. The LinkDetectorAdapter (shared by both v1 and v2) activates links on any click, unlike the old FilePathLinkProvider which required Cmd/Ctrl. The fix adds a three-line metaKey/ctrlKey guard at the top of the v1 onFileLinkClick handler in helpers.ts, restoring the modifier-key requirement without touching v2 behavior.

  • Adds if (!event.metaKey && !event.ctrlKey) { return; } guard to onFileLinkClick in helpers.ts (v1 terminal setup)
  • Aligns v1 file-link behavior with UrlLinkProvider.handleActivation and the v2 TerminalPane.tsx handler, both of which already gate on the same modifier keys
  • The WordLinkDetector path (bare filenames like AGENTS.md) also flows through onFileLinkClick, so it correctly inherits the modifier-key requirement as well
  • v2 terminal behavior is unaffected — its own onFileLinkClick in TerminalPane.tsx already had the same guard independently

Confidence Score: 5/5

  • Safe to merge — the fix is a minimal, targeted guard that correctly restores the intended v1 behavior without touching v2.
  • The change is three lines, matches the established pattern in both v2 (TerminalPane.tsx) and UrlLinkProvider, and directly addresses the regression. The only note is a missing event.preventDefault() for consistency, but it is unlikely to cause a practical problem in the Electron xterm canvas context and does not affect correctness of the primary fix.
  • No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts Adds a metaKey/ctrlKey guard to onFileLinkClick; functionally correct and consistent with v2 and UrlLinkProvider, but is missing event.preventDefault() that both peer implementations call after the guard passes.

Sequence Diagram

sequenceDiagram
    participant User
    participant xterm
    participant LinkDetectorAdapter
    participant onFileLinkClick as onFileLinkClick (helpers.ts)
    participant v1Handler as v1 file-open action

    User->>xterm: plain click on file path
    xterm->>LinkDetectorAdapter: activate(event, detectedLink)
    LinkDetectorAdapter->>onFileLinkClick: onFileLinkClick(event, link)
    onFileLinkClick->>onFileLinkClick: "metaKey || ctrlKey?"
    Note over onFileLinkClick: NO → early return (fix)
    onFileLinkClick-->>LinkDetectorAdapter: (no-op)

    User->>xterm: Cmd/Ctrl+click on file path
    xterm->>LinkDetectorAdapter: activate(event, detectedLink)
    LinkDetectorAdapter->>onFileLinkClick: onFileLinkClick(event, link)
    onFileLinkClick->>onFileLinkClick: "metaKey || ctrlKey?"
    Note over onFileLinkClick: YES → proceed
    onFileLinkClick->>v1Handler: openFileInEditor.mutate(...)
    v1Handler-->>User: file opens in editor
Loading

Comments Outside Diff (1)

  1. apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts, line 171-178 (link)

    P2 Missing event.preventDefault() after guard

    Both peer implementations call event.preventDefault() after the modifier-key guard passes, before performing the action:

    • UrlLinkProvider.handleActivation (line 350 of url-link-provider.ts): calls event.preventDefault() before this.onOpen(...).
    • v2 TerminalPane.tsx (line 146): calls _event.preventDefault() before openFileInEditor.mutate(...).

    Without it, Cmd+Click (macOS) or Ctrl+Click (Windows) can still bubble through xterm's event handling and trigger Electron/Chromium defaults (e.g., Cmd+Click on a canvas in Electron may be interpreted by the OS or the webContents in unexpected ways). Aligning with the pattern used by both sibling implementations would be safer:

Reviews (1): Last reviewed commit: "fix(desktop): restore cmd+click requirem..." | Re-trigger Greptile

MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request Apr 15, 2026
superset-sh#3457)

Gate `onFileLinkClick` behind metaKey/ctrlKey in v1 helpers so file-path
links once again require cmd/ctrl+click, matching pre-superset-sh#3382 behavior.
The shared `LinkDetectorAdapter` (used by v2) still activates on plain
click and is untouched.
MocA-Love added a commit to MocA-Love/superset that referenced this pull request Apr 15, 2026
MocA-Love added a commit to MocA-Love/superset that referenced this pull request Apr 15, 2026
-s ours merge to record that upstream commits a3e34bf through
de70163 (13 commits) are semantically already present on origin/main
via the PR1-6 cherry-pick series (PRs #176, #177, #178, #179, #180,
#182), plus fork-adaptation fixes layered on top.

This merge target is de70163 specifically (not upstream/main) so
newer upstream commits (9fff075 and later) remain visible in future
behind counts.

Upstream commits covered by this audit merge:
- a3e34bf  fix(desktop): restore cmd+click requirement for v1 terminal file links (superset-sh#3457)  [PR1/#176]
- 57557f8  fix(desktop): gate v2 workspace children on collection readiness (superset-sh#3464)       [PR1/#176]
- 4ee2e61  fix(desktop): use native clipboard for copy path in v2 sidebar (superset-sh#3462)         [PR1/#176]
- 87d6e93  feat(desktop): close settings with Escape key (superset-sh#3466)                          [PR1/#176]
- 9c7f5f4  chore(desktop): auto-restart host-service on bundle change in dev (superset-sh#3461)      [PR1/#176]
- 93140d9  fix(mcp): accept MCP resource URL as valid OAuth audience (superset-sh#3459)              [PR2/#177]
- be9e000  fix(desktop): drive tray menu off events, fetch real org name (superset-sh#3458)          [PR2/#177]
- c5f791e  feat(v2): unify workspace delete through host-service (superset-sh#3443)                  [PR3/#178]
- 2c24d93  feat(desktop): paginated branch picker with checkout + open actions (superset-sh#3397)    [PR4/#179]
- 2bf1049  feat(desktop/hotkeys): v1 directional pane focus + best-effort v1 override migrator (superset-sh#3460)  [PR5/#180]
- 1294a7d  feat(desktop/hotkeys): restore Cmd+Alt+Arrow for tab/workspace nav (superset-sh#3472)    [PR5/#180]
- de70163  feat(desktop): v2 review tab first pass — PR info, checks, comments (superset-sh#3463)    [PR6/#182]

Intentionally skipped (version bump, fork has independent versioning):
- 1e23353  chore(desktop): bump version to 1.5.5 (superset-sh#3473)

Fork-adaptation fixes layered on top of the cherry-picks:
- PR1: host-service-coordinator alias import fix, settings Escape
       selector narrowing (role-based + popper wrapper), Escape
       close uses replace navigation
- PR2: dual quit mode preservation (requestQuit "release"/"stop"),
       trayUpdateToken guard for stale async fetchHostInfo results
- PR4: ChangesHeader.normalizeBranchName regex rewrite (lint false
       positive), worktree add uses fullRef for remote-tracking
       refs, syncTimedOut reset on pendingId change, GIT_REFS.md
       barrel example fix
- PR5: migrate.ts re-sanitize of existing localStorage overrides
       (v2 marker bump intent), FOCUS_PANE_* enabled:isActive for
       KeepAliveWorkspaces, CATEGORY_ORDER merges Navigation (upstream)
       and Browser (fork)
- PR6: normalizeThreadsToComments flattens all thread.comments (not
       just first), CommentPane overrides <a> (openUrl) and <img>
       (SafeImage), zero-badge suppression, pr-null comments gate

Fork features verified intact (Explore agent audit of combined
36d4de4..35d95f3 range):
- BROWSER_RELOAD / BROWSER_HARD_RELOAD hotkeys
- dual quit mode menu in tray
- v1 terminal cold-restore + retry reconnect (out of range but
  unaffected)
- KeepAliveWorkspaces (FOCUS_PANE_* gated on isActive)
- useCommandPalette + addMemoTab in v2 workspace
- host-service-coordinator rename alias pattern
@Kitenite Kitenite deleted the garrulous-ricotta branch May 6, 2026 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant