-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
[codex] Expose equalize splits as a keyboard shortcut #3200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -693,6 +693,14 @@ struct cmuxApp: App { | |
| performBrowserSplitFromMenu(direction: .down) | ||
| } | ||
|
|
||
| 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 | ||
| } | ||
| } | ||
|
Comment on lines
+696
to
+702
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This menu-item action shares the same beep-on-no-op problem as the keyboard handler: splitCommandButton(title: String(localized: "command.equalizeSplits.title", defaultValue: "Equalize Splits"), shortcut: menuShortcut(for: .equalizeSplits)) {
if let workspace = activeTabManager.selectedWorkspace {
_ = activeTabManager.equalizeSplits(tabId: workspace.id)
}
} |
||
|
|
||
| Divider() | ||
|
|
||
| // Numbered workspace selection (9 = last workspace) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import XCTest | ||
| import Bonsplit | ||
|
|
||
| #if canImport(cmux_DEV) | ||
| @testable import cmux_DEV | ||
|
|
@@ -11,6 +12,16 @@ private final class FakeWKInspectorContainerView: NSView {} | |
| private final class FocusableTestView: NSView { | ||
| override var acceptsFirstResponder: Bool { true } | ||
| } | ||
|
|
||
| private func shortcutRoutingSplitNodes(in node: ExternalTreeNode) -> [ExternalSplitNode] { | ||
| switch node { | ||
| case .pane: | ||
| return [] | ||
| case .split(let split): | ||
| return [split] + shortcutRoutingSplitNodes(in: split.first) + shortcutRoutingSplitNodes(in: split.second) | ||
| } | ||
| } | ||
|
|
||
| private final class GhosttyCommandEquivalentProbeView: GhosttyNSView { | ||
| var afterMenuMissCallCount = 0 | ||
| var pasteCallCount = 0 | ||
|
|
@@ -680,6 +691,63 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { | |
| XCTAssertEqual(workspace.panels.count, initialPanelCount, "Unmatched chord suffix must not trigger the action") | ||
| } | ||
|
|
||
| func testConfiguredEqualizeSplitsShortcutBalancesWorkspaceDividers() { | ||
| guard let appDelegate = AppDelegate.shared else { | ||
| XCTFail("Expected AppDelegate.shared") | ||
| return | ||
| } | ||
|
|
||
| let windowId = appDelegate.createMainWindow() | ||
| defer { closeWindow(withId: windowId) } | ||
|
|
||
| guard let window = window(withId: windowId), | ||
| let manager = appDelegate.tabManagerFor(windowId: windowId), | ||
| let workspace = manager.selectedWorkspace, | ||
| let leftPanelId = workspace.focusedPanelId, | ||
| let rightPanel = workspace.newTerminalSplit(from: leftPanelId, orientation: .horizontal), | ||
| workspace.newTerminalSplit(from: rightPanel.id, orientation: .vertical) != nil else { | ||
| XCTFail("Expected nested split setup") | ||
| return | ||
| } | ||
|
|
||
| window.makeKeyAndOrderFront(nil) | ||
| RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05)) | ||
|
|
||
| let seededSplits = shortcutRoutingSplitNodes(in: workspace.bonsplitController.treeSnapshot()) | ||
| XCTAssertGreaterThanOrEqual(seededSplits.count, 2, "Expected nested splits") | ||
|
|
||
| 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) | ||
|
Comment on lines
+719
to
+747
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assert seeded layout is actually uneven before invoking equalize.
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 |
||
| } | ||
| } | ||
|
|
||
| func testCreateMainWindowDoesNotDisallowFullScreenTilingByDefault() { | ||
| guard let appDelegate = AppDelegate.shared else { | ||
| XCTFail("Expected AppDelegate.shared") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
equalizeSplits(tabId:)returnsfalsein two distinct cases: the tab wasn't found, and the workspace has no splits at all (foundSplitstaysfalse→false && allSucceeded). The current guard treats both as failures and firesNSSound.beep(), so a user with a single unsplit pane gets an audible error every time they press Cmd+Ctrl+=.The parallel
toggleSplitZoomhandler 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 niltabManager/workspace rather than on the action's Boolean result—would avoid the unexpected beep.