From 428038fdffc713acd98a4873938bad32a884e7c3 Mon Sep 17 00:00:00 2001 From: qappell <164906821+qappell@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:55:32 -0500 Subject: [PATCH] input: modifier-aware keybind resolution and macOS fixes --- include/ghostty.h | 1 + .../Ghostty/Ghostty.MenuShortcutManager.swift | 71 ++++++++------ macos/Sources/Ghostty/NSEvent+Extension.swift | 32 ++++--- .../Surface View/SurfaceView_AppKit.swift | 93 ++++++++++++++++++- src/apprt/embedded.zig | 45 ++++++++- src/build/uucode_config.zig | 2 +- src/config/Config.zig | 51 +++++----- src/input/Binding.zig | 57 +++++------- 8 files changed, 248 insertions(+), 104 deletions(-) diff --git a/include/ghostty.h b/include/ghostty.h index 72bbb57a80b..710666a8ad2 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -1123,6 +1123,7 @@ GHOSTTY_API void ghostty_surface_set_color_scheme(ghostty_surface_t, GHOSTTY_API ghostty_input_mods_e ghostty_surface_key_translation_mods(ghostty_surface_t, ghostty_input_mods_e); GHOSTTY_API bool ghostty_surface_key(ghostty_surface_t, ghostty_input_key_s); +GHOSTTY_API void ghostty_surface_record_inspector_key(ghostty_surface_t, ghostty_input_key_s); GHOSTTY_API bool ghostty_surface_key_is_binding(ghostty_surface_t, ghostty_input_key_s, ghostty_binding_flags_e*); diff --git a/macos/Sources/Ghostty/Ghostty.MenuShortcutManager.swift b/macos/Sources/Ghostty/Ghostty.MenuShortcutManager.swift index d7145745f21..764b9c32686 100644 --- a/macos/Sources/Ghostty/Ghostty.MenuShortcutManager.swift +++ b/macos/Sources/Ghostty/Ghostty.MenuShortcutManager.swift @@ -1,5 +1,6 @@ import AppKit import SwiftUI +import GhosttyKit extension Ghostty { /// The manager that's responsible for updating shortcuts of Ghostty's app menu @@ -34,41 +35,36 @@ extension Ghostty { /// 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 + // Look up and dispatch the menu item for a given shortcut key. + func dispatch(_ key: MenuShortcutKey) -> Bool { + guard let weakItem = menuItemsByShortcut[key] else { return false } + guard let item = weakItem.value else { + menuItemsByShortcut.removeValue(forKey: key) + return false + } + guard let parentMenu = item.menu else { return false } + 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 } - // 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 + // Match the unshifted character with full modifiers, + // matching how shortcuts are stored from configuration. + if let key = MenuShortcutKey(event: event), dispatch(key) { + return true } - // 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 + // Match the produced character with consumed modifiers stripped, + // for shortcuts where the character requires translation + // modifiers to type. + if let key = MenuShortcutKey(producedFromEvent: event), dispatch(key) { + return true } - 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 + return false } } } @@ -126,11 +122,28 @@ extension Ghostty.MenuShortcutManager { self.modifiersRawValue = mods.rawValue } + /// Create from an `NSEvent` using the unshifted character with full + /// modifiers, matching how shortcuts are stored from configuration. init?(event: NSEvent) { guard let keyEquivalent = event.charactersIgnoringModifiers else { return nil } self.init(keyEquivalent: keyEquivalent, modifiers: event.modifierFlags) } + /// Create from an `NSEvent` using the produced character with consumed + /// modifiers stripped, for shortcuts where the character requires + /// translation modifiers to type. + init?(producedFromEvent event: NSEvent) { + guard let keyEquivalent = event.ghosttyCharacters else { return nil } + + let consumed = Ghostty.Input.Mods( + cMods: event.ghosttyKeyEvent(GHOSTTY_ACTION_PRESS).consumed_mods + ).nsFlags + self.init( + keyEquivalent: keyEquivalent, + modifiers: event.modifierFlags.subtracting(consumed), + ) + } + /// Create from a `NSMenuItem` /// /// - Important: This will check whether the `keyEquivalent` is uppercased by `.shift` modifier. diff --git a/macos/Sources/Ghostty/NSEvent+Extension.swift b/macos/Sources/Ghostty/NSEvent+Extension.swift index 55888944ecb..62d4626d728 100644 --- a/macos/Sources/Ghostty/NSEvent+Extension.swift +++ b/macos/Sources/Ghostty/NSEvent+Extension.swift @@ -32,10 +32,10 @@ extension NSEvent { (translationMods ?? modifierFlags) .subtracting([.control, .command])) - // Our unshifted codepoint is the codepoint with no modifiers. We - // ignore multi-codepoint values. We have to use `byApplyingModifiers` - // instead of `charactersIgnoringModifiers` because the latter changes - // behavior with ctrl pressed and we don't want any of that. + // `charactersIgnoringModifiers` returns a control character when Ctrl is + // held (e.g. Ctrl+A yields U+0001, not "a"), so it does not give the true + // unmodified character. `characters(byApplyingModifiers: [])` does. Ignore + // multi-codepoint results. key_ev.unshifted_codepoint = 0 if type == .keyDown || type == .keyUp { if let chars = characters(byApplyingModifiers: []), @@ -47,25 +47,29 @@ extension NSEvent { return key_ev } - /// Returns the text to set for a key event for Ghostty. + /// Returns the text to send to Ghostty for this key event. /// - /// This namely contains logic to avoid control characters, since we handle control character - /// mapping manually within Ghostty. + /// macOS returns "" for `characters` while Command is held and produces + /// control characters while Control is held, so under either we re-derive the + /// produced character via `characters(byApplyingModifiers:)` with the + /// translation modifiers (Shift, Option, Caps). var ghosttyCharacters: String? { - // If we have no characters associated with this event we do nothing. + if modifierFlags.contains(.command) || modifierFlags.contains(.control) { + return self.characters(byApplyingModifiers: modifierFlags.intersection([.shift, .option, .capsLock])) + } + guard let characters else { return nil } if characters.count == 1, let scalar = characters.unicodeScalars.first { - // If we have a single control character, then we return the characters - // without control pressed. We do this because we handle control character - // encoding directly within Ghostty's KeyEncoder. + // C0 control characters come from keys like Enter and Tab, which + // Ghostty's KeyEncoder maps from the keycode rather than from text. if scalar.value < 0x20 { - return self.characters(byApplyingModifiers: modifierFlags.subtracting(.control)) + return self.characters(byApplyingModifiers: modifierFlags) } - // If we have a single value in the PUA, then it's a function key and - // we don't want to send PUA ranges down to Ghostty. + // Drop function keys that are encoded in the Private Use Area + // U+F700–U+F8FF so they don't reach Ghostty as text. if scalar.value >= 0xF700 && scalar.value <= 0xF8FF { return nil } diff --git a/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift index 7e76e35543e..a271bc57670 100644 --- a/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift @@ -1153,6 +1153,29 @@ extension Ghostty { // `interpretKeyEvents` may dispatch it. self.lastPerformKeyEvent = nil + // AppKit routes key events with Command or Control through + // performKeyEquivalent before keyDown, but we should also let the + // main menu handle Option-modified events (e.g. option+shift+m as a + // macOS App Shortcut) before they get swallowed by interpretKeyEvents, + // unless it is a binding. + if keySequence.isEmpty, + keyTables.isEmpty, + !event.modifierFlags.contains(.command), + !event.modifierFlags.contains(.control), + event.modifierFlags.contains(.option) { + var ghosttyEvent = event.ghosttyKeyEvent(GHOSTTY_ACTION_PRESS) + let isBinding = (event.ghosttyCharacters ?? "").withCString { ptr in + ghosttyEvent.text = ptr + return surfaceModel?.keyIsBinding(ghosttyEvent) + } + + if isBinding == nil, + let mainMenu = NSApp.mainMenu, + mainMenu.performKeyEquivalent(with: event) { + return + } + } + self.interpretKeyEvents([translationEvent]) // If our keyboard changed from this we just assume an input method @@ -1297,7 +1320,7 @@ extension Ghostty { // Get information about if this is a binding. let bindingFlags = surfaceModel.flatMap { surface in var ghosttyEvent = event.ghosttyKeyEvent(GHOSTTY_ACTION_PRESS) - return (event.characters ?? "").withCString { ptr in + return (event.ghosttyCharacters ?? "").withCString { ptr in ghosttyEvent.text = ptr return surface.keyIsBinding(ghosttyEvent) } @@ -1317,14 +1340,71 @@ extension Ghostty { bindingFlags.contains(.consumed) { if let appDelegate = NSApp.delegate as? AppDelegate, appDelegate.performGhosttyBindingMenuKeyEquivalent(with: event) { + + // keyDown is bypassed when the menu dispatches, so + // record the key for the inspector here. + if let surface = self.surface { + var ghosttyEvent = event.ghosttyKeyEvent(GHOSTTY_ACTION_PRESS) + (event.ghosttyCharacters ?? "").withCString { ptr in + ghosttyEvent.text = ptr + ghostty_surface_record_inspector_key(surface, ghosttyEvent) + } + } + return true } } + // A consumed binding should not forward text to the terminal, + // so skip interpretKeyEvents and dispatch directly to core. + // On layouts where that key is a dead key, feeding the event to + // the input client would start an unfinished composition. + if bindingFlags.contains(.consumed) { + if hasMarkedText() { + unmarkText() + } + let action = event.isARepeat ? GHOSTTY_ACTION_REPEAT : GHOSTTY_ACTION_PRESS + _ = keyAction(action, event: event, text: event.ghosttyCharacters) + return true + } + self.keyDown(with: event) return true } + // AppKit strips Shift from printable key equivalents, so Cmd+Shift+X + // would match a Cmd+X menu item once this method returns false and the + // main menu runs. If this event isn't a binding but Shift-removed would + // be, the Shift is meaningful, so then route it through keyDown and + // return true so that AppKit's menu never gets to fuzzy-match it. + if bindingFlags == nil, + event.modifierFlags.contains(.shift), + event.modifierFlags.contains(.command) || event.modifierFlags.contains(.control), + let surface = self.surface { + let dropped = event.modifierFlags.subtracting(.shift) + var probe = event.ghosttyKeyEvent(GHOSTTY_ACTION_PRESS) + probe.mods = Ghostty.ghosttyMods(dropped) + probe.consumed_mods = Ghostty.ghosttyMods( + dropped.subtracting([.control, .command]) + ) + var flags = ghostty_binding_flags_e(0) + let strippedText = event.characters( + byApplyingModifiers: dropped.intersection([.shift, .option, .capsLock]) + ) + let shiftedText = event.ghosttyCharacters + guard (strippedText ?? "").lowercased() == (shiftedText ?? "").lowercased() else { + return false + } + let strippedIsBinding = (strippedText ?? "").withCString { ptr in + probe.text = ptr + return ghostty_surface_key_is_binding(surface, probe, &flags) + } + if strippedIsBinding { + self.keyDown(with: event) + return true + } + } + let equivalent: String switch event.charactersIgnoringModifiers { case "\r": @@ -1461,10 +1541,19 @@ extension Ghostty { var key_ev = event.ghosttyKeyEvent(action, translationMods: translationEvent?.modifierFlags) key_ev.composing = composing + // Fall back to ghosttyCharacters when no explicit text was supplied, + // since `event.characters` is "" while Command is held. + let effectiveText: String? = { + if let text, !text.isEmpty { + return text + } + return event.ghosttyCharacters + }() + // For text, we only encode UTF8 if we don't have a single control // character. Control characters are encoded by Ghostty itself. // Without this, `ctrl+enter` does the wrong thing. - if let text, text.count > 0, + if let text = effectiveText, text.count > 0, let codepoint = text.utf8.first, codepoint >= 0x20 { return text.withCString { ptr in key_ev.text = ptr diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 7310159cc94..06cdd6be3b3 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -16,7 +16,8 @@ const internal_os = @import("../os/main.zig"); const renderer = @import("../renderer.zig"); const terminal = @import("../terminal/main.zig"); const CoreApp = @import("../App.zig"); -const CoreInspector = @import("../inspector/main.zig").Inspector; +const inspectorpkg = @import("../inspector/main.zig"); +const CoreInspector = inspectorpkg.Inspector; const CoreSurface = @import("../Surface.zig"); const configpkg = @import("../config.zig"); const Config = configpkg.Config; @@ -1775,6 +1776,48 @@ pub const CAPI = struct { return @intCast(@as(input.Mods.Backing, @bitCast(result))); } + /// Record a key event in the terminal inspector without executing any bindings. + /// The recorded event preserves original modifiers, while binding lookup mirrors + /// keyCallback by applying any configured key remaps first. + export fn ghostty_surface_record_inspector_key( + surface: *Surface, + event: KeyEvent, + ) void { + const core_event = event.keyEvent().core() orelse return; + const inspector = surface.core_surface.inspector orelse return; + + var insp_ev: inspectorpkg.KeyEvent = .{ .event = core_event }; + insp_ev.event.utf8 = ""; + if (core_event.utf8.len > 0) { + insp_ev.event.utf8 = surface.core_surface.alloc.dupe( + u8, + core_event.utf8, + ) catch &.{}; + } + + var lookup_event = core_event; + if (surface.core_surface.config.key_remaps.isRemapped(core_event.mods)) { + lookup_event.mods = surface.core_surface.config.key_remaps.apply(core_event.mods); + } + + if (surface.core_surface.config.keybind.set.getEvent(lookup_event)) |entry| { + const actions = switch (entry.value_ptr.*) { + .leader => &.{}, + inline .leaf, .leaf_chained => |leaf| leaf.generic().actionsSlice(), + }; + insp_ev.binding = surface.core_surface.alloc.dupe( + input.Binding.Action, + actions, + ) catch &.{}; + } + + if (inspector.recordKeyEvent(surface.core_surface.alloc, insp_ev)) { + surface.queueInspectorRender(); + } else |_| { + insp_ev.deinit(surface.core_surface.alloc); + } + } + /// Send this for raw keypresses (i.e. the keyDown event on macOS). /// This will handle the keymap translation and send the appropriate /// key and char events. diff --git a/src/build/uucode_config.zig b/src/build/uucode_config.zig index 2bb0d4508b0..c55a0d656d6 100644 --- a/src/build/uucode_config.zig +++ b/src/build/uucode_config.zig @@ -87,7 +87,7 @@ pub const tables = [_]config.Table{ .extensions = &.{}, .fields = &.{ d.field("is_emoji_presentation"), - d.field("case_folding_full"), + d.field("case_folding_simple"), }, }, .{ diff --git a/src/config/Config.zig b/src/config/Config.zig index fa491e49ccc..db138e5e6f8 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1539,30 +1539,33 @@ class: ?[:0]const u8 = null, /// overwrite previously set values. The list of actions is available in /// the documentation or using the `ghostty +list-actions` command. /// -/// Trigger: `+`-separated list of keys and modifiers. Example: `ctrl+a`, -/// `ctrl+shift+b`, `up`. -/// -/// If the key is a single Unicode codepoint, the trigger will match -/// any presses that produce that codepoint. These are impacted by -/// keyboard layouts. For example, `a` will match the `a` key on a -/// QWERTY keyboard, but will match the `q` key on a AZERTY keyboard -/// (assuming US physical layout). -/// -/// For Unicode codepoints, matching is done by comparing the set of -/// modifiers with the unmodified codepoint. The unmodified codepoint is -/// sometimes called an "unshifted character" in other software, but all -/// modifiers are considered, not only shift. For example, `ctrl+a` will match -/// `a` but not `ctrl+shift+a` (which is `A` on a US keyboard). -/// -/// Further, codepoint matching is case-insensitive and the unmodified -/// codepoint is always case folded for comparison. As a result, -/// `ctrl+A` configured will match when `ctrl+a` is pressed. Note that -/// this means some key combinations are impossible depending on keyboard -/// layout. For example, `ctrl+_` is impossible on a US keyboard because -/// `_` is `shift+-` and `ctrl+shift+-` is not equal to `ctrl+_` (because -/// the modifiers don't match!). More details on impossible key combinations -/// can be found at this excellent source written by Qt developers: -/// https://doc.qt.io/qt-6/qkeysequence.html#keyboard-layout-issues +/// Trigger: zero or more modifiers and one character or key, separated by `+`. +/// Examples: `ctrl+a`, `ctrl+shift+b`, `ctrl++`, `up`, `ctrl+page_down` +/// +/// The character in a trigger is matched case-insensitively using simple case +/// folding, so configuring `ctrl+G` is identical to configuring `ctrl+g`. +/// +/// If a key event's raw modifiers, paired with either its physical key +/// (detailed further below) or its base character, match a configured trigger, +/// the associated action executes. +/// +/// To support different keyboard layouts, Ghostty also matches against the +/// Unicode character a keypress produces, after stripping the layout-required +/// modifiers that produced the character from the event. For example, `ctrl+\` +/// matches whether `\` is a single key on a US layout or typed using +/// `option+shift+:` on a French AZERTY layout. However, when the produced +/// character differs from the base character only in case (such as `a` versus +/// `A`), no modifiers are stripped, so pressing `ctrl+shift+a` keeps its shift +/// and does not match `ctrl+a`. +/// +/// Because translation modifiers are stripped for character-based triggers, a +/// binding that uses the produced character cannot require a modifier that is +/// also needed to type that character. On a US keyboard, `{` is typed as +/// `shift+[`, so a trigger `ctrl+shift+{` is impossible to match. `ctrl+{` and +/// `ctrl+shift+[` both work. +/// +/// System shortcuts may prevent custom keybinds from triggering in Ghostty, +/// such as `Cmd+Shift+3` taking a screenshot on macOS. /// /// Physical key codes can be specified by using any of the key codes /// as specified by the [W3C specification](https://www.w3.org/TR/uievents-code/). diff --git a/src/input/Binding.zig b/src/input/Binding.zig index d60f2933bd8..66592f7bde3 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -1952,23 +1952,11 @@ pub const Trigger = struct { std.hash.autoHash(hasher, self.mods.binding()); } - /// The codepoint we use for comparisons. Case folding can result - /// in more codepoints so we need to use a 3 element array. - fn foldedCodepoint(cp: u21) [3]u21 { - // ASCII fast path - if (uucode.ascii.isAlphabetic(cp)) { - return .{ uucode.ascii.toLower(cp), 0, 0 }; - } - - // Unicode slow path. Case folding can result in more codepoints. - // If more codepoints are produced then we return the codepoint - // as-is which isn't correct but until we have a failing test - // then I don't want to handle this. - var buffer: [1]u21 = undefined; - const slice = uucode.get(.case_folding_full, cp).with(&buffer, cp); - var array: [3]u21 = [_]u21{0} ** 3; - @memcpy(array[0..slice.len], slice); - return array; + /// Returns the codepoint used for comparisons. Simple case folding + /// always maps a single codepoint to a single codepoint (full case + /// folding can expand, e.g. ß to "ss"), so no buffer is needed. + fn foldedCodepoint(cp: u21) u21 { + return uucode.get(.case_folding_simple, cp); } /// Returns true if two triggers are equal. @@ -1992,11 +1980,7 @@ pub const Trigger = struct { if (self_tag != other_tag) return false; return switch (self.key) { .physical => |v| v == other.key.physical, - .unicode => |v| deepEqual( - [3]u21, - foldedCodepoint(v), - foldedCodepoint(other.key.unicode), - ), + .unicode => |v| foldedCodepoint(v) == foldedCodepoint(other.key.unicode), .catch_all => true, }; } @@ -2651,8 +2635,11 @@ pub const Set = struct { /// Get an entry for the given key event. This will attempt to find /// a binding using multiple parts of the event in the following order: /// - /// 1. Physical key (event.physical_key) - /// 2. Unshifted Unicode codepoint (event.unshifted_codepoint) + /// 1. Physical key (event.physical_key) with raw modifiers + /// 2. Unshifted Unicode codepoint (event.unshifted_codepoint) with raw modifiers + /// 3. Produced Unicode codepoint (event.utf8) with modifiers masked when its + /// simple folded value differs from the simple folded unshifted codepoint + /// 4. catch_all with remaining modifiers, then with no modifiers /// pub fn getEvent(self: *const Set, event: KeyEvent) ?Entry { var trigger: Trigger = .{ @@ -2661,6 +2648,12 @@ pub const Set = struct { }; if (self.get(trigger)) |v| return v; + // Match the raw modifier set against the unshifted codepoint. + if (event.unshifted_codepoint > 0) { + trigger.key = .{ .unicode = event.unshifted_codepoint }; + if (self.get(trigger)) |v| return v; + } + // If our UTF-8 text is exactly one codepoint, we try to match that. if (event.utf8.len > 0) unicode: { const view = std.unicode.Utf8View.init(event.utf8) catch break :unicode; @@ -2670,16 +2663,14 @@ pub const Set = struct { const cp = it.nextCodepoint() orelse break :unicode; if (it.nextCodepoint() != null) break :unicode; - trigger.key = .{ .unicode = cp }; - if (self.get(trigger)) |v| return v; - } + // Strip consumed modifiers via effectiveMods unless the produced + // character is only a case variant of the unshifted codepoint, + // in which case the raw modifiers are kept. + const case_change_only = event.unshifted_codepoint > 0 and + Trigger.foldedCodepoint(cp) == Trigger.foldedCodepoint(event.unshifted_codepoint); - // Finally fallback to the full unshifted codepoint if we have one. - // Question: should we be doing this if we have UTF-8 text? I - // suspect "no" but we don't currently have any failing scenarios - // to verify this. - if (event.unshifted_codepoint > 0) { - trigger.key = .{ .unicode = event.unshifted_codepoint }; + trigger.mods = if (case_change_only) event.mods.binding() else event.effectiveMods().binding(); + trigger.key = .{ .unicode = cp }; if (self.get(trigger)) |v| return v; }