From 5001e2c60c5aa400721f7e43c31bf7eeea1dfdec Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Nov 2023 13:26:37 -0800 Subject: [PATCH] macos: filter option in AppKit when option-as-alt set Fixes #872 In #867 we fixed macos-option-as-alt, but unfortunately AppKit ALSO does some translation so some behaviors were not working correctly. Specifically, when you had macos-option-as-alt set, option+e would properly send `esc+e` to the pty but it would ALSO set the dead key state for "`" since AppKit was still translating the option key. This commit introduces a function to strip alt when necessary from the translation modifiers used at the AppKit layer, preventing this behavior. --- include/ghostty.h | 1 + macos/Sources/Ghostty/SurfaceView.swift | 41 +++++++++++++- src/Surface.zig | 2 + src/apprt/embedded.zig | 18 +++++++ src/input/key.zig | 72 +++++++++++++++++++++++++ 5 files changed, 133 insertions(+), 1 deletion(-) diff --git a/include/ghostty.h b/include/ghostty.h index 59ceef503..0546115a2 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -428,6 +428,7 @@ void ghostty_surface_refresh(ghostty_surface_t); void ghostty_surface_set_content_scale(ghostty_surface_t, double, double); void ghostty_surface_set_focus(ghostty_surface_t, bool); void ghostty_surface_set_size(ghostty_surface_t, uint32_t, uint32_t); +ghostty_input_mods_e ghostty_surface_key_translation_mods(ghostty_surface_t, ghostty_input_mods_e); void ghostty_surface_key(ghostty_surface_t, ghostty_input_key_s); void ghostty_surface_text(ghostty_surface_t, const char *, uintptr_t); void ghostty_surface_mouse_button(ghostty_surface_t, ghostty_input_mouse_state_e, ghostty_input_mouse_button_e, ghostty_input_mods_e); diff --git a/macos/Sources/Ghostty/SurfaceView.swift b/macos/Sources/Ghostty/SurfaceView.swift index 16ff2bc99..d8c69d882 100644 --- a/macos/Sources/Ghostty/SurfaceView.swift +++ b/macos/Sources/Ghostty/SurfaceView.swift @@ -725,12 +725,51 @@ extension Ghostty { } override func keyDown(with event: NSEvent) { + guard let surface = self.surface else { + self.interpretKeyEvents([event]) + return + } + + // We need to translate the mods (maybe) to handle configs such as option-as-alt + let translationModsGhostty = Ghostty.eventModifierFlags( + mods: ghostty_surface_key_translation_mods( + surface, + Ghostty.ghosttyMods(event.modifierFlags) + ) + ) + + // There are hidden bits set in our event that matter for certain dead keys + // so we can't use translationModsGhostty directly. Instead, we just check + // for exact states and set them. + var translationMods = event.modifierFlags + for flag in [NSEvent.ModifierFlags.shift, .control, .option, .command] { + if (translationModsGhostty.contains(flag)) { + translationMods.insert(flag) + } else { + translationMods.remove(flag) + } + } + + // Build a new NSEvent we use only for translation + let translationEvent = NSEvent.keyEvent( + with: event.type, + location: event.locationInWindow, + modifierFlags: translationMods, + timestamp: event.timestamp, + windowNumber: event.windowNumber, + context: nil, + characters: event.characters ?? "", + charactersIgnoringModifiers: event.charactersIgnoringModifiers ?? "", + isARepeat: event.isARepeat, + keyCode: event.keyCode + ) ?? event + // By setting this to non-nil, we note that we'rein a keyDown event. From here, // we call interpretKeyEvents so that we can handle complex input such as Korean // language. keyTextAccumulator = [] defer { keyTextAccumulator = nil } - self.interpretKeyEvents([event]) + self.interpretKeyEvents([translationEvent]) let action = event.isARepeat ? GHOSTTY_ACTION_REPEAT : GHOSTTY_ACTION_PRESS diff --git a/src/Surface.zig b/src/Surface.zig index 11ad9b810..16ceb93ae 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1026,6 +1026,8 @@ fn resize(self: *Surface, size: renderer.ScreenSize) !void { /// keyCallback and we rely completely on the apprt implementation to track /// the preedit state correctly. pub fn preeditCallback(self: *Surface, preedit_: ?u21) !void { + // log.debug("preedit cp={any}", .{preedit_}); + const preedit: ?renderer.State.Preedit = if (preedit_) |cp| preedit: { const width = ziglyph.display_width.codePointWidth(cp, .half); diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 2e417a92e..f505d6b4c 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -1348,6 +1348,24 @@ pub const CAPI = struct { surface.focusCallback(focused); } + /// Filter the mods if necessary. This handles settings such as + /// `macos-option-as-alt`. The filtered mods should be used for + /// key translation but should NOT be sent back via the `_key` + /// function -- the original mods should be used for that. + export fn ghostty_surface_key_translation_mods( + surface: *Surface, + mods_raw: c_int, + ) c_int { + const mods: input.Mods = @bitCast(@as( + input.Mods.Backing, + @truncate(@as(c_uint, @bitCast(mods_raw))), + )); + const result = mods.translation( + surface.core_surface.config.macos_option_as_alt, + ); + return @intCast(@as(input.Mods.Backing, @bitCast(result))); + } + /// 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/input/key.zig b/src/input/key.zig index c4aa9c3b7..2d9a16dcb 100644 --- a/src/input/key.zig +++ b/src/input/key.zig @@ -1,6 +1,8 @@ const std = @import("std"); +const builtin = @import("builtin"); const Allocator = std.mem.Allocator; const cimgui = @import("cimgui"); +const config = @import("../config.zig"); /// A generic key input event. This is the information that is necessary /// regardless of apprt in order to generate the proper terminal @@ -121,6 +123,37 @@ pub const Mods = packed struct(Mods.Backing) { return copy; } + /// Return the mods to use for key translation. This handles settings + /// like macos-option-as-alt. The translation mods should be used for + /// translation but never sent back in for the key callback. + pub fn translation(self: Mods, option_as_alt: config.OptionAsAlt) Mods { + // We currently only process macos-option-as-alt so other + // platforms don't need to do anything. + if (comptime !builtin.target.isDarwin()) return self; + + // We care if only alt is set. + const alt_only: bool = alt_only: { + const alt_mods: Mods = .{ .alt = true }; + var compare = self; + compare.sides = .{}; + break :alt_only alt_mods.equal(compare); + }; + if (!alt_only) return self; + + // Alt has to be set only on the correct side + switch (option_as_alt) { + .false => return self, + .true => {}, + .left => if (self.sides.alt == .right) return self, + .right => if (self.sides.alt == .left) return self, + } + + // Unset alt + var result = self; + result.alt = false; + return result; + } + // For our own understanding test { const testing = std.testing; @@ -130,6 +163,45 @@ pub const Mods = packed struct(Mods.Backing) { @as(Backing, 0b0000_0001), ); } + + test "translation macos-option-as-alt" { + if (comptime !builtin.target.isDarwin()) return error.SkipZigTest; + + const testing = std.testing; + + // Unset + { + const mods: Mods = .{}; + const result = mods.translation(.true); + try testing.expectEqual(result, mods); + } + + // Set + { + const mods: Mods = .{ .alt = true }; + const result = mods.translation(.true); + try testing.expectEqual(Mods{}, result); + } + + // Set but disabled + { + const mods: Mods = .{ .alt = true }; + const result = mods.translation(.false); + try testing.expectEqual(result, mods); + } + + // Set wrong side + { + const mods: Mods = .{ .alt = true, .sides = .{ .alt = .right } }; + const result = mods.translation(.left); + try testing.expectEqual(result, mods); + } + { + const mods: Mods = .{ .alt = true, .sides = .{ .alt = .left } }; + const result = mods.translation(.right); + try testing.expectEqual(result, mods); + } + } }; /// The action associated with an input event. This is backed by a c_int