From fc37e86f04da6d831de98e3e647a7cab5f09e5e8 Mon Sep 17 00:00:00 2001 From: austinywang Date: Fri, 8 May 2026 17:17:45 -0700 Subject: [PATCH 1/2] Restore right-sidebar shortcut intent The remappable right-sidebar shortcuts had drifted: the action labeled Toggle Right Sidebar only moved focus back to the terminal, and Open File Explorer reused the last right-sidebar mode instead of selecting Files. Route those shortcuts through explicit shared actions so Cmd-Option-B toggles visibility and Cmd-Shift-E opens the Files mode regardless of the previous mode. Constraint: Do not include the pre-existing vendor/bonsplit submodule dirty state Rejected: Keep Cmd-Shift-E as a generic sidebar visibility toggle | it preserves stale Find mode and violates the action label Confidence: high Scope-risk: narrow Directive: Keep right-sidebar shortcut labels aligned with visibility and mode behavior; do not route Open File Explorer through a mode-preserving toggle Tested: ./scripts/reload.sh --tag fix-right-sidebar-toggle --launch Not-tested: Local unit test suite per repository testing policy --- Sources/AppDelegate.swift | 60 ++++----- Sources/cmuxApp.swift | 2 +- .../AppDelegateShortcutRoutingTests.swift | 125 ++++++++++++++++++ 3 files changed, 150 insertions(+), 37 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 07ffed19be7e..706db5cf5e4d 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -5496,32 +5496,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return false } - @discardableResult - func toggleRightSidebarInActiveMainWindow(preferredWindow: NSWindow? = nil) -> Bool { - guard let context = preferredRegisteredMainWindowContext(preferredWindow: preferredWindow) else { - if let fileExplorerState { - fileExplorerState.toggle() - return true - } - return false - } - - let window = context.window ?? windowForMainWindowId(context.windowId) - if let window { - setActiveMainWindow(window) - } - - guard let state = context.fileExplorerState ?? fileExplorerState else { - return false - } - let wasVisible = state.isVisible - state.toggle() - if wasVisible && !state.isVisible { - _ = context.keyboardFocusCoordinator.restoreTerminalFocusAfterRightSidebarHiddenIfNeeded() - } - return true - } - @discardableResult func restoreTerminalFocusAfterRightSidebarHidden(in window: NSWindow?) -> Bool { let context = preferredRegisteredMainWindowContext(preferredWindow: window) @@ -5823,13 +5797,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } @discardableResult - func toggleRightSidebarKeyboardFocusInActiveMainWindow(preferredWindow: NSWindow? = nil) -> Bool { + func toggleRightSidebarVisibilityInActiveMainWindow(preferredWindow: NSWindow? = nil) -> Bool { let context = preferredRegisteredMainWindowContext(preferredWindow: preferredWindow) guard let context else { #if DEBUG dlog( - "rs.focus.toggle.abort reason=noContext preferred={\(debugWindowToken(preferredWindow))} " + + "rs.visibility.toggle.abort reason=noContext preferred={\(debugWindowToken(preferredWindow))} " + "\(debugShortcutRouteSnapshot())" ) #endif @@ -5839,19 +5813,29 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent #if DEBUG let beforeResponder = window?.firstResponder.map { String(describing: type(of: $0)) } ?? "nil" dlog( - "rs.focus.toggle.begin preferred={\(debugWindowToken(preferredWindow))} " + + "rs.visibility.toggle.begin preferred={\(debugWindowToken(preferredWindow))} " + "context={\(debugContextToken(context))} targetWin={\(debugWindowToken(window))} " + "fr=\(beforeResponder)" ) #endif + guard let state = context.fileExplorerState ?? fileExplorerState else { + return false + } + + if state.isVisible { + state.setVisible(false) + _ = context.keyboardFocusCoordinator.restoreTerminalFocusAfterRightSidebarHiddenIfNeeded() + return true + } + if let window { mainWindowVisibilityController.focusForInWindowCommand(window, reason: .rightSidebarToggle) } - let result = context.keyboardFocusCoordinator.toggleRightSidebarOrTerminalFocus() + let result = context.keyboardFocusCoordinator.focusRightSidebar() #if DEBUG let afterResponder = window?.firstResponder.map { String(describing: type(of: $0)) } ?? "nil" dlog( - "rs.focus.toggle.end result=\(result ? 1 : 0) " + + "rs.visibility.toggle.end result=\(result ? 1 : 0) " + "targetWin={\(debugWindowToken(window))} fr=\(afterResponder)" ) #endif @@ -11105,11 +11089,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent if matchConfiguredShortcut(event: event, action: .toggleFileExplorer) { // Escape AppKit's performKeyEquivalent animation context. Without - // deferring the toggle, NSAnimationContext implicitly animates the + // deferring the reveal, NSAnimationContext implicitly animates the // layout change. let preferredWindow = mainWindowForShortcutEvent(event) ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow Task { @MainActor [weak self, weak preferredWindow] in - _ = self?.toggleRightSidebarInActiveMainWindow(preferredWindow: preferredWindow) + _ = self?.focusRightSidebarInActiveMainWindow( + mode: .files, + focusFirstItem: true, + preferredWindow: preferredWindow + ) } return true } @@ -11121,18 +11109,18 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent ?? NSApp.keyWindow?.firstResponder ?? NSApp.mainWindow?.firstResponder dlog( - "rs.focus.toggle.shortcut.begin event=\(NSWindow.keyDescription(event)) " + + "rs.visibility.toggle.shortcut.begin event=\(NSWindow.keyDescription(event)) " + "preferred={\(debugWindowToken(preferredWindow))} fr=\(beforeResponder.map { String(describing: type(of: $0)) } ?? "nil") " + "\(debugShortcutRouteSnapshot(event: event))" ) #endif - let result = toggleRightSidebarKeyboardFocusInActiveMainWindow(preferredWindow: preferredWindow) + let result = toggleRightSidebarVisibilityInActiveMainWindow(preferredWindow: preferredWindow) #if DEBUG let afterResponder = preferredWindow?.firstResponder ?? NSApp.keyWindow?.firstResponder ?? NSApp.mainWindow?.firstResponder dlog( - "rs.focus.toggle.shortcut.end result=\(result ? 1 : 0) " + + "rs.visibility.toggle.shortcut.end result=\(result ? 1 : 0) " + "preferred={\(debugWindowToken(preferredWindow))} fr=\(afterResponder.map { String(describing: type(of: $0)) } ?? "nil") " + "\(debugShortcutRouteSnapshot(event: event))" ) diff --git a/Sources/cmuxApp.swift b/Sources/cmuxApp.swift index a9a42c4cac3c..780485ed1ae4 100644 --- a/Sources/cmuxApp.swift +++ b/Sources/cmuxApp.swift @@ -645,7 +645,7 @@ struct cmuxApp: App { } splitCommandButton(title: String(localized: "menu.view.focusRightSidebar", defaultValue: "Toggle Right Sidebar"), shortcut: menuShortcut(for: .focusRightSidebar)) { - if AppDelegate.shared?.toggleRightSidebarKeyboardFocusInActiveMainWindow() != true { + if AppDelegate.shared?.toggleRightSidebarVisibilityInActiveMainWindow() != true { if AppDelegate.shared?.focusRightSidebarInActiveMainWindow( preferredWindow: NSApp.keyWindow ?? NSApp.mainWindow ) != true { diff --git a/cmuxTests/AppDelegateShortcutRoutingTests.swift b/cmuxTests/AppDelegateShortcutRoutingTests.swift index 4d55361bc4fa..af9cfe9cf13d 100644 --- a/cmuxTests/AppDelegateShortcutRoutingTests.swift +++ b/cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -4484,6 +4484,131 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { } } + func testFocusRightSidebarShortcutTogglesRightSidebarVisibility() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let originalVisibility = UserDefaults.standard.object(forKey: "fileExplorer.isVisible") + defer { + restoreDefaultsValue(originalVisibility, forKey: "fileExplorer.isVisible", defaults: .standard) + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId), + let state = appDelegate.fileExplorerState else { + XCTFail("Expected test window and right sidebar state") + return + } + + window.makeKeyAndOrderFront(nil) + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05)) + state.setVisible(false) + + guard let showEvent = makeKeyDownEvent( + key: "b", + modifiers: [.command, .option], + keyCode: 11, + windowNumber: window.windowNumber + ), let hideEvent = makeKeyDownEvent( + key: "b", + modifiers: [.command, .option], + keyCode: 11, + windowNumber: window.windowNumber + ) else { + XCTFail("Failed to construct right sidebar toggle shortcut events") + return + } + + #if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: showEvent)) + #else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") + #endif + XCTAssertTrue(state.isVisible, "First shortcut press should show the right sidebar") + + #if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: hideEvent)) + #else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") + #endif + XCTAssertFalse(state.isVisible, "Second shortcut press should hide the right sidebar") + } + + func testOpenFileExplorerShortcutSelectsFilesModeWhenFindWasActive() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let originalVisibility = UserDefaults.standard.object(forKey: "fileExplorer.isVisible") + let originalMode = UserDefaults.standard.object(forKey: "rightSidebar.mode") + defer { + restoreDefaultsValue(originalVisibility, forKey: "fileExplorer.isVisible", defaults: .standard) + restoreDefaultsValue(originalMode, forKey: "rightSidebar.mode", defaults: .standard) + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId), + let state = appDelegate.fileExplorerState else { + XCTFail("Expected test window and right sidebar state") + return + } + + window.makeKeyAndOrderFront(nil) + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05)) + state.mode = .find + state.setVisible(false) + + guard let hiddenEvent = makeKeyDownEvent( + key: "e", + modifiers: [.command, .shift], + keyCode: 14, + windowNumber: window.windowNumber + ) else { + XCTFail("Failed to construct Open File Explorer shortcut event") + return + } + + #if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: hiddenEvent)) + #else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") + #endif + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05)) + + XCTAssertTrue(state.isVisible, "Open File Explorer should reveal the right sidebar") + XCTAssertEqual(state.mode, .files, "Open File Explorer must select Files, not the previously active Find mode") + + state.mode = .find + state.setVisible(true) + + guard let visibleEvent = makeKeyDownEvent( + key: "e", + modifiers: [.command, .shift], + keyCode: 14, + windowNumber: window.windowNumber + ) else { + XCTFail("Failed to construct second Open File Explorer shortcut event") + return + } + + #if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: visibleEvent)) + #else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") + #endif + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05)) + + XCTAssertTrue(state.isVisible, "Open File Explorer should keep the right sidebar open") + XCTAssertEqual(state.mode, .files, "Open File Explorer should switch an already visible right sidebar to Files") + } + func testBrowserFindCommandPreflightConsultsConfiguredFindFamilyShortcuts() { #if DEBUG let cases: [(action: KeyboardShortcutSettings.Action, modifiers: NSEvent.ModifierFlags, key: String, keyCode: UInt16)] = [ From b470f2bb814b33fb0336c1f34f0a88cb1c1b0b32 Mon Sep 17 00:00:00 2001 From: austinywang Date: Fri, 8 May 2026 17:20:40 -0700 Subject: [PATCH 2/2] ok --- vendor/bonsplit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/bonsplit b/vendor/bonsplit index f65eccb2e4bf..90f4981f6a99 160000 --- a/vendor/bonsplit +++ b/vendor/bonsplit @@ -1 +1 @@ -Subproject commit f65eccb2e4bf3e77662902f7c44a650fba3b3540 +Subproject commit 90f4981f6a990a2282b2433a67f32edc89306c36