feat(browser): add Cmd+Shift+Click to toggle terminal link destination#1470
feat(browser): add Cmd+Shift+Click to toggle terminal link destination#1470dortort wants to merge 1 commit into
Conversation
|
@dortort is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds extensive localization updates, makes terminal link opening respect the Shift modifier combined with the user setting to choose embedded vs external browser, updates a settings subtitle, and advances the ghostty submodule pointer. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Keyboard/Mouse)
participant TerminalView as TerminalView
participant Settings as Settings (openTerminalLinksInCmuxBrowser)
participant CmuxBrowser as CmuxBrowser (embedded)
participant SystemBrowser as SystemBrowser (external)
User->>TerminalView: Click link (may hold Shift)
TerminalView->>User: read modifier state (Shift)
TerminalView->>Settings: read openTerminalLinksInCmuxBrowser
TerminalView->>TerminalView: compute useCmuxBrowser = (ShiftHeld XOR setting)
alt useCmuxBrowser == true
TerminalView->>CmuxBrowser: open URL (embedded)
else
TerminalView->>SystemBrowser: open URL (external)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="ghostty">
<violation number="1" location="ghostty:1">
P1: This submodule pointer references a Ghostty commit that is not yet available on an accessible remote, which will break CI and fresh checkouts when resolving submodules.</violation>
</file>
<file name="Resources/Localizable.xcstrings">
<violation number="1" location="Resources/Localizable.xcstrings:51355">
P3: Fix the Polish translation grammar: "w drugim przeglądarce" is incorrect and should use a grammatically correct phrase (e.g., "w innej przeglądarce").</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| "state": "translated", | ||
| "value": "Po wyłączeniu linki kliknięte w terminalu otwierają się w domyślnej przeglądarce." | ||
| "state": "needs_review", | ||
| "value": "Ustawia domyślną akcję Cmd+Klik. Cmd+Shift+Klik otwiera w drugim przeglądarce." |
There was a problem hiding this comment.
P3: Fix the Polish translation grammar: "w drugim przeglądarce" is incorrect and should use a grammatically correct phrase (e.g., "w innej przeglądarce").
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Resources/Localizable.xcstrings, line 51355:
<comment>Fix the Polish translation grammar: "w drugim przeglądarce" is incorrect and should use a grammatically correct phrase (e.g., "w innej przeglądarce").</comment>
<file context>
@@ -51292,109 +51292,109 @@
- "state": "translated",
- "value": "Po wyłączeniu linki kliknięte w terminalu otwierają się w domyślnej przeglądarce."
+ "state": "needs_review",
+ "value": "Ustawia domyślną akcję Cmd+Klik. Cmd+Shift+Klik otwiera w drugim przeglądarce."
}
},
</file context>
4083008 to
49bc422
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ghostty`:
- Line 1: The parent repo's submodule pointer references commit
068064f06be42c7ba5000171178fde43559a0c9e which is not reachable by CI; push that
exact commit to the manaflow-ai/ghostty remote (or the branch you use for
submodules) first so the SHA is resolvable, then update the parent repo's
submodule pointer (the gitlink) to that commit and re-run CI to verify ancestry;
in future, always push the submodule commit to its remote before committing the
updated pointer in the parent repository.
In `@Resources/Localizable.xcstrings`:
- Around line 51354-51355: The Polish subtitle text uses the incorrect phrase "w
drugim przeglądarce," which is grammatically wrong. Update the phrase to the
correct declension "w drugiej przeglądarce" to ensure proper Polish grammar and
natural reading in the UI text. Locate this fix in the subtitle value string in
the resources file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 714acb3d-7c87-403d-8bb5-6860fb982654
📒 Files selected for processing (4)
Resources/Localizable.xcstringsSources/GhosttyTerminalView.swiftSources/cmuxApp.swiftghostty
| @@ -1 +1 @@ | |||
| Subproject commit bc9be90a21997a4e5f06bf15ae2ec0f937c2dc42 | |||
| Subproject commit 068064f06be42c7ba5000171178fde43559a0c9e | |||
There was a problem hiding this comment.
Blocker: submodule SHA must be reachable by CI before merging
The parent repo now references 068064f06be42c7ba5000171178fde43559a0c9e, but this SHA is currently not resolvable by CI (as noted in the PR summary). Please push this exact commit to the manaflow-ai/ghostty fork/branch first, then keep this pointer update.
Based on learnings: When modifying a submodule, push the submodule commit to its remote branch before committing the updated pointer in the parent repo, and verify ancestry from the submodule remote.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ghostty` at line 1, The parent repo's submodule pointer references commit
068064f06be42c7ba5000171178fde43559a0c9e which is not reachable by CI; push that
exact commit to the manaflow-ai/ghostty remote (or the branch you use for
submodules) first so the SHA is resolvable, then update the parent repo's
submodule pointer (the gitlink) to that commit and re-run CI to verify ancestry;
in future, always push the submodule commit to its remote before committing the
updated pointer in the parent repository.
Cmd+Click opens links using the default destination per the "Open Terminal Links in cmux Browser" setting. Cmd+Shift+Click opens in the opposite browser, giving users quick access to both without changing the setting. - Read shift state in OPEN_URL handler and XOR with setting - Update setting subtitle to explain the Cmd+Shift shortcut - Update all xcstrings translations (marked needs_review) - Update ghostty submodule for shift-aware link detection Closes manaflow-ai#719 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
49bc422 to
f1c37fb
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Resources/Localizable.xcstrings (1)
51354-51355:⚠️ Potential issue | 🟡 MinorFix the Polish declension in this subtitle.
Line 51355 still uses
w drugim przeglądarce, which is ungrammatical in Polish and will read awkwardly in Settings.✏️ Suggested fix
- "value": "Ustawia domyślną akcję Cmd+Klik. Cmd+Shift+Klik otwiera w drugim przeglądarce." + "value": "Ustawia domyślną akcję Cmd+Klik. Cmd+Shift+Klik otwiera w drugiej przeglądarce."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Localizable.xcstrings` around lines 51354 - 51355, Update the Polish subtitle value in Resources/Localizable.xcstrings by correcting the declension “w drugim przeglądarce” to the grammatically correct form “w drugiej przeglądarce” in the string value "Ustawia domyślną akcję Cmd+Klik. Cmd+Shift+Klik otwiera w drugim przeglądarze." so it reads "Ustawia domyślną akcję Cmd+Klik. Cmd+Shift+Klik otwiera w drugiej przeglądarce."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Resources/Localizable.xcstrings`:
- Around line 51354-51355: Update the Polish subtitle value in
Resources/Localizable.xcstrings by correcting the declension “w drugim
przeglądarce” to the grammatically correct form “w drugiej przeglądarce” in the
string value "Ustawia domyślną akcję Cmd+Klik. Cmd+Shift+Klik otwiera w drugim
przeglądarze." so it reads "Ustawia domyślną akcję Cmd+Klik. Cmd+Shift+Klik
otwiera w drugiej przeglądarce."
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e956150-5888-45de-9401-fd2bef60a44b
📒 Files selected for processing (4)
Resources/Localizable.xcstringsSources/GhosttyTerminalView.swiftSources/cmuxApp.swiftghostty
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/cmuxApp.swift
- ghostty
Summary
Implements the feature requested in #719 (comment):
Changes
Ghostty submodule (manaflow-ai/ghostty#13)
linkAtPos()so Cmd+Shift activates links the same as Cmdlink_pointcache inmodsChanged()so cursor shape updates when mods changeSwift (
Sources/GhosttyTerminalView.swift)OPEN_URLhandler and XOR with the setting to determine browser destinationSettings (
Sources/cmuxApp.swift,Resources/Localizable.xcstrings)needs_review)Dependencies
Ghostty submodule PR must merge first: manaflow-ai/ghostty#13
Test plan
Demo
Screen.Recording.2026-03-15.at.1.47.34.PM.mov
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation