macOS: refactor MenuShortcutManager (#12271)

Closes #11995

Yet another small step to fix menu shortcut-related issues.

1. Create `MenuShortcutKey` from `NSMenuItem` and `KeyboardShortcut`.
2. Add `updateMenuShortcut` to update to Ghostty ones only.

Doesn't contain any actual changes to pass previous test cases.
This commit is contained in:
Mitchell Hashimoto
2026-04-14 07:05:10 -07:00
committed by GitHub
2 changed files with 67 additions and 30 deletions

View File

@@ -1,4 +1,5 @@
import AppKit
import SwiftUI
extension Ghostty {
/// The manager that's responsible for updating shortcuts of Ghostty's app menu
@@ -22,31 +23,10 @@ extension Ghostty {
func syncMenuShortcut(_ config: Ghostty.Config, action: String?, menuItem: NSMenuItem?) {
guard let menu = menuItem else { return }
guard let action, let shortcut = config.keyboardShortcut(for: action) else {
// No shortcut, clear the menu item
if !updateMenuShortcut(config, action: action, menuItem: menu) {
menu.keyEquivalent = ""
menu.keyEquivalentModifierMask = []
return
}
let keyEquivalent = shortcut.key.character.description
let modifierMask = NSEvent.ModifierFlags(swiftUIFlags: shortcut.modifiers)
menu.keyEquivalent = keyEquivalent
menu.keyEquivalentModifierMask = modifierMask
// Build a direct lookup for key-equivalent dispatch so we don't need to
// linearly walk the full menu hierarchy at event time.
guard let key = MenuShortcutKey(
// We don't want to check missing `shift` for Ghostty configured shortcuts,
// because we know it's there when it needs to be
keyEquivalent: keyEquivalent.lowercased(),
modifiers: modifierMask
) else {
return
}
// Later registrations intentionally override earlier ones for the same key.
menuItemsByShortcut[key] = .init(menu)
}
/// Attempts to perform a menu key equivalent only for menu items that represent
@@ -93,13 +73,43 @@ extension Ghostty {
}
}
private extension Ghostty.MenuShortcutManager {
/// Syncs a single menu shortcut for the given action. The action string is the same
/// action string used for the Ghostty configuration.
///
/// - Returns: Whether the menu item is updated and saved in ``menuItemsByShortcut``
func updateMenuShortcut(_ config: Ghostty.Config, action: String?, menuItem menu: NSMenuItem) -> Bool {
guard
let action,
let shortcut = config.keyboardShortcut(for: action),
// Build a direct lookup for key-equivalent dispatch so we don't need to
// linearly walk the full menu hierarchy at event time.
let key = MenuShortcutKey(shortcut)
else {
return false
}
menu.keyEquivalent = key.keyEquivalent
menu.keyEquivalentModifierMask = key.modifierFlags
// Later registrations intentionally override earlier ones for the same key.
menuItemsByShortcut[key] = .init(menu)
return true
}
}
extension Ghostty.MenuShortcutManager {
/// Hashable key for a menu shortcut match, normalized for quick lookup.
struct MenuShortcutKey: Hashable {
private static let shortcutModifiers: NSEvent.ModifierFlags = [.shift, .control, .option, .command]
let keyEquivalent: String
let modifiersRawValue: UInt
// Make it Hashable
private let modifiersRawValue: UInt
var modifierFlags: NSEvent.ModifierFlags {
NSEvent.ModifierFlags(rawValue: modifiersRawValue)
}
init?(keyEquivalent: String, modifiers: NSEvent.ModifierFlags) {
let normalized = keyEquivalent.lowercased()
@@ -120,5 +130,32 @@ extension Ghostty.MenuShortcutManager {
guard let keyEquivalent = event.charactersIgnoringModifiers else { return nil }
self.init(keyEquivalent: keyEquivalent, modifiers: event.modifierFlags)
}
/// Create from a `NSMenuItem`
///
/// - Important: This will check whether the `keyEquivalent` is uppercased by `.shift` modifier.
init?(_ menuItem: NSMenuItem) {
self.init(
keyEquivalent: menuItem.keyEquivalent,
modifiers: menuItem.keyEquivalentModifierMask,
)
}
/// Create from a swiftUI `KeyboardShortcut`
init?(_ shortcut: KeyboardShortcut) {
// Ghostty configured shortcuts are already normalized
// in `Ghostty.keyboardShortcut(for:)`, see also gh-#12039
let keyEquivalent = shortcut.key.character.description
let modifierMask = NSEvent.ModifierFlags(swiftUIFlags: shortcut.modifiers)
self.init(keyEquivalent: keyEquivalent, modifiers: modifierMask)
}
var swiftUIShortcut: KeyboardShortcut? {
guard let character = keyEquivalent.first else { return nil }
return KeyboardShortcut(
KeyEquivalent(character),
modifiers: .init(nsFlags: modifierFlags)
)
}
}
}

View File

@@ -28,28 +28,28 @@ struct NormalizedMenuShortcutKeyTests {
@Test func preservesShortcutModifiers() {
let key = Key(keyEquivalent: "c", modifiers: [.shift, .control, .option, .command])
let allMods: NSEvent.ModifierFlags = [.shift, .control, .option, .command]
#expect(key?.modifiersRawValue == allMods.rawValue)
#expect(key?.modifierFlags == allMods)
}
@Test func uppercaseLetterInsertsShift() {
// "A" is uppercase and case-sensitive, so .shift should be added
let key = Key(keyEquivalent: "A", modifiers: .command)
let expected = NSEvent.ModifierFlags([.command, .shift]).rawValue
#expect(key?.modifiersRawValue == expected)
let expected = NSEvent.ModifierFlags([.command, .shift])
#expect(key?.modifierFlags == expected)
}
@Test func lowercaseLetterDoesNotInsertShift() {
let key = Key(keyEquivalent: "a", modifiers: .command)
let expected = NSEvent.ModifierFlags.command.rawValue
#expect(key?.modifiersRawValue == expected)
let expected = NSEvent.ModifierFlags.command
#expect(key?.modifierFlags == expected)
}
@Test func nonCaseSensitiveCharacterDoesNotInsertShift() {
// "1" is not case-sensitive (uppercased == lowercased is false for digits,
// but "1".uppercased() == "1".lowercased() == "1" so isCaseSensitive is false)
let key = Key(keyEquivalent: "1", modifiers: .command)
let expected = NSEvent.ModifierFlags.command.rawValue
#expect(key?.modifiersRawValue == expected)
let expected = NSEvent.ModifierFlags.command
#expect(key?.modifierFlags == expected)
}
// MARK: - Equality / Hashing