From 5001e2c60c5aa400721f7e43c31bf7eeea1dfdec Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Nov 2023 13:26:37 -0800 Subject: [PATCH 1/3] 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 From b4d393fdcf6dc735cd48acc1ac31e8c43ce794d6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Nov 2023 13:50:00 -0800 Subject: [PATCH 2/3] input: process alt-prefix even if utf8 text doesn't exist --- src/input/KeyEncoder.zig | 102 ++++++++++++++++++++++++++------------- 1 file changed, 68 insertions(+), 34 deletions(-) diff --git a/src/input/KeyEncoder.zig b/src/input/KeyEncoder.zig index 39c39021d..9e1bc0bc4 100644 --- a/src/input/KeyEncoder.zig +++ b/src/input/KeyEncoder.zig @@ -223,9 +223,16 @@ fn legacy( return buf[0..1]; } - // If we have no UTF8 text then at this point there is nothing to do. + // If we have no UTF8 text then the only possibility is the + // alt-prefix handling of unshifted codepoints... so we process that. const utf8 = self.event.utf8; - if (utf8.len == 0) return ""; + if (utf8.len == 0) { + if (try self.legacyAltPrefix(binding_mods, all_mods)) |byte| { + return try std.fmt.bufPrint(buf, "\x1B{c}", .{byte}); + } + + return ""; + } // In modify other keys state 2, we send the CSI 27 sequence // for any char with a modifier. Ctrl sequences like Ctrl+a @@ -293,44 +300,55 @@ fn legacy( // If we have alt-pressed and alt-esc-prefix is enabled, then // we need to prefix the utf8 sequence with an esc. - if (binding_mods.alt and self.alt_esc_prefix) alt: { - const byte = byte: { - // On macOS, we only handle option like alt in certain circumstances. - // Otherwise, macOS does a unicode translation and we allow that to - // happen. - if (comptime builtin.target.isDarwin()) { - switch (self.macos_option_as_alt) { - .false => break :alt, - .left => if (all_mods.sides.alt == .right) break :alt, - .right => if (all_mods.sides.alt == .left) break :alt, - .true => {}, - } - - if (self.event.unshifted_codepoint > 0) { - if (std.math.cast(u8, self.event.unshifted_codepoint)) |byte| { - break :byte byte; - } - } - } - - // Otherwise, we require utf8 to already have the byte represented. - if (utf8.len == 1) { - if (std.math.cast(u8, utf8[0])) |byte| { - break :byte byte; - } - } - - // Else, we can't figure out the byte to alt-prefix so we - // exit this handling. - break :alt; - }; - + if (try self.legacyAltPrefix(binding_mods, all_mods)) |byte| { return try std.fmt.bufPrint(buf, "\x1B{c}", .{byte}); } return try copyToBuf(buf, utf8); } +fn legacyAltPrefix( + self: *const KeyEncoder, + binding_mods: key.Mods, + mods: key.Mods, +) !?u8 { + // This only takes effect with alt pressed + if (!binding_mods.alt or !self.alt_esc_prefix) return null; + + // On macOS, we only handle option like alt in certain + // circumstances. Otherwise, macOS does a unicode translation + // and we allow that to happen. + if (comptime builtin.target.isDarwin()) { + switch (self.macos_option_as_alt) { + .false => return null, + .left => if (mods.sides.alt == .right) return null, + .right => if (mods.sides.alt == .left) return null, + .true => {}, + } + + if (self.event.unshifted_codepoint > 0) { + if (std.math.cast( + u8, + self.event.unshifted_codepoint, + )) |byte| { + return byte; + } + } + } + + // Otherwise, we require utf8 to already have the byte represented. + const utf8 = self.event.utf8; + if (utf8.len == 1) { + if (std.math.cast(u8, utf8[0])) |byte| { + return byte; + } + } + + // Else, we can't figure out the byte to alt-prefix so we + // exit this handling. + return null; +} + /// A helper to memcpy a src value to a buffer and return the result. fn copyToBuf(buf: []u8, src: []const u8) ![]const u8 { if (src.len > buf.len) return error.OutOfMemory; @@ -1178,6 +1196,22 @@ test "legacy: alt+c" { try testing.expectEqualStrings("\x1Bc", actual); } +test "legacy: alt+e only unshifted" { + var buf: [128]u8 = undefined; + var enc: KeyEncoder = .{ + .event = .{ + .key = .e, + .unshifted_codepoint = 'e', + .mods = .{ .alt = true }, + }, + .alt_esc_prefix = true, + .macos_option_as_alt = .true, + }; + + const actual = try enc.legacy(&buf); + try testing.expectEqualStrings("\x1Be", actual); +} + test "legacy: alt+x macos" { if (comptime !builtin.target.isDarwin()) return error.SkipZigTest; From 63e106390f4a99ad903449f4630c8eb2e41dc1bb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Nov 2023 13:58:41 -0800 Subject: [PATCH 3/3] input: fix failing test on Linux --- src/input/KeyEncoder.zig | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/input/KeyEncoder.zig b/src/input/KeyEncoder.zig index 9e1bc0bc4..521a78b95 100644 --- a/src/input/KeyEncoder.zig +++ b/src/input/KeyEncoder.zig @@ -344,6 +344,16 @@ fn legacyAltPrefix( } } + // If UTF8 isn't set, we will allow unshifted codepoints through. + if (self.event.unshifted_codepoint > 0) { + if (std.math.cast( + u8, + self.event.unshifted_codepoint, + )) |byte| { + return byte; + } + } + // Else, we can't figure out the byte to alt-prefix so we // exit this handling. return null;