diff --git a/macos/Ghostty.xcodeproj/project.pbxproj b/macos/Ghostty.xcodeproj/project.pbxproj index 3758c325d..68d055dd5 100644 --- a/macos/Ghostty.xcodeproj/project.pbxproj +++ b/macos/Ghostty.xcodeproj/project.pbxproj @@ -210,6 +210,7 @@ Ghostty/Ghostty.Error.swift, Ghostty/Ghostty.Event.swift, Ghostty/Ghostty.Input.swift, + Ghostty/Ghostty.MenuShortcutManager.swift, Ghostty/Ghostty.Surface.swift, "Ghostty/NSEvent+Extension.swift", "Ghostty/Surface View/InspectorView.swift", diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index b02337e4b..645915ae1 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -154,12 +154,7 @@ class AppDelegate: NSObject, /// The custom app icon image that is currently in use. @Published private(set) var appIcon: NSImage? - /// Ghostty menu items indexed by their normalized shortcut. This avoids traversing - /// the entire menu tree on every key equivalent event. - /// - /// We store a weak reference so this cache can never be the owner of menu items. - /// If multiple items map to the same shortcut, the most recent one wins. - private var menuItemsByShortcut: [MenuShortcutKey: Weak] = [:] + @MainActor private lazy var menuShortcutManager = Ghostty.MenuShortcutManager() override init() { #if DEBUG @@ -784,7 +779,9 @@ class AppDelegate: NSObject, } // Config could change keybindings, so update everything that depends on that - syncMenuShortcuts(config) + DispatchQueue.main.async { + self.syncMenuShortcuts(config) + } TerminalController.all.forEach { $0.relabelTabs() } // Update our badge since config can change what we show. @@ -1144,11 +1141,10 @@ extension AppDelegate { } /// Sync all of our menu item keyboard shortcuts with the Ghostty configuration. - private func syncMenuShortcuts(_ config: Ghostty.Config) { + @MainActor private func syncMenuShortcuts(_ config: Ghostty.Config) { guard ghostty.readiness == .ready else { return } - // Reset our shortcut index since we're about to rebuild all menu bindings. - menuItemsByShortcut.removeAll(keepingCapacity: true) + menuShortcutManager.reset() syncMenuShortcut(config, action: "check_for_updates", menuItem: self.menuCheckForUpdates) syncMenuShortcut(config, action: "open_config", menuItem: self.menuOpenConfig) @@ -1215,97 +1211,12 @@ extension AppDelegate { reloadDockMenu() } - /// Syncs a single menu shortcut for the given action. The action string is the same - /// action string used for the Ghostty configuration. - private func syncMenuShortcut(_ config: Ghostty.Config, action: String, menuItem: NSMenuItem?) { - guard let menu = menuItem else { return } - - guard let shortcut = config.keyboardShortcut(for: action) else { - // No shortcut, clear the menu item - 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( - keyEquivalent: keyEquivalent, - modifiers: modifierMask - ) else { - return - } - - // Later registrations intentionally override earlier ones for the same key. - menuItemsByShortcut[key] = .init(menu) + @MainActor private func syncMenuShortcut(_ config: Ghostty.Config, action: String, menuItem: NSMenuItem?) { + menuShortcutManager.syncMenuShortcut(config, action: action, menuItem: menuItem) } - /// Attempts to perform a menu key equivalent only for menu items that represent - /// Ghostty keybind actions. This is important because it lets our surface dispatch - /// bindings through the menu so they flash but also lets our surface override macOS built-ins - /// like Cmd+H. - func performGhosttyBindingMenuKeyEquivalent(with event: NSEvent) -> Bool { - // Convert this event into the same normalized lookup key we use when - // syncing menu shortcuts from configuration. - guard let key = MenuShortcutKey(event: event) else { - return false - } - - // If we don't have an entry for this key combo, no Ghostty-owned - // menu shortcut exists for this event. - guard let weakItem = menuItemsByShortcut[key] else { - return false - } - - // Weak references can be nil if a menu item was deallocated after sync. - guard let item = weakItem.value else { - menuItemsByShortcut.removeValue(forKey: key) - return false - } - - guard let parentMenu = item.menu else { - return false - } - - // Keep enablement state fresh in case menu validation hasn't run yet. - parentMenu.update() - guard item.isEnabled else { - return false - } - - let index = parentMenu.index(of: item) - guard index >= 0 else { - return false - } - - parentMenu.performActionForItem(at: index) - return true - } - - /// Hashable key for a menu shortcut match, normalized for quick lookup. - private struct MenuShortcutKey: Hashable { - private static let shortcutModifiers: NSEvent.ModifierFlags = [.shift, .control, .option, .command] - - private let keyEquivalent: String - private let modifiersRawValue: UInt - - init?(keyEquivalent: String, modifiers: NSEvent.ModifierFlags) { - let normalized = keyEquivalent.lowercased() - guard !normalized.isEmpty else { return nil } - - self.keyEquivalent = normalized - self.modifiersRawValue = modifiers.intersection(Self.shortcutModifiers).rawValue - } - - init?(event: NSEvent) { - guard let keyEquivalent = event.charactersIgnoringModifiers else { return nil } - self.init(keyEquivalent: keyEquivalent, modifiers: event.modifierFlags) - } + @MainActor func performGhosttyBindingMenuKeyEquivalent(with event: NSEvent) -> Bool { + menuShortcutManager.performGhosttyBindingMenuKeyEquivalent(with: event) } } diff --git a/macos/Sources/Ghostty/Ghostty.Config.swift b/macos/Sources/Ghostty/Ghostty.Config.swift index 160894b18..743ebfa2f 100644 --- a/macos/Sources/Ghostty/Ghostty.Config.swift +++ b/macos/Sources/Ghostty/Ghostty.Config.swift @@ -45,6 +45,10 @@ extension Ghostty { self.init(config: ghostty_config_clone(config)) } + func clone(config: ghostty_config_t) { + self.config = config + } + deinit { self.config = nil } diff --git a/macos/Sources/Ghostty/Ghostty.MenuShortcutManager.swift b/macos/Sources/Ghostty/Ghostty.MenuShortcutManager.swift new file mode 100644 index 000000000..e97f71a62 --- /dev/null +++ b/macos/Sources/Ghostty/Ghostty.MenuShortcutManager.swift @@ -0,0 +1,124 @@ +import AppKit + +extension Ghostty { + /// The manager that's responsible for updating shortcuts of Ghostty's app menu + @MainActor + class MenuShortcutManager { + + /// Ghostty menu items indexed by their normalized shortcut. This avoids traversing + /// the entire menu tree on every key equivalent event. + /// + /// We store a weak reference so this cache can never be the owner of menu items. + /// If multiple items map to the same shortcut, the most recent one wins. + private var menuItemsByShortcut: [MenuShortcutKey: Weak] = [:] + + /// Reset our shortcut index since we're about to rebuild all menu bindings. + func reset() { + menuItemsByShortcut.removeAll(keepingCapacity: true) + } + + /// Syncs a single menu shortcut for the given action. The action string is the same + /// action string used for the Ghostty configuration. + 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 + 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 + /// Ghostty keybind actions. This is important because it lets our surface dispatch + /// bindings through the menu so they flash but also lets our surface override macOS built-ins + /// like Cmd+H. + func performGhosttyBindingMenuKeyEquivalent(with event: NSEvent) -> Bool { + // Convert this event into the same normalized lookup key we use when + // syncing menu shortcuts from configuration. + guard let key = MenuShortcutKey(event: event) else { + return false + } + + // If we don't have an entry for this key combo, no Ghostty-owned + // menu shortcut exists for this event. + guard let weakItem = menuItemsByShortcut[key] else { + return false + } + + // Weak references can be nil if a menu item was deallocated after sync. + guard let item = weakItem.value else { + menuItemsByShortcut.removeValue(forKey: key) + return false + } + + guard let parentMenu = item.menu else { + return false + } + + // Keep enablement state fresh in case menu validation hasn't run yet. + parentMenu.update() + guard item.isEnabled else { + return false + } + + let index = parentMenu.index(of: item) + guard index >= 0 else { + return false + } + + parentMenu.performActionForItem(at: index) + 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 + + init?(keyEquivalent: String, modifiers: NSEvent.ModifierFlags) { + let normalized = keyEquivalent.lowercased() + guard !normalized.isEmpty else { return nil } + var mods = modifiers.intersection(Self.shortcutModifiers) + if + keyEquivalent.lowercased() != keyEquivalent.uppercased(), + normalized.uppercased() == keyEquivalent { + // If key equivalent is case sensitive and + // it's originally uppercased, then we need to add `shift` to the modifiers + mods.insert(.shift) + } + self.keyEquivalent = normalized + self.modifiersRawValue = mods.rawValue + } + + init?(event: NSEvent) { + guard let keyEquivalent = event.charactersIgnoringModifiers else { return nil } + self.init(keyEquivalent: keyEquivalent, modifiers: event.modifierFlags) + } + } +} diff --git a/macos/Tests/Ghostty/ConfigTests.swift b/macos/Tests/Ghostty/ConfigTests.swift index b9c9d6a4a..fa2199537 100644 --- a/macos/Tests/Ghostty/ConfigTests.swift +++ b/macos/Tests/Ghostty/ConfigTests.swift @@ -1,6 +1,5 @@ import Testing @testable import Ghostty -@testable import GhosttyKit import SwiftUI @Suite @@ -182,6 +181,14 @@ struct ConfigTests { #expect(config.loaded == true) } + @Test func reloadConfig() throws { + let config = try TemporaryConfig("background-opacity = 0.5") + #expect(config.backgroundOpacity == 0.5) + + try config.reload("background-opacity = 0.7") + #expect(config.backgroundOpacity == 0.7) + } + @Test func defaultConfigIsLoaded() throws { let config = try TemporaryConfig("") #expect(config.optionalAutoUpdateChannel != nil) // release or tip @@ -214,31 +221,3 @@ struct ConfigTests { #expect(config.focusFollowsMouse == true) } } - -/// Create a temporary config file and delete it when this is deallocated -class TemporaryConfig: Ghostty.Config { - let temporaryFile: URL - - init(_ configText: String, finalize: Bool = true) throws { - let temporaryFile = FileManager.default.temporaryDirectory - .appendingPathComponent(UUID().uuidString) - .appendingPathExtension("ghostty") - try configText.write(to: temporaryFile, atomically: true, encoding: .utf8) - self.temporaryFile = temporaryFile - super.init(config: Self.loadConfig(at: temporaryFile.path(), finalize: finalize)) - } - - var optionalAutoUpdateChannel: Ghostty.AutoUpdateChannel? { - guard let config = self.config else { return nil } - var v: UnsafePointer? - let key = "auto-update-channel" - guard ghostty_config_get(config, &v, key, UInt(key.lengthOfBytes(using: .utf8))) else { return nil } - guard let ptr = v else { return nil } - let str = String(cString: ptr) - return Ghostty.AutoUpdateChannel(rawValue: str) - } - - deinit { - try? FileManager.default.removeItem(at: temporaryFile) - } -} diff --git a/macos/Tests/Ghostty/MenuShortcutManagerTests.swift b/macos/Tests/Ghostty/MenuShortcutManagerTests.swift new file mode 100644 index 000000000..ab8806b9b --- /dev/null +++ b/macos/Tests/Ghostty/MenuShortcutManagerTests.swift @@ -0,0 +1,50 @@ +import AppKit +import Foundation +import Testing +@testable import Ghostty + +struct MenuShortcutManagerTests { + @Test(.bug("https://github.com/ghostty-org/ghostty/issues/779", id: 779)) + func unbindShouldDiscardDefault() async throws { + let config = try TemporaryConfig("keybind = super+d=unbind") + + let item = NSMenuItem(title: "Split Right", action: #selector(BaseTerminalController.splitRight(_:)), keyEquivalent: "d") + item.keyEquivalentModifierMask = .command + let manager = await Ghostty.MenuShortcutManager() + await manager.reset() + await manager.syncMenuShortcut(config, action: "new_split:right", menuItem: item) + + #expect(item.keyEquivalent.isEmpty) + #expect(item.keyEquivalentModifierMask.isEmpty) + + try config.reload("") + + await manager.reset() + await manager.syncMenuShortcut(config, action: "new_split:right", menuItem: item) + + #expect(item.keyEquivalent == "d") + #expect(item.keyEquivalentModifierMask == .command) + } + + @Test(.bug("https://github.com/ghostty-org/ghostty/issues/11396", id: 11396)) + func overrideDefault() async throws { + let config = try TemporaryConfig("keybind=super+h=goto_split:left") + + let hideItem = NSMenuItem(title: "Hide Ghostty", action: "hide:", keyEquivalent: "h") + hideItem.keyEquivalentModifierMask = .command + + let goToLeftItem = NSMenuItem(title: "Select Split Left", action: "splitMoveFocusLeft:", keyEquivalent: "") + + let manager = await Ghostty.MenuShortcutManager() + await manager.reset() + + await manager.syncMenuShortcut(config, action: nil, menuItem: hideItem) + await manager.syncMenuShortcut(config, action: "goto_split:left", menuItem: goToLeftItem) + + #expect(hideItem.keyEquivalent.isEmpty) + #expect(hideItem.keyEquivalentModifierMask.isEmpty) + + #expect(goToLeftItem.keyEquivalent == "h") + #expect(goToLeftItem.keyEquivalentModifierMask == .command) + } +} diff --git a/macos/Tests/Ghostty/NormalizedMenuShortcutKeyTests.swift b/macos/Tests/Ghostty/NormalizedMenuShortcutKeyTests.swift new file mode 100644 index 000000000..5fb984a2f --- /dev/null +++ b/macos/Tests/Ghostty/NormalizedMenuShortcutKeyTests.swift @@ -0,0 +1,93 @@ +import AppKit +import Testing +@testable import Ghostty + +@Suite +struct NormalizedMenuShortcutKeyTests { + typealias Key = Ghostty.MenuShortcutManager.MenuShortcutKey + + // MARK: - Init from keyEquivalent + modifiers + + @Test func returnsNilForEmptyKeyEquivalent() { + let key = Key(keyEquivalent: "", modifiers: .command) + #expect(key == nil) + } + + @Test func lowercasesKeyEquivalent() { + let key = Key(keyEquivalent: "A", modifiers: .command) + #expect(key?.keyEquivalent == "a") + } + + @Test func stripsNonShortcutModifiers() { + // .capsLock and .function should be stripped + let key = Key(keyEquivalent: "c", modifiers: [.command, .capsLock, .function]) + let expected = Key(keyEquivalent: "c", modifiers: .command) + #expect(key == expected) + } + + @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) + } + + @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) + } + + @Test func lowercaseLetterDoesNotInsertShift() { + let key = Key(keyEquivalent: "a", modifiers: .command) + let expected = NSEvent.ModifierFlags.command.rawValue + #expect(key?.modifiersRawValue == 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) + } + + // MARK: - Equality / Hashing + + @Test func sameKeyAndModsAreEqual() { + let a = Key(keyEquivalent: "c", modifiers: .command) + let b = Key(keyEquivalent: "c", modifiers: .command) + #expect(a == b) + } + + @Test func uppercaseAndLowercaseWithShiftAreEqual() { + // "C" with .command should equal "c" with [.command, .shift] + // because the uppercase init auto-inserts .shift + let fromUpper = Key(keyEquivalent: "C", modifiers: .command) + let fromLowerWithShift = Key(keyEquivalent: "c", modifiers: [.command, .shift]) + #expect(fromUpper == fromLowerWithShift) + } + + @Test func differentKeysAreNotEqual() { + let a = Key(keyEquivalent: "a", modifiers: .command) + let b = Key(keyEquivalent: "b", modifiers: .command) + #expect(a != b) + } + + @Test func differentModifiersAreNotEqual() { + let a = Key(keyEquivalent: "c", modifiers: .command) + let b = Key(keyEquivalent: "c", modifiers: .option) + #expect(a != b) + } + + @Test func canBeUsedAsDictionaryKey() { + let key = Key(keyEquivalent: "c", modifiers: .command)! + var dict: [Key: String] = [:] + dict[key] = "copy" + #expect(dict[key] == "copy") + + // Same key created separately should find the same entry + let key2 = Key(keyEquivalent: "c", modifiers: .command)! + #expect(dict[key2] == "copy") + } +} diff --git a/macos/Tests/Helpers/TemporaryConfig.swift b/macos/Tests/Helpers/TemporaryConfig.swift new file mode 100644 index 000000000..f3e18dc53 --- /dev/null +++ b/macos/Tests/Helpers/TemporaryConfig.swift @@ -0,0 +1,45 @@ +import Foundation +@testable import Ghostty +@testable import GhosttyKit + +/// Create a temporary config file and delete it when this is deallocated +class TemporaryConfig: Ghostty.Config { + enum Error: Swift.Error { + case failedToLoad + } + + let temporaryFile: URL + + init(_ configText: String, finalize: Bool = true) throws { + let temporaryFile = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString) + .appendingPathExtension("ghostty") + try configText.write(to: temporaryFile, atomically: true, encoding: .utf8) + self.temporaryFile = temporaryFile + super.init(config: Self.loadConfig(at: temporaryFile.path(), finalize: finalize)) + } + + func reload(_ newConfigText: String?, finalize: Bool = true) throws { + if let newConfigText { + try newConfigText.write(to: temporaryFile, atomically: true, encoding: .utf8) + } + guard let cfg = Self.loadConfig(at: temporaryFile.path(), finalize: finalize) else { + throw Error.failedToLoad + } + clone(config: cfg) + } + + var optionalAutoUpdateChannel: Ghostty.AutoUpdateChannel? { + guard let config = self.config else { return nil } + var v: UnsafePointer? + let key = "auto-update-channel" + guard ghostty_config_get(config, &v, key, UInt(key.lengthOfBytes(using: .utf8))) else { return nil } + guard let ptr = v else { return nil } + let str = String(cString: ptr) + return Ghostty.AutoUpdateChannel(rawValue: str) + } + + deinit { + try? FileManager.default.removeItem(at: temporaryFile) + } +}