Skip to content

Restore right sidebar Ctrl shortcuts - #3796

Merged
lawrencecchen merged 3 commits into
mainfrom
task-ctrl-shortcuts-right-sidebar
May 9, 2026
Merged

lawrencecchen merged 3 commits into
mainfrom
task-ctrl-shortcuts-right-sidebar

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented May 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Restore private Ctrl+1 through Ctrl+5 defaults for right-sidebar mode switching.
  • Add regression tests proving the shortcuts stay hidden from public Settings while defaults work.
  • Add coverage that cmux.json can still remap the hidden right-sidebar shortcut actions.

Testing

  • git diff --check
  • ./scripts/reload.sh --tag rsctrl
  • Not run: XCTest locally per repo policy.

Note

Low Risk
Low risk change limited to keyboard shortcut defaults and tests; main concern is potential shortcut collisions with existing Ctrl+digit bindings on some layouts or user configurations.

Overview
Restores hidden default shortcuts for right-sidebar mode switching by binding switchRightSidebarTo{Files,Find,Sessions,Feed,Dock} to Ctrl+1..5 instead of leaving them unbound.

Updates and extends regression tests to assert these actions remain non-public/non-settings-visible, still resolve via the default Ctrl+digit bindings, and can be remapped via both UserDefaults and cmux.json overrides.

Reviewed by Cursor Bugbot for commit 06e88fa. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Restore private Ctrl+1-5 shortcuts to switch right sidebar modes. They stay hidden from Settings and can still be remapped via cmux.json.

  • Bug Fixes
    • Set default Ctrl+1-5 for Files/Find/Sessions/Feed/Dock.
    • Keep these actions non-public so they don’t show in Settings.
    • Tighten regression tests for defaults, Settings visibility, and cmux.json override behavior.

Written for commit 08b3ddf. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Default keyboard shortcuts for right‑sidebar mode switching are now enabled: Ctrl+1 through Ctrl+5.
  • Tests

    • Updated and added tests to verify the new default shortcuts and that user-configured overrides are respected.

Review Change Stack

@vercel

vercel Bot commented May 9, 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 May 9, 2026 9:38am
cmux-staging Building Building Preview, Comment May 9, 2026 9:38am

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented May 9, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1c03d5fa-0fcd-46b1-a29d-8907bb2c3788

📥 Commits

Reviewing files that changed from the base of the PR and between 06e88fa and 08b3ddf.

📒 Files selected for processing (1)
  • cmuxTests/ShortcutAndCommandPaletteTests.swift

📝 Walkthrough

Walkthrough

Right-sidebar mode switch action defaults are updated from unbound to Ctrl+1–5 in keyboard shortcut settings; tests verify Control+digit key events resolve to the expected modes and that a custom settings file binding overrides the default.

Changes

Right-Sidebar Mode Shortcut Defaults

Layer / File(s) Summary
Default Binding Configuration
Sources/KeyboardShortcutSettings.swift
switchRightSidebarToFiles/Find/Sessions/Feed/Dock actions now default to Ctrl + 1…5 instead of .unbound.
Test Coverage
cmuxTests/ShortcutAndCommandPaletteTests.swift, cmuxTests/WorkspaceUnitTests.swift
New tests verify Control+digit key events map to expected sidebar modes, custom settings file bindings override defaults, and each action has correct private default binding with only Control modifier enabled.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • manaflow-ai/cmux#3728: Directly related—both modify default bindings and test expectations for the same right-sidebar mode actions.
  • manaflow-ai/cmux#3408: Related—adds command-palette commands that map to and use these right-sidebar mode shortcut actions.
  • manaflow-ai/cmux#3468: Related—refactors KeyboardShortcutSettings.Action conflict and matching logic alongside shortcut binding changes.

Poem

🐰 Ctrl held tight, five digits hop in line,
Modes awaken: files, find, sessions, feed, dock shine.
From unbound quiet to shortcuts now alive,
Tests nibble proofs so keypresses thrive.
Hooray — a small hop that helps navigation jive!

🚥 Pre-merge checks | ✅ 14 | ❌ 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 (14 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: restoring Ctrl shortcuts for the right sidebar mode switching actions (Ctrl+1–5).
Description check ✅ Passed The description covers the summary, testing approach, includes detailed context via Cursor and cubic summaries, and provides a checklist. However, the Testing section lacks detail on how to verify the change manually, a demo video is not applicable here, and the checklist items are not formally checked.
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.
Cmux Swift Actor Isolation ✅ Passed PR updates keyboard shortcut defaults in computed properties returning StoredShortcut value structs. No shared mutable state, implicit MainActor isolation, or Sendable violations.
Cmux Swift Blocking Runtime ✅ Passed No blocking/timing synchronization patterns introduced in production code. Changes limited to keyboard shortcut default assignments and test scaffolding.
Cmux No Hacky Sleeps ✅ Passed Not applicable. All changes are Swift files (.swift). Check covers TypeScript, JavaScript, shell, or runtime scripts only. Swift code is covered separately.
Cmux Swift Concurrency ✅ Passed Task pattern at OS callback boundary (allowed exception). Test DispatchQueue patterns for XCTest sync (allowed exception). No legacy async patterns that should be modernized.
Cmux Swift @Concurrent ✅ Passed The PR makes purely configuration/test changes to keyboard shortcut defaults with no modifications to Swift concurrency annotations, async function declarations, or actor isolation patterns.
Cmux Swift File And Package Boundaries ✅ Passed Minimal focused changes to existing oversized KeyboardShortcutSettings.swift (+2 net lines) restoring Ctrl+1-5 defaults. Only affects keyboard shortcut default definitions. Test updates are standard.
Cmux Swift Logging ✅ Passed Production Swift changes contain only debug-only logging via cmuxDebugLog() guarded by #if DEBUG. No print, NSLog, dump, debugPrint, file I/O, or unguarded logging violations found.
Cmux Swiftui State Layout ✅ Passed PR updates keyboard shortcut defaults in a static enum. No SwiftUI state patterns, GeometryReader, lazy views, or render-time mutations introduced. Changes are data structures and logic only.
Cmux Architecture Rethink ✅ Passed Restores Ctrl+1-5 defaults for sidebar shortcuts. No timing repairs, locks, observers, split ownership, or mutable state. Clean fix with clear owner. Passes architectural rethink rules.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR modifies keyboard shortcut defaults only. No auxiliary windows are created or materially changed. NSWindow usage appears only in test fixtures (explicitly allowed). Check not applicable.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task-ctrl-shortcuts-right-sidebar

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 and usage tips.

@greptile-apps

greptile-apps Bot commented May 9, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Restores private Ctrl+1–5 default shortcuts for right-sidebar mode switching (switchRightSidebarToFiles through switchRightSidebarToDock) and updates tests to assert the new defaults work, stay hidden from the Settings UI, and can be overridden via cmux.json.

  • KeyboardShortcutSettings.swift: Replaces the five .unbound defaults with explicit ctrl+1..5 StoredShortcut values, one per sidebar mode.
  • WorkspaceUnitTests.swift: Renames and updates the existing regression test to assert the key/modifier values instead of checking for .unbound.
  • ShortcutAndCommandPaletteTests.swift: Renames the existing dispatch test and adds testModeShortcutUsesSettingsFileBindings which verifies that a cmux.json override for one action displaces only that action's default, leaving all four sibling defaults intact — closing the gap identified in the previous review.

Confidence Score: 5/5

Safe to merge; the change is limited to assigning five default shortcut values and updating test assertions to match.

The production change is a one-to-one substitution of five .unbound entries with concrete StoredShortcut values. The dispatch gate (shouldRouteRightSidebarModeShortcut) already handled the key-sharing with selectSurfaceByNumber before this PR and is unchanged. All three changed files have solid test coverage, and the previous review gaps (sibling-default isolation after a partial override) are closed by the new testModeShortcutUsesSettingsFileBindings test.

No files require special attention.

Important Files Changed

Filename Overview
Sources/KeyboardShortcutSettings.swift Replaces five .unbound defaults with explicit ctrl+1..5 StoredShortcut values; the actions remain non-public and the dispatch gate (shouldRouteRightSidebarModeShortcut) continues to guard against collisions with selectSurfaceByNumber.
cmuxTests/ShortcutAndCommandPaletteTests.swift Renames the default-check test to match new semantics and adds testModeShortcutUsesSettingsFileBindings, which now verifies all four unmodified sibling defaults still resolve after a partial cmux.json override.
cmuxTests/WorkspaceUnitTests.swift Renames and extends the private-defaults regression test to assert key + all four modifier booleans per action, plus the existing isPublicShortcutAction / settingsVisibleActions guards.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["NSEvent (keyDown, Ctrl+1..5)"] --> B["AppDelegate.handleCustomShortcut"]
    B --> C{"RightSidebarMode.modeShortcut(for:)"}
    C -- "nil" --> E["Fall through to selectSurfaceByNumber"]
    C -- "mode (files/find/...)" --> D{"shouldRouteRightSidebarModeShortcut"}
    D -- "false (terminal focused)" --> E
    D -- "true (sidebar focused)" --> F["focusRightSidebarInActiveMainWindow(mode:)"]
    E --> G["tabManager?.selectSurface(at:)"]
    F --> H["Right sidebar switches to mode"]
Loading

Reviews (2): Last reviewed commit: "Tighten right sidebar settings override ..." | Re-trigger Greptile

Comment thread cmuxTests/ShortcutAndCommandPaletteTests.swift
Comment thread Sources/KeyboardShortcutSettings.swift
@lawrencecchen
lawrencecchen merged commit 0e4277f into main May 9, 2026
29 checks passed
@lawrencecchen
lawrencecchen deleted the task-ctrl-shortcuts-right-sidebar branch May 9, 2026 09:48

This branch was successfully deployed

1 active deployment
Preview – cmux — 08b3ddfe Deployed May 9, 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