Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 56 additions & 7 deletions Sources/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10890,6 +10890,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
static func installWindowResponderSwizzlesForTesting() {
_ = didInstallApplicationAccessibilitySwizzle
_ = didInstallApplicationSendActionSwizzle
_ = didInstallApplicationSendEventSwizzle
_ = didInstallWindowKeyEquivalentSwizzle
_ = didInstallWindowFirstResponderSwizzle
_ = didInstallWindowSendEventSwizzle
Expand Down Expand Up @@ -13098,6 +13099,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
/// through the same app-level shortcut handler used by the local key monitor.
@discardableResult
func handleBrowserSurfaceKeyEquivalent(_ event: NSEvent) -> Bool {
handleConfiguredShortcutKeyEquivalent(event)
}

/// Route AppKit key-equivalent fallbacks through the same configured shortcut
/// dispatcher as the local key monitor before any stale menu item can run.
@discardableResult
func handleConfiguredShortcutKeyEquivalent(_ event: NSEvent) -> Bool {
handleCustomShortcut(event: event)
}

Expand Down Expand Up @@ -13526,25 +13534,54 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
.subtracting([.numericPad, .function, .capsLock])
guard flags.contains(.command) else { return false }

for action in KeyboardShortcutSettings.Action.allCases {
let currentShortcut = KeyboardShortcutSettings.shortcut(for: action)
if matchesKeyboardShortcutEvent(event, action: action, shortcut: currentShortcut) {
let staleDefaultActions = KeyboardShortcutSettings.Action.allCases.filter { action in
isMenuBackedShortcutAction(action) &&
matchesKeyboardShortcutEvent(event, action: action, shortcut: action.defaultShortcut)
}
guard !staleDefaultActions.isEmpty else { return false }

for action in staleDefaultActions {
if currentShortcutMatchesKeyboardShortcutEvent(event, action: action) {
return false
}
}

for action in KeyboardShortcutSettings.Action.allCases where isMenuBackedShortcutAction(action) {
if matchesKeyboardShortcutEvent(event, action: action, shortcut: action.defaultShortcut) {
return true
if staleDefaultActions.contains(where: isCloseShortcutAction) {
return true
}

for action in KeyboardShortcutSettings.Action.allCases {
if currentShortcutMatchesKeyboardShortcutEvent(event, action: action) {
return false
}
}
return false
return true
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

private func currentShortcutMatchesKeyboardShortcutEvent(
_ event: NSEvent,
action: KeyboardShortcutSettings.Action
) -> Bool {
let currentShortcut = KeyboardShortcutSettings.shortcut(for: action)
if action.usesNumberedDigitMatching {
return numberedShortcutDigit(event: event, shortcut: currentShortcut) != nil
}
return matchesKeyboardShortcutEvent(event, action: action, shortcut: currentShortcut)
}

private func isMenuBackedShortcutAction(_ action: KeyboardShortcutSettings.Action) -> Bool {
action != .showHideAllWindows && action != .globalSearch
}

private func isCloseShortcutAction(_ action: KeyboardShortcutSettings.Action) -> Bool {
switch action {
case .closeTab, .closeWorkspace, .closeWindow:
return true
default:
return false
}
}
Comment thread
austinywang marked this conversation as resolved.

private func numberedShortcutDigit(event: NSEvent, stroke: ShortcutStroke) -> Int? {
let flags = event.modifierFlags.intersection(.deviceIndependentFlagsMask)
.subtracting([.numericPad, .function, .capsLock])
Expand Down Expand Up @@ -14734,6 +14771,12 @@ private extension NSApplication {
return
}
if AppDelegate.shared?.shouldSuppressStaleCmuxMenuShortcut(event: event) == true {
if AppDelegate.shared?.handleConfiguredShortcutKeyEquivalent(event) == true {
#if DEBUG
cmuxDebugLog("app.sendEvent routed configured shortcut before stale cmux menu shortcut")
#endif
return
}
let responder = event.window?.firstResponder
?? keyWindow?.firstResponder
?? mainWindow?.firstResponder
Expand Down Expand Up @@ -15284,6 +15327,12 @@ private extension NSWindow {
return true
}
if AppDelegate.shared?.shouldSuppressStaleCmuxMenuShortcut(event: event) == true {
if AppDelegate.shared?.handleConfiguredShortcutKeyEquivalent(event) == true {
#if DEBUG
cmuxDebugLog(" → consumed by configured shortcut before stale cmux menu shortcut")
#endif
return true
}
if let firstResponderGhosttyView {
firstResponderGhosttyView.keyDown(with: event)
#if DEBUG
Expand Down
167 changes: 167 additions & 0 deletions cmuxTests/AppDelegateShortcutRoutingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5656,6 +5656,144 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
}
}

func testCurrentNumberedDigitShortcutIsNotSuppressedAsStaleMenuShortcut() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}

guard let event = makeKeyDownEvent(
key: "2",
modifiers: [.command],
keyCode: 19,
windowNumber: 0
) else {
XCTFail("Failed to construct Cmd+2 event")
return
}

let remappedWorkspaceNumber = StoredShortcut(
key: "1",
command: false,
shift: false,
option: false,
control: true
)
let currentSurfaceNumber = StoredShortcut(
key: "1",
command: true,
shift: false,
option: false,
control: false
)

withTemporaryShortcut(action: .selectWorkspaceByNumber, shortcut: remappedWorkspaceNumber) {
withTemporaryShortcut(action: .selectSurfaceByNumber, shortcut: currentSurfaceNumber) {
XCTAssertFalse(
appDelegate.shouldSuppressStaleCmuxMenuShortcut(event: event),
"A current numbered-digit shortcut must own Cmd+2 before stale menu suppression"
)
}
}
}

func testStaleCloseDefaultShortcutsSuppressMenuFallbackAfterReassignment() {
assertStaleCloseDefaultShortcutSuppressesMenuFallback(
staleAction: .closeTab,
replacementAction: .newTab,
replacementShortcut: StoredShortcut(key: "w", command: true, shift: false, option: false, control: false),
remappedStaleShortcut: StoredShortcut(key: "w", command: true, shift: false, option: true, control: false)
)

assertStaleCloseDefaultShortcutSuppressesMenuFallback(
staleAction: .closeWorkspace,
replacementAction: .newWindow,
replacementShortcut: StoredShortcut(key: "w", command: true, shift: true, option: false, control: false),
remappedStaleShortcut: StoredShortcut(key: "w", command: true, shift: true, option: true, control: false)
)

assertStaleCloseDefaultShortcutSuppressesMenuFallback(
staleAction: .closeWindow,
replacementAction: .toggleFullScreen,
replacementShortcut: StoredShortcut(key: "w", command: true, shift: false, option: false, control: true),
remappedStaleShortcut: StoredShortcut(key: "w", command: true, shift: false, option: true, control: true)
)
}

func testApplicationSendEventRoutesReassignedCmdWBeforeStaleCloseTabMenuEquivalent() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}

AppDelegate.installWindowResponderSwizzlesForTesting()

let windowId = appDelegate.createMainWindow()
guard let window = appDelegate.windowForMainWindowId(windowId),
let manager = appDelegate.tabManagerFor(windowId: windowId),
let initialSidebarVisible = appDelegate.sidebarVisibility(windowId: windowId) else {
closeWindow(withId: windowId)
XCTFail("Expected a main window context")
return
}

let previousMainMenu = NSApp.mainMenu
let menuProbe = MenuActionProbe()

defer {
NSApp.mainMenu = previousMainMenu
closeWindow(withId: windowId)
}

let staleMenu = NSMenu(title: "Test")
let staleCloseItem = NSMenuItem(
title: "Close Tab",
action: #selector(MenuActionProbe.perform(_:)),
keyEquivalent: "w"
)
staleCloseItem.keyEquivalentModifierMask = [.command]
staleCloseItem.target = menuProbe
staleMenu.addItem(staleCloseItem)
NSApp.mainMenu = staleMenu

window.makeKeyAndOrderFront(nil)
window.displayIfNeeded()

guard let event = makeKeyDownEvent(
key: "w",
modifiers: [.command],
keyCode: 13,
windowNumber: window.windowNumber
) else {
XCTFail("Failed to construct Cmd+W event")
return
}

let initialWorkspaceCount = manager.tabs.count
let remappedCloseTab = StoredShortcut(key: "w", command: true, shift: false, option: true, control: false)
let reassignedSidebarToggle = StoredShortcut(key: "w", command: true, shift: false, option: false, control: false)

withTemporaryShortcut(action: .closeTab, shortcut: remappedCloseTab) {
withTemporaryShortcut(action: .toggleSidebar, shortcut: reassignedSidebarToggle) {
NSApp.sendEvent(event)
}
}

RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))

XCTAssertEqual(menuProbe.callCount, 0, "A stale Cmd+W Close Tab menu item must not run after Cmd+W is reassigned")
XCTAssertEqual(
manager.tabs.count,
initialWorkspaceCount,
"Plain Cmd+W must not close a tab after Close Tab is remapped away"
)
XCTAssertEqual(
appDelegate.sidebarVisibility(windowId: windowId),
!initialSidebarVisible,
"The action currently assigned to Cmd+W should run before stale Close Tab menu fallback"
)
}

func testApplicationSendEventSuppressesRemappedCmdDStaleMenuShortcut() {
let previousMainMenu = NSApp.mainMenu
let probeWindow = NSWindow(
Expand Down Expand Up @@ -6209,6 +6347,35 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
body()
}

private func assertStaleCloseDefaultShortcutSuppressesMenuFallback(
staleAction: KeyboardShortcutSettings.Action,
replacementAction: KeyboardShortcutSettings.Action,
replacementShortcut: StoredShortcut,
remappedStaleShortcut: StoredShortcut,
file: StaticString = #filePath,
line: UInt = #line
) {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared", file: file, line: line)
return
}
guard let event = makeKeyDownEvent(shortcut: replacementShortcut, windowNumber: 0) else {
XCTFail("Failed to construct reassigned close-default shortcut event", file: file, line: line)
return
}

withTemporaryShortcut(action: staleAction, shortcut: remappedStaleShortcut) {
withTemporaryShortcut(action: replacementAction, shortcut: replacementShortcut) {
XCTAssertTrue(
appDelegate.shouldSuppressStaleCmuxMenuShortcut(event: event),
"\(staleAction.rawValue) should suppress its stale default menu fallback after that key is reassigned",
file: file,
line: line
)
}
}
}

private func assertEscapeKeyUpIsConsumedAfterCommandPaletteOpenRequest(
_ openRequest: (_ appDelegate: AppDelegate, _ window: NSWindow) -> Void,
file: StaticString = #filePath,
Expand Down
Loading