[codex] Expose equalize splits as a keyboard shortcut - #3200
Closed
simpsonkorea wants to merge 2 commits into
Closed
simpsonkorea wants to merge 2 commits into
simpsonkorea wants to merge 2 commits into
Conversation
Ghostty already has an equalize_splits action, but cmux did not expose an editable app-level shortcut for restoring split pane divider balance. Route Cmd+Ctrl+= through the cmux shortcut layer to the existing TabManager equalize implementation, show that shortcut in the command palette/menu/docs, and allow settings.json overrides via the existing shortcut registry. Constraint: New cmux-owned shortcuts must be in KeyboardShortcutSettings, settings.json schema, and docs. Rejected: Rely on the existing RPC workaround | not discoverable for keyboard-driven pane workflows Confidence: medium Scope-risk: narrow Directive: Keep equalize_splits wired to the existing TabManager.equalizeSplits path unless split layout ownership changes. Tested: git diff --check; JSON parse for web/data/cmux-settings.schema.json and Resources/Localizable.xcstrings Not-tested: Full Xcode app build blocked locally because Zig is not installed and GhosttyKit.xcframework is absent
|
Someone is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
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 (1)
📝 WalkthroughWalkthroughAdds a new keyboard shortcut action Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppDelegate
participant TabManager
participant Workspace
participant SystemAudio
User->>AppDelegate: Press ⌘+⌃+= (equalizeSplits)
AppDelegate->>TabManager: equalizeSplits(tabId: selectedWorkspaceId)
TabManager->>Workspace: applyEqualSizes()
Workspace-->>TabManager: success / failure
alt success
TabManager-->>AppDelegate: true
AppDelegate-->>User: consume event (no further handling)
else failure or missing workspace
TabManager-->>AppDelegate: false
AppDelegate->>SystemAudio: NSSound.beep()
AppDelegate-->>User: consume event
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
simpsonkorea
marked this pull request as ready for review
April 29, 2026 05:56
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmuxTests/AppDelegateShortcutRoutingTests.swift`:
- Around line 719-747: Before invoking debugHandleCustomShortcut, assert the
seeded layout is actually uneven by checking the divider positions returned from
shortcutRoutingSplitNodes(in: workspace.bonsplitController.treeSnapshot()) (or
by querying each split's dividerPosition after calling
bonsplitController.setDividerPosition) and failing the test if all splits are
0.5; this ensures setDividerPosition (used in the loop over seededSplits and its
targetPosition) actually changed at least one split away from 0.5 before you
call appDelegate.debugHandleCustomShortcut(event:) and verify equalization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d79ddbc3-06c4-4fc1-92e6-a6750b22e8fe
📒 Files selected for processing (7)
Sources/AppDelegate.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/cmuxApp.swiftcmuxTests/AppDelegateShortcutRoutingTests.swiftweb/data/cmux-settings.schema.jsonweb/data/cmux-shortcuts.ts
Comment on lines
+719
to
+747
| for (index, split) in seededSplits.enumerated() { | ||
| guard let splitId = UUID(uuidString: split.id) else { | ||
| XCTFail("Expected split ID to be a UUID") | ||
| return | ||
| } | ||
| let targetPosition: CGFloat = index.isMultiple(of: 2) ? 0.2 : 0.8 | ||
| XCTAssertTrue(workspace.bonsplitController.setDividerPosition(targetPosition, forSplit: splitId)) | ||
| } | ||
|
|
||
| guard let event = makeKeyDownEvent( | ||
| key: "=", | ||
| modifiers: [.command, .control], | ||
| keyCode: 24, | ||
| windowNumber: window.windowNumber | ||
| ) else { | ||
| XCTFail("Failed to construct Cmd+Ctrl+= event") | ||
| return | ||
| } | ||
|
|
||
| #if DEBUG | ||
| XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event)) | ||
| #else | ||
| XCTFail("debugHandleCustomShortcut is only available in DEBUG") | ||
| #endif | ||
|
|
||
| let equalizedSplits = shortcutRoutingSplitNodes(in: workspace.bonsplitController.treeSnapshot()) | ||
| XCTAssertEqual(equalizedSplits.count, seededSplits.count) | ||
| for split in equalizedSplits { | ||
| XCTAssertEqual(split.dividerPosition, 0.5, accuracy: 0.000_1) |
There was a problem hiding this comment.
Assert seeded layout is actually uneven before invoking equalize.
setDividerPosition(...) == true alone doesn’t guarantee the layout moved away from 0.5. Without a precondition check, this test can false-pass if seeding becomes a no-op.
Suggested patch
for (index, split) in seededSplits.enumerated() {
guard let splitId = UUID(uuidString: split.id) else {
XCTFail("Expected split ID to be a UUID")
return
}
let targetPosition: CGFloat = index.isMultiple(of: 2) ? 0.2 : 0.8
XCTAssertTrue(workspace.bonsplitController.setDividerPosition(targetPosition, forSplit: splitId))
}
+
+ let postSeedSplits = shortcutRoutingSplitNodes(in: workspace.bonsplitController.treeSnapshot())
+ XCTAssertTrue(
+ postSeedSplits.contains(where: { abs($0.dividerPosition - 0.5) > 0.000_1 }),
+ "Precondition failed: expected at least one non-equal divider before triggering equalize shortcut"
+ )
guard let event = makeKeyDownEvent(
key: "=",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmuxTests/AppDelegateShortcutRoutingTests.swift` around lines 719 - 747,
Before invoking debugHandleCustomShortcut, assert the seeded layout is actually
uneven by checking the divider positions returned from
shortcutRoutingSplitNodes(in: workspace.bonsplitController.treeSnapshot()) (or
by querying each split's dividerPosition after calling
bonsplitController.setDividerPosition) and failing the test if all splits are
0.5; this ensures setDividerPosition (used in the loop over seededSplits and its
targetPosition) actually changed at least one split away from 0.5 before you
call appDelegate.debugHandleCustomShortcut(event:) and verify equalization.
Contributor
Greptile SummaryThis PR exposes
Confidence Score: 3/5Not safe to merge as-is; the beep-on-no-op bug will affect any user who invokes the shortcut without active splits. Two P1 findings (AppDelegate and cmuxApp both beep when workspace has no splits) lower the score below the P1 ceiling of 4. The core equalization logic itself is correct — the issue is solely in how callers interpret the return value. Sources/AppDelegate.swift and Sources/cmuxApp.swift — both need the beep guard updated to treat a no-split workspace as a silent no-op. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AppDelegate
participant cmuxApp as cmuxApp (View Menu)
participant ContentView as ContentView (Palette)
participant TabManager
User->>AppDelegate: Cmd+Ctrl+= key event
AppDelegate->>AppDelegate: matchConfiguredShortcut(.equalizeSplits)
AppDelegate->>TabManager: equalizeSplits(tabId: workspace.id)
TabManager-->>AppDelegate: Bool (false if no splits)
alt false (no splits or error)
AppDelegate->>User: NSSound.beep() ⚠️
else true
AppDelegate->>User: (silent success)
end
User->>cmuxApp: View → Equalize Splits
cmuxApp->>TabManager: equalizeSplits(tabId: workspace.id)
TabManager-->>cmuxApp: Bool (false if no splits)
alt false (no splits or error)
cmuxApp->>User: NSSound.beep() ⚠️
else true
cmuxApp->>User: (silent success)
end
User->>ContentView: Command Palette → palette.equalizeSplits
ContentView->>TabManager: equalizeSplits(tabId: workspace.id)
TabManager-->>ContentView: Bool
alt false
ContentView->>User: NSSound.beep()
else true
ContentView->>User: (silent success)
end
Reviews (1): Last reviewed commit: "Expose equalize splits as a cmux shortcu..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+10612
to
+10619
| if matchConfiguredShortcut(event: event, action: .equalizeSplits) { | ||
| guard let workspace = tabManager?.selectedWorkspace, | ||
| tabManager?.equalizeSplits(tabId: workspace.id) == true else { | ||
| NSSound.beep() | ||
| return true | ||
| } | ||
| return true | ||
| } |
Contributor
There was a problem hiding this comment.
equalizeSplits(tabId:) returns false in two distinct cases: the tab wasn't found, and the workspace has no splits at all (foundSplit stays false → false && allSucceeded). The current guard treats both as failures and fires NSSound.beep(), so a user with a single unsplit pane gets an audible error every time they press Cmd+Ctrl+=.
The parallel toggleSplitZoom handler immediately above discards its return value (_ = tabManager?.toggleFocusedSplitZoom()) and never beeps on a no-op. Aligning with that pattern—or at least guarding only on a nil tabManager/workspace rather than on the action's Boolean result—would avoid the unexpected beep.
if matchConfiguredShortcut(event: event, action: .equalizeSplits) {
if let workspace = tabManager?.selectedWorkspace {
_ = tabManager?.equalizeSplits(tabId: workspace.id)
}
return true
}
Comment on lines
+696
to
+702
| splitCommandButton(title: String(localized: "command.equalizeSplits.title", defaultValue: "Equalize Splits"), shortcut: menuShortcut(for: .equalizeSplits)) { | ||
| guard let workspace = activeTabManager.selectedWorkspace, | ||
| activeTabManager.equalizeSplits(tabId: workspace.id) else { | ||
| NSSound.beep() | ||
| return | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
This menu-item action shares the same beep-on-no-op problem as the keyboard handler: equalizeSplits(tabId:) returns false for an unsplit workspace, triggering NSSound.beep(). A user choosing "Equalize Splits" from the menu with only one pane open will hear an error sound with no visual explanation.
splitCommandButton(title: String(localized: "command.equalizeSplits.title", defaultValue: "Equalize Splits"), shortcut: menuShortcut(for: .equalizeSplits)) {
if let workspace = activeTabManager.selectedWorkspace {
_ = activeTabManager.equalizeSplits(tabId: workspace.id)
}
}
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
4 tasks done
austinywang
added a commit
that referenced
this pull request
Apr 29, 2026
Follow-up to community PR #3200. The shortcut was exposed, but review and manual verification found three rough edges: the regression did not prove the seeded layout was uneven, no-op single-pane invocations beeped, and a single programmatic equalize pass let parent split resizing leave nested split views stale until a later interaction. This keeps the shortcut configurable through KeyboardShortcutSettings and settings.json, gives its Settings label a dedicated localization key, treats single-pane equalize as a silent no-op, and reasserts equalized divider positions on deferred main-queue follow-up passes so nested horizontal and vertical splits settle from one shortcut invocation. Constraint: Follow-up to community PR #3200; push only to manaflow-ai/cmux Rejected: Treat equalize false as user-facing error | single-pane workspaces are a valid no-op Rejected: Only refresh terminal geometry cache | manual verification showed the nested NSSplitView itself still needed a deferred divider reassertion Confidence: high Scope-risk: narrow Tested: git diff --check Tested: JSON parse for Resources/Localizable.xcstrings and web/data/cmux-settings.schema.json Tested: ./scripts/reload.sh --tag codex-equalize-splits-shortcut-followup --launch Tested: Manual tagged build verification with uneven nested horizontal+vertical splits; one Cmd+Ctrl+= changed root 925/405 to 665/665 and nested 298/618 to 458/458 without another click or keypress Not-tested: Local unit tests because repo runner invokes xcodebuild directly
austinywang
added a commit
that referenced
this pull request
May 4, 2026
* Expose equalize splits as a cmux shortcut Ghostty already has an equalize_splits action, but cmux did not expose an editable app-level shortcut for restoring split pane divider balance. Route Cmd+Ctrl+= through the cmux shortcut layer to the existing TabManager equalize implementation, show that shortcut in the command palette/menu/docs, and allow settings.json overrides via the existing shortcut registry. Constraint: New cmux-owned shortcuts must be in KeyboardShortcutSettings, settings.json schema, and docs. Rejected: Rely on the existing RPC workaround | not discoverable for keyboard-driven pane workflows Confidence: medium Scope-risk: narrow Directive: Keep equalize_splits wired to the existing TabManager.equalizeSplits path unless split layout ownership changes. Tested: git diff --check; JSON parse for web/data/cmux-settings.schema.json and Resources/Localizable.xcstrings Not-tested: Full Xcode app build blocked locally because Zig is not installed and GhosttyKit.xcframework is absent * Make equalize splits shortcut merge-ready Follow-up to community PR #3200. The shortcut was exposed, but review and manual verification found three rough edges: the regression did not prove the seeded layout was uneven, no-op single-pane invocations beeped, and a single programmatic equalize pass let parent split resizing leave nested split views stale until a later interaction. This keeps the shortcut configurable through KeyboardShortcutSettings and settings.json, gives its Settings label a dedicated localization key, treats single-pane equalize as a silent no-op, and reasserts equalized divider positions on deferred main-queue follow-up passes so nested horizontal and vertical splits settle from one shortcut invocation. Constraint: Follow-up to community PR #3200; push only to manaflow-ai/cmux Rejected: Treat equalize false as user-facing error | single-pane workspaces are a valid no-op Rejected: Only refresh terminal geometry cache | manual verification showed the nested NSSplitView itself still needed a deferred divider reassertion Confidence: high Scope-risk: narrow Tested: git diff --check Tested: JSON parse for Resources/Localizable.xcstrings and web/data/cmux-settings.schema.json Tested: ./scripts/reload.sh --tag codex-equalize-splits-shortcut-followup --launch Tested: Manual tagged build verification with uneven nested horizontal+vertical splits; one Cmd+Ctrl+= changed root 925/405 to 665/665 and nested 298/618 to 458/458 without another click or keypress Not-tested: Local unit tests because repo runner invokes xcodebuild directly * Allow extracted equalize split support to refresh workspace layout snapshots The helper now lives outside Workspace.swift to satisfy the file-length budget, so the cached tmux layout snapshot needs a module-internal setter for same-module Workspace extensions. Constraint: Workspace.swift must remain within the existing file-length budget.\nRejected: Put didProgrammaticallyChangeSplitGeometry back in Workspace.swift | would regress the budget fix.\nConfidence: high\nScope-risk: narrow\nTested: git diff --check; swift_file_length_budget.py; conflict-marker scan.\nNot-tested: Local unit/UI test suites, per repo policy. * Make tagged reload app names filesystem-safe Branch-style tags can contain slashes. reload.sh already derives a filesystem-safe tag slug for paths, sockets, and DerivedData; use that same slug for the default copied app bundle name so the mandatory branch-name tag can build and launch. Constraint: The user-required reload command passes a branch name containing '/'.\nRejected: Require a different --tag value | task requires the branch name exactly.\nConfidence: high\nScope-risk: narrow\nTested: bash -n scripts/reload.sh; git diff --check scripts/reload.sh.\nNot-tested: Local test suites, per repo policy. * Avoid recursive shortcut store initialization during config parsing Settings-file parsing should normalize action-specific shortcut shapes without consulting the shared settings store that is currently being constructed. Using the existing conflict-ignoring normalization path prevents the launch-time dispatch_once recursion crash while keeping numbered and system-wide shortcut validation. Constraint: KeyboardShortcutSettingsFileStore.swift is at its file-length budget.\nRejected: Use full normalizedRecordedShortcut during config load | it consults KeyboardShortcutSettings.settingsFileStore and recursively initializes the store.\nConfidence: high\nScope-risk: narrow\nTested: git diff --check; swift_file_length_budget.py.\nNot-tested: Local unit/UI test suites, per repo policy. * Keep equalize split geometry state coherent Review feedback showed the equalize shortcut could mutate some divider positions while skipping snapshot refresh, terminal reconciliation, or follow-up passes when another divider failed. The handler now uses the same transient terminal-focus guard as split creation, records DEBUG routing traces, and debug-logs failed no-op paths without surfacing single-pane workspaces to users. Equalize traversal now keeps recursing after malformed split IDs, uses Bonsplit's external-change path, and refreshes workspace geometry whenever a split node was visited. Shortcut config parsing also avoids live system-wide conflict checks while loading settings files so it cannot recurse through the shared store during initialization. Constraint: Single-pane equalize remains a silent no-op for users. Rejected: Surface an alert on false equalize results | The PR behavior explicitly keeps single-pane workspaces silent; DEBUG logging gives diagnostics without changing UX. Confidence: high Scope-risk: moderate Directive: Keep programmatic split geometry changes on the same Workspace splitTabBar(didChangeGeometry:) path so layout snapshot, terminal geometry, and focus reconciliation stay coupled. Tested: git diff --check; ./scripts/reload.sh --tag pr3309-comments Not-tested: Local unit tests; project policy says tests run in CI/prefer CI. * Respect shortcut parsing file length budget The conflict-bypass fix for settings-file parsing was behaviorally correct, but formatting alone pushed two already-budgeted Swift files over the CI line-count guard. Collapse the added parsing arguments back into the existing compact style without changing the recursion fix. Constraint: workflow-guard-tests enforces existing Swift file length budgets. Rejected: Refresh the budget | This PR can stay inside the current budget without accepting new debt. Confidence: high Scope-risk: narrow Tested: git diff --check; python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv; ./scripts/reload.sh --tag pr3309-comments Not-tested: Local unit tests; project policy says tests run in CI/prefer CI. --------- Co-authored-by: Gyusup Sim <simpson.sim@retn.kr>
Collaborator
|
Equalize splits is on main via commit e2f4d36. Thank you for the shortcut implementation :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Ghostty already has equalize_splits, but cmux users cannot reliably discover or use an app-level shortcut for restoring split pane balance. This addresses #2482 by exposing the behavior through cmux's shortcut system instead of relying on the RPC workaround.
Validation
Not tested
Fixes #2482
Summary by cubic
Adds an app-level Equalize Splits shortcut to balance pane dividers. Default is Cmd+Ctrl+=, and it’s configurable via settings. Fixes #2482.
equalizeSplitsinKeyboardShortcutSettingswith a localized label and default Cmd+Ctrl+=.TabManager.equalizeSplitswith a beep fallback when no workspace is active.web/data/cmux-settings.schema.jsonandweb/data/cmux-shortcuts.tsfor schema, hints, and docs; added a regression test that seeds uneven splits and verifies they reset to 0.5 via the shortcut.Written for commit 29ee116. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Configuration
Tests