Skip to content

Show split action shortcuts in collapsed menu - #271

Open
austinywang wants to merge 1 commit into
mainfrom
feat-split-menu-shortcuts
Open

austinywang wants to merge 1 commit into
mainfrom
feat-split-menu-shortcuts

Conversation

@austinywang

@austinywang austinywang commented Oct 2, 2026 •

Copy link
Copy Markdown

The collapsed split action menu now accepts host-provided SwiftUI keyboard shortcuts and attaches them to each action item. This lets cmux display its live configurable shortcuts beside New Tab and Split actions.

— unregistered


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Shows host-provided keyboard shortcuts beside the collapsed split action menu items so the menu stays in sync with cmux's configurable shortcuts.

  • Adds splitButtonShortcuts to BonsplitController so hosts can map split actions to SwiftUI KeyboardShortcuts.
  • Attaches the matching shortcut to each collapsed menu item when one is set.

Written for commit 1b52547. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Split actions in the narrow-pane overflow menu now display their configured keyboard shortcuts.
    • Added a controller setting for assigning keyboard shortcuts to split actions.

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: fcd79eef-e6dc-460a-9f2f-7afc1a2c1e15

📥 Commits

Reviewing files that changed from the base of the PR and between 64ac6d4 and 1b52547.

📒 Files selected for processing (2)
  • Sources/Bonsplit/Internal/Views/TabBarView.swift
  • Sources/Bonsplit/Public/BonsplitController.swift

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The controller now exposes a dictionary for split-action keyboard shortcuts. The narrow-pane overflow menu applies a configured shortcut to the matching split action.

Changes

Split action keyboard shortcuts

Layer / File(s) Summary
Configure and apply split action shortcuts
Sources/Bonsplit/Public/BonsplitController.swift, Sources/Bonsplit/Internal/Views/TabBarView.swift
BonsplitController adds an initially empty shortcut dictionary keyed by split action. The overflow menu uses a shared builder that preserves each item's action and tooltip and applies its configured shortcut when available.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: lawrencecchen

Merge Risk: ⚪ Minimal · up to 1b525

Configured shortcuts apply to their matching menu actions, and host changes update the menu. No merge-blocking issue is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 1b525

The new shortcuts use existing pane actions and retain their interactivity and split-permission checks. No privilege expansion was established, but keyboard routing across hidden menus and multiple panes remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is configured actions in narrow-pane menu instances, including host-defined custom callbacks. Multiple panes can receive the same action-keyed shortcut; source establishes each callback's pane identity, but not which instance wins keyboard routing.

Trust Boundaries and Controls

  • observed — The host supplies shortcut values, while configured buttons supply action identities. Keystrokes invoke existing callbacks rather than supplying action identifiers or executable code. The action sink and its checks are shared with existing mouse controls.

Resilience and Maintainability Implications

  • observed — Noninteractive dispatch returns before issuing an action request, and split permission or delegate rejection occurs before the split mutation. These existing rejection controls remain on the shortcut path.

Hardening Proposals

  • proposed — Establish hidden-menu and duplicate-shortcut routing behavior before a host relies on visibility or focused-pane selection to constrain sensitive custom actions. This is a validation proposal, not an observed control bypass.
🚥 Pre-merge checks | ✅ 4 | ❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: displaying split action shortcuts in the collapsed menu.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

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