Skip to content

tmux stuff - #717

Merged
austinywang merged 2 commits into
mainfrom
cmux/tmux-bindings
Mar 3, 2026
Merged

austinywang merged 2 commits into
mainfrom
cmux/tmux-bindings

Conversation

@austinywang

@austinywang austinywang commented Mar 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Command-key shortcuts are now routed intelligently to the terminal when focused — preserving standard app behaviors (e.g., Preferences via Cmd+, and copy when text is selected).
  • Tests

    • Added tests covering command-shortcut routing across modifier combinations, keystrokes, and terminal selection states.

@vercel

vercel Bot commented Mar 1, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Mar 2, 2026 11:34pm

@coderabbitai

coderabbitai Bot commented Mar 1, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 253c196 and fa0b914.

📒 Files selected for processing (2)
  • Sources/AppDelegate.swift
  • cmuxTests/CmuxWebViewKeyEquivalentTests.swift
🚧 Files skipped from review as they are similar to previous changes (2)
  • cmuxTests/CmuxWebViewKeyEquivalentTests.swift
  • Sources/AppDelegate.swift

📝 Walkthrough

Walkthrough

Adds routing that can forward Cmd-key shortcuts from the focused Ghostty terminal view to Ghostty instead of the app menu. A new predicate decides routing based on normalized modifiers, key/char, and terminal selection; NSWindow’s key-equivalent handler forwards qualifying events to Ghostty. Tests exercise these cases.

Changes

Cohort / File(s) Summary
Terminal Command-Shortcut Routing Implementation
Sources/AppDelegate.swift
Add shouldRouteTerminalCommandShortcutToGhostty(...) to normalize modifiers and decide routing; in cmux_performKeyEquivalent introduce passthrough branch that forwards qualifying Cmd-key events to the focused Ghostty view via ghosttyView.keyDown(with:).
Routing Logic Test Suite
cmuxTests/CmuxWebViewKeyEquivalentTests.swift
Add TerminalCommandShortcutRoutingPolicyTests with cases for: routing Cmd+C when no selection, preserving Cmd+C when selection exists, preserving Cmd+, for Preferences, requiring Cmd modifier, and routing other Cmd-based shortcuts to terminal.

Sequence Diagram

sequenceDiagram
    participant User
    participant NSWindow
    participant Router as shouldRouteTerminalCommandShortcutToGhostty()
    participant Ghostty as Ghostty Terminal View
    participant AppMenu as App Menu System

    User->>NSWindow: Press Command+Key
    NSWindow->>NSWindow: cmux_performKeyEquivalent
    NSWindow->>NSWindow: Is Ghostty focused?
    alt Ghostty focused
        NSWindow->>Router: Evaluate (flags, chars/keyCode, selection)
        alt Route to Ghostty
            Router-->>NSWindow: true
            NSWindow->>Ghostty: keyDown(with:event)
            Ghostty->>User: Handle in terminal
        else Route to App Menu
            Router-->>NSWindow: false
            NSWindow->>AppMenu: Dispatch to menu system
            AppMenu->>User: Execute app action
        end
    else Not focused
        NSWindow->>AppMenu: Dispatch to menu system
        AppMenu->>User: Execute app action
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped on keys with ears aflap,
Cmd whispers sent through Ghostty's gap,
Some shortcuts stay, some find the shell,
Tests watch closely — all is well.
A tiny hop, a tidy patch, hooray!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Title check ❓ Inconclusive The title 'tmux stuff' is vague and generic, using non-descriptive language that does not convey meaningful information about the substantial changes in the pull request. Use a more descriptive title that captures the main change, such as 'Route Command shortcuts to Ghostty terminal' or 'Add Command shortcut routing policy for terminal.'
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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 cmux/tmux-bindings

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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)

2190-2207: Split the dual-assert test into separate test methods for sharper failures.

testRoutesOtherCommandShortcutsToTerminal currently validates two unrelated inputs. Splitting them makes failures more actionable during regressions.

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

In `@cmuxTests/CmuxWebViewKeyEquivalentTests.swift` around lines 2190 - 2207,
Split the dual-assert test testRoutesOtherCommandShortcutsToTerminal into two
distinct tests so each assertion fails independently: create one test (e.g.,
testRouteCommandOptionCToTerminal) that calls
shouldRouteTerminalCommandShortcutToGhostty(flags: [.command, .option], chars:
"c", keyCode: 8, terminalHasSelection: false) and asserts true, and a second
test (e.g., testRouteCommandVToTerminal) that calls
shouldRouteTerminalCommandShortcutToGhostty(flags: [.command], chars: "v",
keyCode: 9, terminalHasSelection: false) and asserts true; keep the same
parameters and expectations, just move each XCTAssertTrue into its own test
method to improve test failure granularity.
🤖 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 2190-2207: Split the dual-assert test
testRoutesOtherCommandShortcutsToTerminal into two distinct tests so each
assertion fails independently: create one test (e.g.,
testRouteCommandOptionCToTerminal) that calls
shouldRouteTerminalCommandShortcutToGhostty(flags: [.command, .option], chars:
"c", keyCode: 8, terminalHasSelection: false) and asserts true, and a second
test (e.g., testRouteCommandVToTerminal) that calls
shouldRouteTerminalCommandShortcutToGhostty(flags: [.command], chars: "v",
keyCode: 9, terminalHasSelection: false) and asserts true; keep the same
parameters and expectations, just move each XCTAssertTrue into its own test
method to improve test failure granularity.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dba1e23 and 253c196.

📒 Files selected for processing (2)
  • Sources/AppDelegate.swift
  • cmuxTests/CmuxWebViewKeyEquivalentTests.swift

@greptile-apps

greptile-apps Bot commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds support for custom tmux prefixes using Command-based keyboard shortcuts. The implementation introduces a new routing function shouldRouteTerminalCommandShortcutToGhostty that forwards Command key combinations to the terminal when focused, enabling users to configure tmux with prefixes like Cmd+C.

Key changes:

  • Command shortcuts are now routed to the terminal to support custom tmux prefix bindings
  • Cmd+, (Preferences) is preserved for consistent macOS UX
  • Cmd+C with text selection is preserved for standard copy behavior
  • Critical app shortcuts (Cmd+Q, Cmd+N, Cmd+T, Cmd+W) are handled earlier in the event chain via handleCustomShortcut and remain unaffected
  • Comprehensive test coverage validates all edge cases and exceptions

The implementation follows existing patterns in the codebase (similar to shouldRouteTerminalFontZoomShortcutToGhostty) and balances advanced tmux user needs with essential app functionality.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is well-designed with appropriate safeguards for critical shortcuts, follows existing code patterns, includes comprehensive test coverage, and the changes are isolated to keyboard event routing logic
  • No files require special attention

Important Files Changed

Filename Overview
Sources/AppDelegate.swift Added shouldRouteTerminalCommandShortcutToGhostty function and integrated it into key event handling to support custom tmux prefixes with Command keys while preserving critical shortcuts
cmuxTests/CmuxWebViewKeyEquivalentTests.swift Added comprehensive test suite for the new Command shortcut routing function covering all edge cases and exceptions

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Key Event] --> B{Local Event Monitor<br/>`handleCustomShortcut`}
    B -->|Cmd+Q, Cmd+N, Cmd+T, Cmd+W<br/>handled| C[Event Consumed]
    B -->|Pass through| D{`performKeyEquivalent`}
    D --> E{Terminal focused?}
    E -->|No| F[Normal event handling]
    E -->|Yes| G{Font zoom shortcut?}
    G -->|Yes| H[Route to terminal]
    G -->|No| I{Browser event?}
    I -->|Yes| J[Handle browser event]
    I -->|No| K{**NEW: Command shortcut?**<br/>`shouldRouteTerminalCommand...`}
    K -->|Cmd+, or<br/>Cmd+C with selection| L{Main menu check}
    K -->|Other Cmd shortcuts| M[Route to terminal<br/>for tmux prefix]
    L -->|Consumed by menu| C
    L -->|Not consumed| F
Loading

Last reviewed commit: 253c196

@austinywang
austinywang merged commit a3681ed into main Mar 3, 2026
8 checks passed
austinywang added a commit that referenced this pull request Mar 6, 2026
This reverts commit a3681ed.
@austinywang austinywang mentioned this pull request Mar 6, 2026
bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026
bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026

This branch was successfully deployed

1 active deployment
Preview — fa0b9141 Deployed Mar 2, 2026 by vercel[bot]
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