From a1f622e1e19b64ab8eec1f37bee3f4385040caba Mon Sep 17 00:00:00 2001 From: Akinori Musha Date: Fri, 8 May 2026 02:20:22 +0900 Subject: [PATCH] macos: apply key-remap to menu key equivalents Refs #12293 Related: #3493 (duplicates: #4374, #5091), #7013 Supersedes: #12597 Apply `key-remap` before keybind lookup and when reporting keybind triggers for AppKit menu key equivalents. Previously, menu shortcuts were registered from the configured trigger without considering modifier remaps. This let AppKit consume the physical shortcut before Ghostty could match the remapped keybind. `Config.keyEventIsBinding` now remaps event modifiers before lookup, so remapped key equivalents match the same keybinds as normal key events. `CApi.config_trigger_` returns the physical trigger that maps back to the configured logical modifiers, and leaves triggers unset when no remapped physical shortcut can produce them. `RemapSet.unapply` performs the reverse lookup and is checked with `apply` so ambiguous or impossible remaps do not register incorrect menu shortcuts. Inspired by: @bo2themax (https://github.com/ghostty-org/ghostty/pull/12597#pullrequestreview-4235918975) AI usage: OpenAI Codex helped investigate, implement, test, and refine this change. I reviewed and tested the resulting code. --- src/config/CApi.zig | 32 +++++++++++++++++- src/config/Config.zig | 19 ++++++----- src/input/key_mods.zig | 76 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 10 deletions(-) diff --git a/src/config/CApi.zig b/src/config/CApi.zig index 7d3366c938b..131a9899bdb 100644 --- a/src/config/CApi.zig +++ b/src/config/CApi.zig @@ -117,7 +117,12 @@ fn config_trigger_( str: []const u8, ) !inputpkg.Binding.Trigger.C { const action = try inputpkg.Binding.Action.parse(str); - const trigger: inputpkg.Binding.Trigger = self.keybind.set.getTrigger(action) orelse .{}; + var trigger: inputpkg.Binding.Trigger = self.keybind.set.getTrigger(action) orelse .{}; + const logical_mods = trigger.mods.binding(); + trigger.mods = self.@"key-remap".unapply(trigger.mods); + if (!self.@"key-remap".apply(trigger.mods).binding().equal(logical_mods)) { + return .{}; + } return trigger.cval(); } @@ -278,3 +283,28 @@ test "ghostty_config_trigger: default keybind" { try testing.expectEqual(.unidentified, trigger.key.physical); } } + +test "ghostty_config_trigger: key-remap" { + if (comptime builtin.target.os.tag != .macos) return error.SkipZigTest; + + const testing = std.testing; + + var cfg = try Config.default(testing.allocator); + defer cfg.deinit(); + + try cfg.@"key-remap".parseCLI(testing.allocator, "command=alt"); + defer cfg.@"key-remap".deinit(testing.allocator); + cfg.@"key-remap".finalize(); + + try cfg.keybind.parseCLI(testing.allocator, "alt+n=new_tab"); + + const trigger = try config_trigger_(&cfg, "new_tab"); + try testing.expectEqual(.unicode, trigger.tag); + try testing.expectEqual(@as(u32, 'n'), trigger.key.unicode); + try testing.expect(trigger.mods.super); + try testing.expect(!trigger.mods.alt); + + const default_trigger = try config_trigger_(&cfg, "new_window"); + try testing.expectEqual(.physical, default_trigger.tag); + try testing.expectEqual(.unidentified, default_trigger.key.physical); +} diff --git a/src/config/Config.zig b/src/config/Config.zig index 5b1d73deb6c..22b0be4d392 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1903,14 +1903,10 @@ keybind: Keybinds = .{}, /// * Generic modifiers (e.g. `ctrl`) match both left and right physical keys. /// Use sided names (e.g. `left_ctrl`) to remap only one side. /// -/// There are other edge case scenarios that may not behave as expected -/// but are working as intended the way this feature is designed: -/// -/// * On macOS, bindings in the main menu will trigger before any remapping -/// is done. This is because macOS itself handles menu activation and -/// this happens before Ghostty receives the key event. To workaround -/// this, you should unbind the menu items and rebind them using your -/// desired modifier. +/// On macOS, Ghostty applies key-remap when configuring menu key equivalents +/// so that menu shortcuts use the physical modifiers that map to the +/// configured keybind. Shortcuts whose configured modifiers cannot be +/// produced after remapping are left unset in the menu. /// /// This configuration can be repeated to specify multiple remaps. @"key-remap": KeyRemapSet = .empty, @@ -6416,8 +6412,13 @@ pub const RepeatableFontVariation = struct { /// a key event should be sent to the terminal or not. pub fn keyEventIsBinding( self: *Config, - event: inputpkg.KeyEvent, + event_orig: inputpkg.KeyEvent, ) bool { + var event = event_orig; + if (self.@"key-remap".isRemapped(event_orig.mods)) { + event.mods = self.@"key-remap".apply(event_orig.mods); + } + switch (event.action) { .release => return false, .press, .repeat => {}, diff --git a/src/input/key_mods.zig b/src/input/key_mods.zig index 35e1c10383d..5643f449f69 100644 --- a/src/input/key_mods.zig +++ b/src/input/key_mods.zig @@ -472,6 +472,28 @@ pub const RemapSet = struct { unreachable; } + /// Apply the inverse of a remap to the given mods. + pub fn unapply(self: *const RemapSet, mods: Mods) Mods { + const mods_binding: Mods.Keys.Backing = @truncate(mods.int()); + const mods_sides: Mods.Side.Backing = @bitCast(mods.sides); + + var it = self.map.iterator(); + while (it.next()) |entry| { + const to = entry.value_ptr.*; + const to_binding: Mods.Keys.Backing = @truncate(to.int()); + if (mods_binding & to_binding != to_binding) continue; + const to_sides: Mods.Side.Backing = @bitCast(to.sides); + if ((mods_sides ^ to_sides) & to_binding != 0) continue; + + var mods_int = mods.int(); + mods_int &= ~to.int(); + mods_int |= entry.key_ptr.*.int(); + return @bitCast(mods_int); + } + + return mods; + } + /// Tracks which modifier keys and sides have remappings registered. /// Used as a fast pre-check before doing expensive map lookups. /// @@ -650,6 +672,57 @@ test "RemapSet: multiple parses accumulate" { try testing.expectEqual(left_ctrl_result, set.apply(left_alt)); } +test "RemapSet: unapply reverses a sided remap" { + const testing = std.testing; + const alloc = testing.allocator; + + var set: RemapSet = .empty; + defer set.deinit(alloc); + + try set.parse(alloc, "left_ctrl=left_super"); + set.finalize(); + + const left_ctrl: Mods = .{ .ctrl = true, .sides = .{ .ctrl = .left } }; + const left_super: Mods = .{ .super = true, .sides = .{ .super = .left } }; + try testing.expectEqual(left_ctrl, set.unapply(left_super)); + try testing.expectEqual(left_super, set.apply(set.unapply(left_super))); +} + +test "RemapSet: unapply returns input when nothing matches" { + const testing = std.testing; + const alloc = testing.allocator; + + var set: RemapSet = .empty; + defer set.deinit(alloc); + + try set.parse(alloc, "ctrl=super"); + set.finalize(); + + const left_alt: Mods = .{ .alt = true, .sides = .{ .alt = .left } }; + try testing.expectEqual(left_alt, set.unapply(left_alt)); +} + +test "RemapSet: unapply with multiple remaps targeting the same modifier" { + const testing = std.testing; + const alloc = testing.allocator; + + var set: RemapSet = .empty; + defer set.deinit(alloc); + + // Both ctrl and alt remap to super. unapply on super is ambiguous, + // so we only assert that the result round-trips back to super under + // apply (the property config_trigger_ relies on for the menu). + try set.parse(alloc, "ctrl=super"); + try set.parse(alloc, "alt=super"); + set.finalize(); + + const left_super: Mods = .{ .super = true, .sides = .{ .super = .left } }; + const reversed = set.unapply(left_super); + try testing.expect(reversed.ctrl or reversed.alt); + try testing.expect(!reversed.super); + try testing.expectEqual(left_super, set.apply(reversed)); +} + test "RemapSet: error on missing assignment" { const testing = std.testing; const alloc = testing.allocator; @@ -791,6 +864,9 @@ test "RemapSet: parse aliased modifiers command" { const left_super: Mods = .{ .super = true, .sides = .{ .super = .left } }; const left_alt: Mods = .{ .alt = true, .sides = .{ .alt = .left } }; try testing.expectEqual(left_alt, set.apply(left_super)); + const unmapped = set.unapply(left_alt); + try testing.expect(unmapped.super); + try testing.expect(!unmapped.alt); } test "RemapSet: parse aliased modifiers opt and option" {