Skip to content

Add configurable Cmd+Shift/Option+Shift override for sidebar PR links - #686

Closed
lawrencecchen wants to merge 1 commit into
mainfrom
task-cmd-shift-browser-open-setting
Closed

lawrencecchen wants to merge 1 commit into
mainfrom
task-cmd-shift-browser-open-setting

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Feb 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add a configurable modifier override for sidebar pull request links with two modes:
    • Cmd+Shift -> cmux browser, Option+Shift -> default browser
    • Cmd+Shift -> default browser, Option+Shift -> cmux browser
  • add settings UI under App settings (next to the sidebar PR link destination toggle)
  • route sidebar PR link opens and the "Open Workspace Pull Requests" command through the same override logic
  • preserve link-open override semantics without triggering unintended multi-select/range selection side effects
  • add/rename regression tests for BrowserLinkOpenSettings modifier behavior and fallback paths

Validation

  • xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -configuration Debug -derivedDataPath /tmp/cmux-dd-task-cmd-shift-browser-open-setting -destination 'platform=macOS' -only-testing:cmuxTests/CmuxWebViewKeyEquivalentTests test
  • xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug -derivedDataPath /tmp/cmux-dd-task-cmd-shift-browser-open-setting -destination 'platform=macOS' build
  • codex review --uncommitted (clean)

Summary by CodeRabbit

  • New Features

    • Added new "Sidebar PR Link Modifier Override" setting allowing users to configure which modifier key combination (Cmd+Shift or Option+Shift) opens pull request links in the cmux browser versus the default browser.
  • Tests

    • Added tests validating modifier key configuration, storage, and fallback behavior.

@vercel

vercel Bot commented Feb 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Building Building Preview, Comment Feb 28, 2026 10:56am

@coderabbitai

coderabbitai Bot commented Feb 28, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The PR implements modifier key-based customization for opening pull request links. It introduces a new BrowserLinkModifierBehavior enum to map Cmd+Shift and Option+Shift modifiers to opening in cmux or default browser, propagates modifier flags through the UI layer, adds configuration UI in Settings, and includes comprehensive tests.

Changes

Cohort / File(s) Summary
Modifier Flags Propagation
Sources/ContentView.swift
Updated openWorkspacePullRequestsInConfiguredBrowser and updateSelection to accept modifierFlags parameter, enabling behavior changes based on modifier keys. Propagates modifier state from command handling through the link-opening call chain.
Settings Infrastructure & API
Sources/Panels/BrowserPanel.swift
Introduced BrowserLinkModifierBehavior enum with two cases mapping Cmd+Shift and Option+Shift to different browser targets. Added settings keys, defaults, and public helper methods including linkModifierBehavior, linkModifierOverrideOpensInCmuxBrowser, and shouldOpenSidebarPullRequestLinkInCmuxBrowser.
UI Configuration
Sources/cmuxApp.swift
Integrated new modifier behavior setting into SettingsView with a picker UI. Added initialization, storage binding, and reset logic for the linkModifierBehavior setting.
Tests
cmuxTests/CmuxWebViewKeyEquivalentTests.swift
Comprehensive unit tests validating modifier behavior defaults, storage, overrides, fallback logic, and edge cases for the new link-opening customization feature.

Sequence Diagram

sequenceDiagram
    actor User
    participant TabItemView
    participant ContentView
    participant BrowserLinkOpenSettings
    participant Browser

    User->>TabItemView: Clicks PR link (with modifier key)
    TabItemView->>ContentView: updateSelection(modifierFlags)
    ContentView->>ContentView: openWorkspacePullRequestsInConfiguredBrowser(modifierFlags)
    ContentView->>BrowserLinkOpenSettings: shouldOpenSidebarPullRequestLinkInCmuxBrowser(modifierFlags)
    BrowserLinkOpenSettings->>BrowserLinkOpenSettings: linkModifierOverrideOpensInCmuxBrowser(modifierFlags)
    alt Override modifier configured
        BrowserLinkOpenSettings-->>BrowserLinkOpenSettings: Returns Bool (opens in cmux or default)
    else No override modifier
        BrowserLinkOpenSettings->>BrowserLinkOpenSettings: Fall back to openSidebarPullRequestLinksInCmuxBrowser
        BrowserLinkOpenSettings-->>BrowserLinkOpenSettings: Returns Bool (default behavior)
    end
    BrowserLinkOpenSettings-->>ContentView: shouldOpen → Bool
    ContentView->>Browser: Open PR link in configured browser
    Browser-->>User: Display PR
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Modifier keys now guide the way,
Cmd and Option lead the play—
Through meadows of the browser bound,
A PR's path by keys is found!
🌿✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding a configurable modifier key override (Cmd+Shift/Option+Shift) for sidebar PR link opening behavior.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch task-cmd-shift-browser-open-setting

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
cmuxTests/CmuxWebViewKeyEquivalentTests.swift (1)

7647-7656: Consider adding one invalid-raw-value fallback test.

A small extra test for an unknown persisted linkModifierBehaviorKey value would harden regression coverage for defaults migration/fallback behavior.

💡 Suggested test addition
+    func testLinkModifierBehaviorFallsBackToDefaultForInvalidStoredValue() {
+        defaults.set("not-a-valid-behavior", forKey: BrowserLinkOpenSettings.linkModifierBehaviorKey)
+        XCTAssertEqual(
+            BrowserLinkOpenSettings.linkModifierBehavior(defaults: defaults),
+            .commandShiftOpensCmuxOptionShiftOpensDefault
+        )
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmuxTests/CmuxWebViewKeyEquivalentTests.swift` around lines 7647 - 7656, Add
a test (e.g. testLinkModifierBehaviorFallsBackOnInvalidRawValue) that writes an
invalid/unknown value into UserDefaults for
BrowserLinkOpenSettings.linkModifierBehaviorKey, then call
BrowserLinkOpenSettings.linkModifierBehavior(defaults: defaults) and assert it
falls back to the same value returned when the key is absent (remove the key and
call linkModifierBehavior again) to verify unknown raw values use the safe
fallback; reference BrowserLinkOpenSettings.linkModifierBehavior(defaults:) and
BrowserLinkOpenSettings.linkModifierBehaviorKey in the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cmuxTests/CmuxWebViewKeyEquivalentTests.swift`:
- Around line 7647-7656: Add a test (e.g.
testLinkModifierBehaviorFallsBackOnInvalidRawValue) that writes an
invalid/unknown value into UserDefaults for
BrowserLinkOpenSettings.linkModifierBehaviorKey, then call
BrowserLinkOpenSettings.linkModifierBehavior(defaults: defaults) and assert it
falls back to the same value returned when the key is absent (remove the key and
call linkModifierBehavior again) to verify unknown raw values use the safe
fallback; reference BrowserLinkOpenSettings.linkModifierBehavior(defaults:) and
BrowserLinkOpenSettings.linkModifierBehaviorKey in the test.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7916b2d and fb1abc4.

📒 Files selected for processing (4)
  • Sources/ContentView.swift
  • Sources/Panels/BrowserPanel.swift
  • Sources/cmuxApp.swift
  • cmuxTests/CmuxWebViewKeyEquivalentTests.swift

@greptile-apps

greptile-apps Bot commented Feb 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Added configurable modifier override for sidebar PR links, allowing users to customize Cmd+Shift and Option+Shift behaviors for opening links in either cmux or default browser. The implementation correctly captures modifier flags before selection updates to prevent unintended multi-select side effects, and falls back to the existing toggle setting when no override modifier is pressed.

  • Introduced BrowserLinkModifierBehavior enum with two configuration modes and proper modifier flag normalization
  • Updated openPullRequestLink and openWorkspacePullRequestsInConfiguredBrowser to route through the new override system
  • Added settings UI picker under App settings with clear display names and descriptions
  • Included comprehensive unit tests covering defaults, overrides, unconfigured modifiers, and fallback behavior

Confidence Score: 5/5

  • This PR is safe to merge with no risks identified
  • Clean implementation with well-thought-out design decisions including proper modifier capture timing, flag normalization, fallback logic, and comprehensive test coverage. All changes follow existing codebase patterns and integrate seamlessly with the settings UI.
  • No files require special attention

Important Files Changed

Filename Overview
Sources/Panels/BrowserPanel.swift Added BrowserLinkModifierBehavior enum with modifier-based browser routing logic and helper methods
Sources/ContentView.swift Updated PR link opening logic to capture modifier flags and route through new override system
Sources/cmuxApp.swift Added settings UI picker for configuring sidebar PR link modifier override behavior
cmuxTests/CmuxWebViewKeyEquivalentTests.swift Added comprehensive unit tests covering modifier behavior, fallback logic, and edge cases

Last reviewed commit: fb1abc4

@lawrencecchen lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 23, 2026

This branch was successfully deployed

1 active deployment
Preview — fb1abc45 Deployed Feb 28, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants