Repository navigation
feat(terminal): configure URL click and middle-click behavior - #21438
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds configurable plain-click and middle-click behavior for terminal links. Users can show actions, open links directly, or do nothing. New settings preserve compatibility with the legacy popover setting. Terminal link detection, mouse suppression, terminal-pane routing, and native-chat handling now use the resolved behavior. Tests cover behavior resolution and direct plain-click and middle-click routing. Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to Several configured link-routing modes do not honor the selected behavior: middle-click actions fail, older profiles cannot re-enable actions, and native-chat modifier-only mode can still open links. These user-facing routing regressions should be fixed before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation Issue ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 783b036e-987e-42b6-b298-a71fd918b7e3
📒 Files selected for processing (14)
src/renderer/src/components/native-chat/native-chat-web-link-actions.tssrc/renderer/src/components/native-chat/use-native-chat-link-actions.tssrc/renderer/src/components/settings/BrowserTerminalLinkActionsSetting.tsxsrc/renderer/src/components/settings/browser-search.tssrc/renderer/src/components/terminal-pane/terminal-link-action-request.tssrc/renderer/src/components/terminal-pane/terminal-link-action-routing.test.tssrc/renderer/src/components/terminal-pane/terminal-link-activation.tssrc/renderer/src/components/terminal-pane/terminal-link-click-behavior.test.tssrc/renderer/src/components/terminal-pane/terminal-link-click-behavior.tssrc/renderer/src/components/terminal-pane/terminal-link-pty-mouse-suppression.tssrc/renderer/src/components/terminal-pane/terminal-pane-mount-preparation.tssrc/renderer/src/components/terminal-pane/terminal-url-link-hit-testing.tssrc/shared/default-global-settings.tssrc/shared/global-settings-types.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.
| if ( | ||
| (!isTerminalLinkDirectActivation(event) && !isTerminalMiddleClickActivation(event)) || | ||
| !shouldSuppressMouseEvent(event) | ||
| ) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,180p' src/renderer/src/components/terminal-pane/terminal-link-pty-mouse-suppression.ts
sed -n '1,130p' src/renderer/src/components/terminal-pane/terminal-link-action-request.ts
rg -n 'claimAction|deferredPtyInput|handleMouseDown|requestTerminalLinkAction' src/renderer/src/components/terminal-paneRepository: stablyai/orca
Length of output: 15493
🏁 Script executed:
sed -n '110,180p' src/renderer/src/components/terminal-pane/terminal-pane-mount-preparation.ts
sed -n '1,230p' src/renderer/src/components/terminal-pane/terminal-pane-mount-preparation.ts
sed -n '1,130p' src/renderer/src/components/terminal-pane/terminal-link-activation.ts
sed -n '1,120p' src/renderer/src/components/terminal-pane/terminal-link-pointer-gesture.ts
rg -n -C 8 'installTerminalLinkPtyMouseSuppression|shouldDeferPlainMouseEvent|shouldSuppressMouseEvent|middleClickBehavior|claimPtyMouse' src/renderer/src/components/terminal-paneRepository: stablyai/orca
Length of output: 50369
🏁 Script executed:
cat -n src/renderer/src/components/terminal-pane/terminal-pane-mount-preparation.ts | sed -n '1,180p'
cat -n src/renderer/src/components/terminal-pane/terminal-link-activation.ts | sed -n '1,120p'
rg -n -C 12 'installTerminalLinkPtyMouseSuppression|shouldDeferPlainMouseEvent|shouldSuppressMouseEvent|middleClickBehavior|claimPtyMouse' src/renderer/src/components/terminal-paneRepository: stablyai/orca
Length of output: 50369
🏁 Script executed:
cat -n src/renderer/src/components/terminal-pane/terminal-url-link-hit-testing.ts | sed -n '120,220p'
cat -n src/renderer/src/components/terminal-pane/terminal-link-pty-mouse-suppression.ts | sed -n '150,225p'
rg -n -C 8 'handlePtyInput|ptyMouseSuppression|forward\\(' src/renderer/src/components/terminal-pane src/renderer/src/components/terminal-pane/terminal-url-link-hit-testing.tsRepository: stablyai/orca
Length of output: 6239
Initialize and gate the middle-click claim.
For a link middle click, handleMouseDown enables suppression but does not initialize deferredPtyInput. Therefore, claimAction() returns false, and requestTerminalLinkAction() exits before opening the URL or showing actions.
Initialize the claim when middleClickBehavior is open or actions. When it is none, bypass the suppression and deferred-claim path so handlePtyInput forwards PTY input normally.
| const getLinkActionContext = (paneId: number): TerminalLinkActionContext | null => { | ||
| if (deps.settingsRef.current?.terminalLinkActionPopoverEnabled === false) { | ||
| const plainClickBehavior = terminalLinkClickBehaviorFor(deps.settingsRef.current) | ||
| if (plainClickBehavior === 'none') { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the action context when middle-click remains enabled.
When plain-click behavior is none, this returns null before middle-click routing can run. middleClickBehavior defaults to open at Line 152, so selecting “Modifier-click only” disables configured middle-click URL handling.
Compute middleClickBehavior before this guard. Return null only when both behaviors are none.
Proposed fix
const getLinkActionContext = (paneId: number): TerminalLinkActionContext | null => {
const plainClickBehavior = terminalLinkClickBehaviorFor(deps.settingsRef.current)
- if (plainClickBehavior === 'none') {
+ const middleClickBehavior = deps.settingsRef.current?.terminalUrlMiddleClickBehavior ?? 'open'
+ if (plainClickBehavior === 'none' && middleClickBehavior === 'none') {
return null
}
@@
focusTerminal: () => pane.terminal.focus(),
plainClickBehavior,
- middleClickBehavior: deps.settingsRef.current?.terminalUrlMiddleClickBehavior ?? 'open'
+ middleClickBehavior
}![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Two gaps in the middle-click policy and a native-chat regression that leaves an existing test red. The direction is sound; the 'none' paths need another pass.
Reviewed changes
- Three-way plain-click mode —
terminalLinkClickBehaviorForreplaces the booleanterminalLinkActionPopoverEnabledfor terminal and native-chat links, with a legacy fallback. - Separate middle-click policy —
isTerminalMiddleClickActivationplusterminalUrlMiddleClickBehaviorrouted throughrequestTerminalLinkAction. - Hit testing and PTY suppression — button-1 events now hit-test links and participate in
mouseEventsRequireAltsuppression. - Settings UI — the single switch becomes two segmented controls, backed by new shared settings types and defaults.
- Tests — new resolver and routing tests.
ℹ️ Nitpicks
openHttpLinkAtTerminalMouseEvent(src/renderer/src/components/terminal-pane/terminal-url-link-hit-testing.ts:100) lost itsisTerminalHttpLinkActivationguard, so it now opens on any left or middle click. It has no production callers, but the change is inconsistent with the click policy and would bypass the popover if it were wired up.- The middle-click row label/description and every option label (
'Show actions','Open directly','Leave alone', …) are hardcoded inBrowserTerminalLinkActionsSetting.tsx:65-:77, while the title, description, and every sibling setting wrap user-facing copy intranslate(). src/renderer/src/components/terminal-pane/terminal-link-action-request.ts:71is unreachable for typed values: reaching it requiresmiddleClickBehavior !== 'none', and'open'returns earlier, so only an untypedundefinedfalls through — afterevent.preventDefault()on line 64 has already swallowed the event.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| } | ||
| if (!isTerminalLinkDirectActivation(event) || !shouldSuppressMouseEvent(event)) { | ||
| if ( | ||
| (!isTerminalLinkDirectActivation(event) && !isTerminalMiddleClickActivation(event)) || |
There was a problem hiding this comment.
shouldSuppressMouseEvent here is isLinkMouseEvent, not gated on the middle-click policy, so a middle click on a URL sets mouseEventsRequireAlt = true even when terminalUrlMiddleClickBehavior === 'none'. For a mouse-aware TUI that swallows the gesture before requestTerminalLinkAction can decline it, so "Leave alone" does not actually leave it alone.
Technical details
# Middle-click 'none' must not claim the gesture
## Affected sites
- `src/renderer/src/components/terminal-pane/terminal-link-pty-mouse-suppression.ts:85` — middle click suppresses the child mouse report unconditionally.
- `src/renderer/src/components/terminal-pane/terminal-url-link-hit-testing.ts:139` — `installHttpLinkClickFallback` passes `isLinkMouseEvent` as the suppression predicate, which never sees the policy.
## Required outcome
- When `middleClickBehavior === 'none'`, a middle click on a URL is not suppressed and reaches the child TUI (subject to the existing pointer-gesture eligibility rules).
## Suggested approach (optional)
- Thread the resolved middle-click policy into the suppression predicate (or the suppression installer) so middle click only suppresses when the policy is `'open'`/`'actions'`.
2440dbd to
836cb38
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 1c34f2e8-092b-40fe-ba0e-7f6fad059b09
📥 Commits
Reviewing files that changed from the base of the PR and between ebac94b and 836cb38cdeefafcf7e3e0b2d168256fa6f7121df.
📒 Files selected for processing (3)
src/renderer/src/components/settings/BrowserTerminalLinkActionsSetting.tsxsrc/renderer/src/components/settings/browser-search.test.tssrc/renderer/src/components/settings/browser-search.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| label="Plain click" | ||
| description="Choose whether a left-click shows actions, opens the URL, or leaves it to the terminal. Cmd/Ctrl-click always opens directly." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,110p' src/renderer/src/components/settings/BrowserTerminalLinkActionsSetting.tsx
sed -n '40,80p;180,210p' src/renderer/src/components/settings/browser-search.ts
rg -n 'translateSearchKeyword|SettingsSegmentedControl|label=.*translate|search keyword|localiz' src/renderer/src/components/settings src/renderer/src/i18n . --glob '!*node_modules*' | head -240Repository: stablyai/orca
Length of output: 37875
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings
Length of output: 23364
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- browser-search ---'
sed -n '1,90p' src/renderer/src/components/settings/browser-search.ts
printf '%s\n' '--- settings-search-keywords ---'
sed -n '1,220p' src/renderer/src/components/settings/settings-search-keywords.ts
printf '%s\n' '--- localized catalog helper ---'
sed -n '1,180p' src/renderer/src/i18n/localized-catalog.ts
printf '%s\n' '--- relevant localization references ---'
rg -n -i 'terminalLinkActions|link click behavior|open directly|modifier-click only|middle click|mouse 3|translateSearchKeyword|English-only|untranslat|keyword' src/renderer/src/i18n src/renderer/src/components/settings --glob '!*.map' | head -260
printf '%s\n' '--- locale/catalog files ---'
git ls-files 'src/renderer/src/i18n/*' | head -120Repository: stablyai/orca
Length of output: 41266
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- browser link routing copy ---'
sed -n '1,130p' src/renderer/src/components/settings/browser-link-routing-copy.ts
printf '%s\n' '--- searchable setting and search matching ---'
sed -n '1,180p' src/renderer/src/components/settings/SearchableSetting.tsx
rg -n -C 4 'function matchesSettingsSearch|matchesSettingsSearch|getTerminalLinkActionSearchKeywords|BrowserTerminalLinkActionsSetting' src/renderer/src/components/settings src/renderer/src --glob '*.{ts,tsx}'
printf '%s\n' '--- browser localization/search tests ---'
rg -n -C 8 'terminalLinkActions|TerminalLinkActions|link click behavior|open directly|modifier-click only|mouse 3|middle click' src/renderer/src --glob '*.{test,spec}.{ts,tsx}' || true
printf '%s\n' '--- translation implementation ---'
sed -n '1,130p' src/renderer/src/i18n/i18n.tsRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- browser link routing copy ---'
sed -n '1,100p' src/renderer/src/components/settings/browser-link-routing-copy.ts
printf '%s\n' '--- searchable setting ---'
sed -n '1,55p' src/renderer/src/components/settings/SearchableSetting.tsx
printf '%s\n' '--- settings search matcher ---'
rg -n -A 35 -B 8 '^export function matchesSettingsSearch|^function matchesSettingsSearch' src/renderer/src/components/settings/settings-search.ts
printf '%s\n' '--- browser-specific tests ---'
rg -l -i 'terminalLinkActions|TerminalLinkActions|link click behavior|open directly|modifier-click only|mouse 3' src/renderer/src --glob '*.{test,spec}.{ts,tsx}' | while read -r f; do
echo "--- $f"
rg -n -C 5 -i 'terminalLinkActions|TerminalLinkActions|link click behavior|open directly|modifier-click only|mouse 3' "$f"
done
printf '%s\n' '--- translation implementation ---'
sed -n '1,115p' src/renderer/src/i18n/i18n.tsRepository: stablyai/orca
Length of output: 10462
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- settings search scoring ---'
sed -n '1,125p' src/renderer/src/components/settings/settings-search.ts
printf '%s\n' '--- browser search tests ---'
sed -n '1,105p' src/renderer/src/components/settings/browser-search.test.tsRepository: stablyai/orca
Length of output: 8090
Localize the new link-click text and search aliases.
BrowserTerminalLinkActionsSetting passes raw English labels, descriptions, and option text to the settings controls, so localized users see English controls. getTerminalLinkActionSearchKeywords() also adds semantic aliases as raw English strings. matchesSettingsSearch() compares queries directly with these values, while translateSearchKeyword() provides localized values plus English fallbacks. Localized queries may therefore miss these aliases.
Use semantic translate() keys for the controls and translateSearchKeyword() IDs for the semantic aliases. Keep platform tokens such as cmd and ctrl as English aliases.
| control={ | ||
| <SettingsSegmentedControl<TerminalLinkClickBehavior> | ||
| value={behavior} | ||
| onChange={(value) => updateSettings({ terminalLinkClickBehavior: value })} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,120p' src/renderer/src/components/terminal-pane/terminal-link-click-behavior.ts
sed -n '20,95p' src/renderer/src/components/settings/BrowserTerminalLinkActionsSetting.tsx
sed -n '1,100p' src/renderer/src/components/terminal-pane/terminal-link-click-behavior.test.tsRepository: stablyai/orca
Length of output: 4123
Honor the explicit actions selection.
The control saves terminalLinkClickBehavior: 'actions', but terminalLinkClickBehaviorFor() only preserves explicit open and none. For an older profile with terminalLinkActionPopoverEnabled: false, it falls through to the legacy flag and returns none, so the control cannot retain the actions selection.
Return the explicit terminalLinkClickBehavior value before applying the legacy flag. Use the legacy flag only when the new setting is unset.
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes
This run covered the copy/label revision force-pushed since the previous review; no behavioral code changed.
- Renamed the Browser settings section to "Terminal URL clicks" and replaced its platform-specific description with platform-neutral copy reused by the settings-search entry.
- Split the single row into explicit "Plain click" and "Middle click" rows with their own labels/descriptions, and reworded the segmented-control options to "Actions" / "Open URL" / "Leave to terminal".
- Updated
browser-search.test.tsexpectations for the new title and description.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| description: translate( | ||
| 'auto.components.settings.BrowserTerminalLinkActionsSetting.description.v2', | ||
| 'Control clicks on detected URLs printed in terminal panes and chat transcripts.' |
There was a problem hiding this comment.
Replacing getTerminalLinkActionsDescription(platform) with this inline translate() removes the helper's last caller — it is now exported and unused in browser-link-routing-copy.ts:7 (verified with a repo-wide search). Delete it, or route both call sites through it so the copy can't drift.
Technical details
# Orphaned description helper
## Affected sites
- `src/renderer/src/components/settings/browser-search.ts:196` — inline `translate()` replaced the helper call.
- `src/renderer/src/components/settings/browser-link-routing-copy.ts:7` — `getTerminalLinkActionsDescription` has no remaining callers.
## Required outcome
- Remove the dead export, or have the component and the search entry share it as the single source of the description copy.| label="Plain click" | ||
| description="Choose whether a left-click shows actions, opens the URL, or leaves it to the terminal. Cmd/Ctrl-click always opens directly." |
There was a problem hiding this comment.
The row's visible text is now Plain click / Middle click (line 65) and the option labels are Open URL / Leave to terminal, but getTerminalLinkActionSearchKeywords still indexes the old wording (open directly, modifier-click only) and none of the new labels. Search matching is whole-query substring, so a user searching plain click or leave to terminal won't match this setting — the row is unfindable by its own text, which the note at browser-search.ts:185 calls out as the reason to index live copy.
Technical details
# Search keywords not updated for renamed copy
## Affected sites
- `src/renderer/src/components/settings/BrowserTerminalLinkActionsSetting.tsx:47` — visible row label `Plain click` (and `Middle click` at :65); option labels at :57-:59 and :74-:76.
- `src/renderer/src/components/settings/browser-search.ts:61` — keywords still list `open directly` / `modifier-click only` from the old labels; none of `plain click`, `open url`, `leave to terminal` are indexed.
- `src/renderer/src/components/settings/settings-search.ts:82` — matching is `includes()` on the whole normalized query, so partial-token hits don't rescue an unindexed label.
## Required outcome
- The search keywords include the new visible labels so the setting stays discoverable by its own copy.
836cb38 to
71b772a
Compare
There was a problem hiding this comment.
ℹ️ No new issues in this revision — the settings section is restructured, not re-behaved.
Reviewed changes
This run covered the settings-section restructure pushed since the prior pullfrog review; no behavioral code changed.
- Promoted the terminal URL controls into a standalone section using the shared
SettingsSubsectionHeader+divide-y divide-border/40grammar, replacing the nested indenteddiv. - Renamed the plain-click radiogroup's
aria-labelfrom the section title toPlain click URL behavior. - Reordered the middle-click options to match the plain-click control (
Actions/Open URL/Leave to terminal).
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Prevent the anchor default for… · native-chat-web-link-actions.ts:64-72
src/renderer/src/components/native-chat/native-chat-web-link-actions.ts:64-72
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent the anchor default for
plainClickBehavior === 'none'.CommentMarkdownrenders HTTP links as anchors withhrefand callshandleNativeChatWebLinkfrom the click handler. A plain primary click satisfiesisTerminalLinkActionActivation, while the'none'branch returnsfalsewithout callingpreventDefault. The anchor therefore opens its URL instead of remaining modifier-only.if (deps.plainClickBehavior === 'none') { event.preventDefault() return true }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 2a8f50d5-3b23-4690-a086-0bc5d527fc57
📥 Commits
Reviewing files that changed from the base of the PR and between 836cb38cdeefafcf7e3e0b2d168256fa6f7121df and 71b772a78a81d272d0ece22028eb8449a3f8b730.
📒 Files selected for processing (1)
src/renderer/src/components/settings/BrowserTerminalLinkActionsSetting.tsx
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
c6876d4 to
f2a634b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Prevent default navigation for… · native-chat-web-link-actions.ts:64-72
src/renderer/src/components/native-chat/native-chat-web-link-actions.ts:64-72
🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrevent default navigation for
plainClickBehavior === 'none'.
isTerminalLinkActionActivationtreats an unmodified primary click as an action click. Thenonebranch returnsfalsewithout callingevent.preventDefault(). The native-chat caller ignores that return value, so an HTTP anchor can perform its default navigation instead of remaining modifier-click-only.if (!deps.actionsEnabled) { if (deps.plainClickBehavior === 'none') { + event.preventDefault() - return false + return true }
🟡 Minor · Localize the terminal-link search keywords. · browser-search.ts:61-65
src/renderer/src/components/settings/browser-search.ts:61-65
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize the terminal-link search keywords.
These five raw English keywords bypass
translateSearchKeyword(...). The localization audit includes Settings searchkeywords, and no English-only exception applies. Add a localization key for each phrase and pass each phrase throughtranslateSearchKeyword(...). This preserves the English fallback and lets non-English users find the setting with localized terms.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 9c72ff07-5a9b-49cb-b2d8-f930aa7ff2ad
📥 Commits
Reviewing files that changed from the base of the PR and between c6876d4067bdd1ecbd5e71a646588edbd5a49c24 and f2a634b877f146a639aab211a06850bc3bf603dc.
📒 Files selected for processing (5)
src/renderer/src/components/native-chat/use-native-chat-link-actions.tssrc/renderer/src/components/terminal-pane/terminal-link-action-routing.test.tssrc/renderer/src/components/terminal-pane/terminal-link-activation.tssrc/renderer/src/components/terminal-pane/terminal-link-click-behavior.tssrc/renderer/src/components/terminal-pane/terminal-pane-mount-preparation.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.
| if (settings?.terminalLinkClickBehavior === 'none') { | ||
| return 'none' | ||
| } | ||
| return settings?.terminalLinkActionPopoverEnabled === false ? 'none' : 'actions' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make explicit 'actions' override the legacy setting.
When an existing profile has terminalLinkActionPopoverEnabled: false, selecting the new 'actions' option still resolves to 'none'. The new preference does not work for that profile.
Return 'actions' when terminalLinkClickBehavior === 'actions' before consulting the legacy field.
Proposed fix
+ if (settings?.terminalLinkClickBehavior === 'actions') {
+ return 'actions'
+ }
if (settings?.terminalLinkClickBehavior === 'open') {
return 'open'
}
There was a problem hiding this comment.
Important
Plain-click "Leave to terminal" no longer works: with the default middle-click policy, the action popover now opens on a plain left-click. One inline fix, plus a latent resolver bug that makes an explicit "Actions" choice impossible to select on a legacy profile.
Reviewed changes
This run covered the delta between the prior pullfrog review (71b772a) and f2a634b, which reworks the 'none' paths.
- Preserved native-chat legacy behavior:
use-native-chat-link-actions.tsmaps a legacyterminalLinkActionPopoverEnabled: falseto'open'before delegating to the resolver, greening the previously red native-chat test (verified: 10/10 pass). - Restored the
isTerminalHttpLinkActivationguard onopenHttpLinkAtTerminalMouseEvent, reverting the prior head's dropped guard. - Kept the middle-click policy independent of plain click:
getLinkActionContextnow returnsnullonly when both behaviors are'none'. - Tightened
isTerminalMiddleClickActivationto also require no Cmd/Ctrl, and added a middle-click routing test for the plain-click'none'case. - Dropped the unused
TerminalUrlMiddleClickBehavioralias.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| !( | ||
| isTerminalLinkActionActivation(event) || | ||
| (isTerminalMiddleClickActivation(event) && context.middleClickBehavior !== 'none') | ||
| ) || |
There was a problem hiding this comment.
With plainClickBehavior === 'none' and a middle-click policy other than 'none' (the default is 'open'), this guard still admits plain left-clicks. getLinkActionContext now keeps the context alive for middle click, so requestTerminalLinkAction claims the gesture and opens the action popover on a plain click — selecting "Leave to terminal" stops working as soon as middle click is enabled. The OSC and file/task link paths share this function and regress the same way.
Technical details
# Plain-click `'none'` must not request actions
## Affected sites
- `src/renderer/src/components/terminal-pane/terminal-link-action-request.ts:52` — the guard admits `isTerminalLinkActionActivation(event)` regardless of `context.plainClickBehavior`.
- `src/renderer/src/components/terminal-pane/terminal-pane-mount-preparation.ts:136` — the new `plainClickBehavior === 'none' && middleClickBehavior === 'none'` condition is what keeps the context non-null.
- `src/renderer/src/components/terminal-pane/terminal-url-link-hit-testing.ts:139` — `shouldDeferPlainMouseEvent` also keys off the context alone, so plain clicks are deferred from the TUI before the guard even runs.
- `src/renderer/src/components/terminal-pane/terminal-handle-links.ts:212` and `terminal-link-handlers.ts:202` — file/task links share `requestTerminalLinkAction`.
## Required outcome
- When `plainClickBehavior === 'none'`, a plain left-click on a link is not claimed and does not open the action popover; middle click still works per its own policy. This matches the prior head, where a `null` context short-circuited the request.
## Suggested approach (optional)
- Gate the plain branch on `context.plainClickBehavior !== 'none'` in the guard (see suggestion), and gate `shouldDeferPlainMouseEvent` on the same predicate so plain clicks reach the TUI without the defer round-trip.| !( | |
| isTerminalLinkActionActivation(event) || | |
| (isTerminalMiddleClickActivation(event) && context.middleClickBehavior !== 'none') | |
| ) || | |
| !( | |
| (isTerminalLinkActionActivation(event) && context.plainClickBehavior !== 'none') || | |
| (isTerminalMiddleClickActivation(event) && context.middleClickBehavior !== 'none') | |
| ) || |
| if (settings?.terminalLinkClickBehavior === 'open') { | ||
| return 'open' | ||
| } | ||
| if (settings?.terminalLinkClickBehavior === 'none') { | ||
| return 'none' | ||
| } | ||
| return settings?.terminalLinkActionPopoverEnabled === false ? 'none' : 'actions' |
There was a problem hiding this comment.
An explicit terminalLinkClickBehavior: 'actions' falls through to the legacy boolean. A profile that previously had terminalLinkActionPopoverEnabled: false therefore resolves to 'none' even after the user selects "Actions" in the new control, and the segmented control snaps back — the settings UI writes only terminalLinkClickBehavior, so the choice can never take effect.
Technical details
# Explicit `'actions'` must win over the legacy boolean
## Affected sites
- `src/renderer/src/components/terminal-pane/terminal-link-click-behavior.ts:12` — only `'open'`/`'none'` are treated as authoritative; `'actions'` falls through to the legacy check.
- `src/renderer/src/components/settings/BrowserTerminalLinkActionsSetting.tsx:60` — the control calls `updateSettings({ terminalLinkClickBehavior: value })` and never clears `terminalLinkActionPopoverEnabled`.
## Required outcome
- Any explicit `terminalLinkClickBehavior` value (including `'actions'`) is authoritative; the legacy boolean applies only when the new key is absent.
## Suggested approach (optional)
- Replace the two equality checks with a single `if (settings?.terminalLinkClickBehavior !== undefined) return settings.terminalLinkClickBehavior`.
## Open questions for the human (optional)
- Should `terminalLinkActionPopoverEnabled` be cleared on upgrade so the two keys cannot disagree?| if (settings?.terminalLinkClickBehavior === 'open') { | |
| return 'open' | |
| } | |
| if (settings?.terminalLinkClickBehavior === 'none') { | |
| return 'none' | |
| } | |
| return settings?.terminalLinkActionPopoverEnabled === false ? 'none' : 'actions' | |
| if (settings?.terminalLinkClickBehavior !== undefined) { | |
| return settings.terminalLinkClickBehavior | |
| } | |
| return settings?.terminalLinkActionPopoverEnabled === false ? 'none' : 'actions' |
f2a634b to
625c68c
Compare
625c68c to
80cb679
Compare
There was a problem hiding this comment.
Important
The new runtime-catalog entry ships the pre-rename English title, so the Browser settings section renders "Show link actions" again and two browser-search.test.ts cases now fail. One inline fix.
Reviewed changes
This run covered the two i18n commits pushed since the prior pullfrog review (f2a634b877).
- Localized the plain-click and middle-click rows and every option label in
BrowserTerminalLinkActionsSetting.tsx, adding nine keys toen.json(addresses the earlier hardcoded-copy nitpick). - Added
BrowserTerminalLinkActionsSetting.titletoen-runtime-required.jsonto satisfy the runtime-catalog gate.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| "d420c43729": "Imported {{value0}} cookies from {{value1}}{{value2}} into {{value3}}." | ||
| }, | ||
| "BrowserTerminalLinkActionsSetting": { | ||
| "title": "Show link actions" |
There was a problem hiding this comment.
Shipping the pre-rename English title here undoes the rename in the English UI. en.json still holds "title": "Show link actions" while the component's inline default is "Terminal URL clicks", so the runtime-catalog generator marked the title as required and this entry now serves the stale string. Two browser-search.test.ts cases fail as a result (verified locally: the search entry's title no longer equals Terminal URL clicks).
Technical details
# Stale English title shipped to the boot bundle
## Affected sites
- `src/renderer/src/i18n/en-runtime-required.json:1161` — ships `"title": "Show link actions"` into the eager English bundle.
- `src/renderer/src/i18n/locales/en.json:11332` — translator catalog still holds the pre-rename `"title": "Show link actions"`.
- `src/renderer/src/components/settings/BrowserTerminalLinkActionsSetting.tsx:32-35` — inline default is `"Terminal URL clicks"`.
- `src/renderer/src/components/settings/browser-search.test.ts:64,104` — assert the title is `Terminal URL clicks`; both now fail (`received "Show link actions"`).
## Required outcome
- English resolves `BrowserTerminalLinkActionsSetting.title` to `Terminal URL clicks`, and `pnpm run verify:localization-runtime-catalog` stays green.
## Suggested approach (optional)
- Change `en.json`'s `title` to `Terminal URL clicks` so it matches the call-site default, then run `pnpm run sync:localization-runtime-catalog`; the key is no longer required and drops out of the boot bundle. Locale catalogs can keep the old translation until retranslated.
What changed
Terminal URLs now support a clear, user-controlled click policy.
In Settings → Browser → Link Routing, users can choose clear policies for terminal URL clicks:
The same setting is used by native chat links so the two surfaces behave consistently. Shift+Cmd/Ctrl keeps its alternate destination behavior.
Middle-click (mouse button 3 / browser
button === 1) is configurable separately for terminal URLs:Middle-click URL gestures are claimed safely from mouse-aware TUIs only when the selected URL policy handles them. Existing right-click paste and context-menu behavior is unchanged.
Older profiles keep their existing behavior through a compatibility resolver: the previous action-popover boolean maps to the equivalent new mode.
Why this approach
The open requests all describe different preferences: some users want a discoverable action menu, some want one-click navigation, and some need plain clicks to reach TUIs. A three-way setting makes those choices explicit without changing the existing default. Middle-click gets its own policy because it is a distinct browser convention and often conflicts with terminal selection paste.
Validation
Visual proof
The settings are now a standalone “Terminal URL clicks” section instead of controls nested under the Link Routing switch. The two controls sit inside an inset bordered panel so their relationship to the section heading is visible at a glance. Both controls use the same option order: “Actions”, “Open URL”, “Leave to terminal”; only their defaults differ. The labels describe the actual gesture and outcome.
The screenshot shows the live Browser settings page with both the three-way URL click control and the middle-click URL control.
Fixes #20554
Related to #9279