From b3cb38c3fa65c2b4ff631bc9b73a5eeba3c01348 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 17 Apr 2025 10:15:47 -0700 Subject: [PATCH 1/7] macOS/libghostty: rework keyboard input handling This is a large refactor of the keyboard input handling code in libghostty and macOS. Previously, libghostty did a lot of things that felt out of scope or was repeated work due to lacking context. For example, libghostty would do full key translation from key event to character (including unshifted translation) as well as managing dead key states and setting the proper preedit text. This is all information the apprt can and should have on its own. NSEvent on macOS already provides us with all of this information, there's no need to redo the work. The reason we did in the first place is mostly historical: libghostty powered our initial macOS port years ago when we didn't have an AppKit runtime yet. This cruft has already practically been the source of numerous issues, e.g. #5558, but many other hacks along the way, too. This commit pushes all preedit (e.g. dead key) handling and key translation including unshifted keys up into the caller of libghostty. Besides code cleanup, a practical benefit of this is that key event handling on macOS is now about 10x faster on average. That's because we're avoiding repeated key translations as well as other unnecessary work. This should have a meaningful impact on input latency but I didn't measure the full end-to-end latency. A scarier part of this commit is that key handling is not well tested since its a GUI component. I suspect we'll have some fallout for certain keyboard layouts or input methods, but I did my best to run through everything I could think of. --- include/ghostty.h | 3 + macos/Sources/Ghostty/NSEvent+Extension.swift | 47 ++- .../Sources/Ghostty/SurfaceView_AppKit.swift | 84 ++-- src/apprt/embedded.zig | 367 +++++------------- 4 files changed, 176 insertions(+), 325 deletions(-) diff --git a/include/ghostty.h b/include/ghostty.h index f30275b2c..c4ef11930 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -254,8 +254,10 @@ typedef enum { typedef struct { ghostty_input_action_e action; ghostty_input_mods_e mods; + ghostty_input_mods_e consumed_mods; uint32_t keycode; const char* text; + uint32_t unshifted_codepoint; bool composing; } ghostty_input_key_s; @@ -725,6 +727,7 @@ ghostty_input_mods_e ghostty_surface_key_translation_mods(ghostty_surface_t, bool ghostty_surface_key(ghostty_surface_t, ghostty_input_key_s); bool ghostty_surface_key_is_binding(ghostty_surface_t, ghostty_input_key_s); void ghostty_surface_text(ghostty_surface_t, const char*, uintptr_t); +void ghostty_surface_preedit(ghostty_surface_t, const char*, uintptr_t); bool ghostty_surface_mouse_captured(ghostty_surface_t); bool ghostty_surface_mouse_button(ghostty_surface_t, ghostty_input_mouse_state_e, diff --git a/macos/Sources/Ghostty/NSEvent+Extension.swift b/macos/Sources/Ghostty/NSEvent+Extension.swift index 4118cd94d..5c13003b3 100644 --- a/macos/Sources/Ghostty/NSEvent+Extension.swift +++ b/macos/Sources/Ghostty/NSEvent+Extension.swift @@ -3,13 +3,56 @@ import GhosttyKit extension NSEvent { /// Create a Ghostty key event for a given keyboard action. + /// + /// This will not set the "text" or "composing" fields since these can't safely be set + /// with the information or lifetimes given. func ghosttyKeyEvent(_ action: ghostty_input_action_e) -> ghostty_input_key_s { - var key_ev = ghostty_input_key_s() + var key_ev: ghostty_input_key_s = .init() key_ev.action = action - key_ev.mods = Ghostty.ghosttyMods(modifierFlags) key_ev.keycode = UInt32(keyCode) + + // We can't infer or set these safely from this method. Since text is + // a cString, we can't use self.characters because of garbage collection. + // We have to let the caller handle this. key_ev.text = nil key_ev.composing = false + + // macOS provides no easy way to determine the consumed modifiers for + // producing text. We apply a simple heuristic here that has worked for years + // so far: control and command never contribute to the translation of text, + // assume everything else did. + key_ev.mods = Ghostty.ghosttyMods(modifierFlags) + key_ev.consumed_mods = Ghostty.ghosttyMods(modifierFlags.subtracting([.control, .command])) + + // Our unshifted codepoint is the codepoint with no modifiers. We + // ignore multi-codepoint values. + key_ev.unshifted_codepoint = 0 + if let charactersIgnoringModifiers, + let codepoint = charactersIgnoringModifiers.unicodeScalars.first + { + key_ev.unshifted_codepoint = codepoint.value + } + return key_ev } + + /// Returns the text to set for a key event for Ghostty. + /// + /// This namely contains logic to avoid control characters, since we handle control character + /// mapping manually within Ghostty. + var ghosttyCharacters: String? { + // If we have no characters associated with this event we do nothing. + guard let characters else { return nil } + + // 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. + if characters.count == 1, + let scalar = characters.unicodeScalars.first, + scalar.value < 0x20 { + return self.characters(byApplyingModifiers: modifierFlags.subtracting(.control)) + } + + return nil + } } diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index 1269f2314..9e5dc0559 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -951,29 +951,39 @@ extension Ghostty { return } - // If we have text, then we've composed a character, send that down. We do this - // first because if we completed a preedit, the text will be available here - // AND we'll have a preedit. - var handled: Bool = false - if let list = keyTextAccumulator, list.count > 0 { - handled = true - for text in list { - _ = keyAction(action, event: event, text: text) + // If we have marked text, we're in a preedit state. The order we + // do this and the key event callbacks below doesn't matter since + // we control the preedit state only through the preedit API. + if markedText.length > 0 { + let str = markedText.string + let len = str.utf8CString.count + if len > 0 { + markedText.string.withCString { ptr in + // Subtract 1 for the null terminator + ghostty_surface_preedit(surface, ptr, UInt(len - 1)) + } } + } else if markedTextBefore { + // If we had marked text before but don't now, we're no longer + // in a preedit state so we can clear it. + ghostty_surface_preedit(surface, nil, 0) } - // If we have marked text, we're in a preedit state. Send that down. - // If we don't have marked text but we had marked text before, then the preedit - // was cleared so we want to send down an empty string to ensure we've cleared - // the preedit. - if (markedText.length > 0 || markedTextBefore) { - handled = true - _ = keyAction(action, event: event, preedit: markedText.string) - } - - if (!handled) { - // No text or anything, we want to handle this manually. - _ = keyAction(action, event: event) + if let list = keyTextAccumulator, list.count > 0 { + // If we have text, then we've composed a character, send that down. + // These never have "composing" set to true because these are the + // result of a composition. + for text in list { + _ = keyAction(action, event: translationEvent, text: text) + } + } else { + // We have no accumulated text so this is a normal key event. + _ = keyAction( + action, + event: translationEvent, + text: translationEvent.ghosttyCharacters, + composing: markedText.length > 0 + ) } } @@ -1165,34 +1175,22 @@ extension Ghostty { _ = keyAction(action, event: event) } - private func keyAction(_ action: ghostty_input_action_e, event: NSEvent) -> Bool { - guard let surface = self.surface else { return false } - return ghostty_surface_key(surface, event.ghosttyKeyEvent(action)) - } - private func keyAction( _ action: ghostty_input_action_e, - event: NSEvent, preedit: String + event: NSEvent, + text: String? = nil, + composing: Bool = false ) -> Bool { guard let surface = self.surface else { return false } - return preedit.withCString { ptr in - var key_ev = event.ghosttyKeyEvent(action) - key_ev.text = ptr - key_ev.composing = true - return ghostty_surface_key(surface, key_ev) - } - } - - private func keyAction( - _ action: ghostty_input_action_e, - event: NSEvent, text: String - ) -> Bool { - guard let surface = self.surface else { return false } - - return text.withCString { ptr in - var key_ev = event.ghosttyKeyEvent(action) - key_ev.text = ptr + var key_ev = event.ghosttyKeyEvent(action) + key_ev.composing = composing + if let text { + return text.withCString { ptr in + key_ev.text = ptr + return ghostty_surface_key(surface, key_ev) + } + } else { return ghostty_surface_key(surface, key_ev) } } diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 50b54435d..e8da8612c 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -73,16 +73,68 @@ pub const App = struct { /// This is the key event sent for ghostty_surface_key and /// ghostty_app_key. pub const KeyEvent = struct { - /// The three below are absolutely required. action: input.Action, mods: input.Mods, + consumed_mods: input.Mods, keycode: u32, - - /// Optionally, the embedder can handle text translation and send - /// the text value here. If text is non-nil, it is assumed that the - /// embedder also handles dead key states and sets composing as necessary. text: ?[:0]const u8, + unshifted_codepoint: u32, composing: bool, + + /// Convert a libghostty key event into a core key event. + fn core(self: KeyEvent) ?input.KeyEvent { + const text: []const u8 = if (self.text) |v| v else ""; + const unshifted_codepoint: u21 = std.math.cast( + u21, + self.unshifted_codepoint, + ) orelse 0; + + // We want to get the physical unmapped key to process keybinds. + const physical_key = keycode: for (input.keycodes.entries) |entry| { + if (entry.native == self.keycode) break :keycode entry.key; + } else .invalid; + + // If the resulting text has length 1 then we can take its key + // and attempt to translate it to a key enum and call the key callback. + // If the length is greater than 1 then we're going to call the + // charCallback. + // + // We also only do key translation if this is not a dead key. + const key = if (!self.composing) key: { + // If our physical key is a keypad key, we use that. + if (physical_key.keypad()) break :key physical_key; + + // A completed key. If the length of the key is one then we can + // attempt to translate it to a key enum and call the key + // callback. First try plain ASCII. + if (text.len > 0) { + if (input.Key.fromASCII(text[0])) |key| { + break :key key; + } + } + + // If the above doesn't work, we use the unmodified value. + if (std.math.cast(u8, unshifted_codepoint)) |ascii| { + if (input.Key.fromASCII(ascii)) |key| { + break :key key; + } + } + + break :key physical_key; + } else .invalid; + + // Build our final key event + return .{ + .action = self.action, + .key = key, + .physical_key = physical_key, + .mods = self.mods, + .consumed_mods = self.consumed_mods, + .composing = self.composing, + .utf8 = text, + .unshifted_codepoint = unshifted_codepoint, + }; + } }; core_app: *CoreApp, @@ -92,10 +144,6 @@ pub const App = struct { /// The configuration for the app. This is owned by this structure. config: Config, - /// The keymap state is used for global keybinds only. Each surface - /// also has its own keymap state for focused keybinds. - keymap_state: input.Keymap.State, - pub fn init( core_app: *CoreApp, config: *const Config, @@ -114,7 +162,6 @@ pub const App = struct { .config = config_clone, .opts = opts, .keymap = keymap, - .keymap_state = .{}, }; } @@ -148,219 +195,6 @@ pub const App = struct { self.core_app.focusEvent(focused); } - /// Convert a C key event into a Zig key event. - /// - /// The buffer is needed for possibly storing translated UTF-8 text. - /// This buffer may (or may not) be referenced by the resulting KeyEvent - /// so it should be valid for the lifetime of the KeyEvent. - /// - /// The size of the buffer doesn't need to be large, we always - /// used to hardcode 128 bytes and never ran into issues. If it isn't - /// large enough an error will be returned. - fn coreKeyEvent( - self: *App, - buf: []u8, - target: KeyTarget, - event: KeyEvent, - ) !?input.KeyEvent { - const action = event.action; - const keycode = event.keycode; - const mods = event.mods; - - // True if this is a key down event - const is_down = action == .press or action == .repeat; - - // If we're on macOS and we have macos-option-as-alt enabled, - // then we strip the alt modifier from the mods for translation. - const translate_mods = translate_mods: { - var translate_mods = mods; - if ((comptime builtin.target.os.tag.isDarwin()) and translate_mods.alt) { - // Note: the keyboardLayout() function is not super cheap - // so we only want to run it if alt is already pressed hence - // the above condition. - const option_as_alt: configpkg.OptionAsAlt = - self.config.@"macos-option-as-alt" orelse - self.keyboardLayout().detectOptionAsAlt(); - - const strip = switch (option_as_alt) { - .false => false, - .true => mods.alt, - .left => mods.sides.alt == .left, - .right => mods.sides.alt == .right, - }; - if (strip) translate_mods.alt = false; - } - - // We strip super on macOS because its not used for translation - // it results in a bad translation. - if (comptime builtin.target.os.tag.isDarwin()) { - translate_mods.super = false; - } - - break :translate_mods translate_mods; - }; - - const event_text: ?[]const u8 = event_text: { - // This logic only applies to macOS. - if (comptime builtin.os.tag != .macos) break :event_text event.text; - - // If we're in a preedit state then we allow it through. This - // allows ctrl sequences that affect IME to work. For example, - // Ctrl+H deletes a character with Japanese input. - if (event.composing) break :event_text event.text; - - // If the modifiers are ONLY "control" then we never process - // the event text because we want to do our own translation so - // we can handle ctrl+c, ctrl+z, etc. - // - // This is specifically because on macOS using the - // "Dvorak - QWERTY ⌘" keyboard layout, ctrl+z is translated as - // "/" (the physical key that is z on a qwerty keyboard). But on - // other layouts, ctrl+ is not translated by AppKit. So, - // we just avoid this by never allowing AppKit to translate - // ctrl+ and instead do it ourselves. - const ctrl_only = comptime (input.Mods{ .ctrl = true }).int(); - break :event_text if (mods.binding().int() == ctrl_only) null else event.text; - }; - - // Translate our key using the keymap for our localized keyboard layout. - // We only translate for keydown events. Otherwise, we only care about - // the raw keycode. - const result: input.Keymap.Translation = if (is_down) translate: { - // If the event provided us with text, then we use this as a result - // and do not do manual translation. - const result: input.Keymap.Translation = if (event_text) |text| .{ - .text = text, - .composing = event.composing, - .mods = translate_mods, - } else try self.keymap.translate( - buf, - switch (target) { - .app => &self.keymap_state, - .surface => |surface| &surface.keymap_state, - }, - @intCast(keycode), - translate_mods, - ); - - // TODO(mitchellh): I think we can get rid of the above keymap - // translation code completely and defer to AppKit/Swift - // (for macOS) for handling all translations. The translation - // within libghostty is an artifact of an earlier design and - // it is buggy (see #5558). We should move closer to a GTK-style - // model of tracking composing states and preedit in the apprt - // and not in libghostty. - - // If this is a dead key, then we're composing a character and - // we need to set our proper preedit state if we're targeting a - // surface. - if (result.composing) { - switch (target) { - .app => {}, - .surface => |surface| surface.core_surface.preeditCallback( - result.text, - ) catch |err| { - log.err("error in preedit callback err={}", .{err}); - return null; - }, - } - } else { - switch (target) { - .app => {}, - .surface => |surface| surface.core_surface.preeditCallback(null) catch |err| { - log.err("error in preedit callback err={}", .{err}); - return null; - }, - } - - // If the text is just a single non-printable ASCII character - // then we clear the text. We handle non-printables in the - // key encoder manual (such as tab, ctrl+c, etc.) - if (result.text.len == 1 and result.text[0] < 0x20) { - break :translate .{}; - } - } - - break :translate result; - } else .{}; - - // We need to always do a translation with no modifiers at all in - // order to get the "unshifted_codepoint" for the key event. - const unshifted_codepoint: u21 = unshifted: { - var nomod_buf: [128]u8 = undefined; - var nomod_state: input.Keymap.State = .{}; - const nomod = try self.keymap.translate( - &nomod_buf, - &nomod_state, - @intCast(keycode), - .{}, - ); - - const view = std.unicode.Utf8View.init(nomod.text) catch |err| { - log.warn("cannot build utf8 view over text: {}", .{err}); - break :unshifted 0; - }; - var it = view.iterator(); - break :unshifted it.nextCodepoint() orelse 0; - }; - - // log.warn("TRANSLATE: action={} keycode={x} dead={} key_len={} key={any} key_str={s} mods={}", .{ - // action, - // keycode, - // result.composing, - // result.text.len, - // result.text, - // result.text, - // mods, - // }); - - // We want to get the physical unmapped key to process keybinds. - const physical_key = keycode: for (input.keycodes.entries) |entry| { - if (entry.native == keycode) break :keycode entry.key; - } else .invalid; - - // If the resulting text has length 1 then we can take its key - // and attempt to translate it to a key enum and call the key callback. - // If the length is greater than 1 then we're going to call the - // charCallback. - // - // We also only do key translation if this is not a dead key. - const key = if (!result.composing) key: { - // If our physical key is a keypad key, we use that. - if (physical_key.keypad()) break :key physical_key; - - // A completed key. If the length of the key is one then we can - // attempt to translate it to a key enum and call the key - // callback. First try plain ASCII. - if (result.text.len > 0) { - if (input.Key.fromASCII(result.text[0])) |key| { - break :key key; - } - } - - // If the above doesn't work, we use the unmodified value. - if (std.math.cast(u8, unshifted_codepoint)) |ascii| { - if (input.Key.fromASCII(ascii)) |key| { - break :key key; - } - } - - break :key physical_key; - } else .invalid; - - // Build our final key event - return .{ - .action = action, - .key = key, - .physical_key = physical_key, - .mods = mods, - .consumed_mods = result.mods, - .composing = result.composing, - .utf8 = result.text, - .unshifted_codepoint = unshifted_codepoint, - }; - } - /// See CoreApp.keyEvent. pub fn keyEvent( self: *App, @@ -368,12 +202,8 @@ pub const App = struct { event: KeyEvent, ) !bool { // Convert our C key event into a Zig one. - var buf: [128]u8 = undefined; - const input_event: input.KeyEvent = (try self.coreKeyEvent( - &buf, - target, - event, - )) orelse return false; + const input_event: input.KeyEvent = event.core() orelse + return false; // Invoke the core Ghostty logic to handle this input. const effect: CoreSurface.InputEffect = switch (target) { @@ -390,23 +220,7 @@ pub const App = struct { return switch (effect) { .closed => true, .ignored => false, - .consumed => consumed: { - const is_down = input_event.action == .press or - input_event.action == .repeat; - - if (is_down) { - // If we consume the key then we want to reset the dead - // key state. - self.keymap_state = .{}; - - switch (target) { - .app => {}, - .surface => |surface| surface.core_surface.preeditCallback(null) catch {}, - } - } - - break :consumed true; - }, + .consumed => true, }; } @@ -414,13 +228,6 @@ pub const App = struct { pub fn reloadKeymap(self: *App) !void { // Reload the keymap try self.keymap.reload(); - - // Clear the dead key state since we changed the keymap, any - // dead key state is just forgotten. i.e. if you type ' on us-intl - // and then switch to us and type a, you'll get a rather than á. - for (self.core_app.surfaces.items) |surface| { - surface.keymap_state = .{}; - } } /// Loads the keyboard layout. @@ -607,7 +414,6 @@ pub const Surface = struct { content_scale: apprt.ContentScale, size: apprt.SurfaceSize, cursor_pos: apprt.CursorPos, - keymap_state: input.Keymap.State, inspector: ?*Inspector = null, /// The current title of the surface. The embedded apprt saves this so @@ -656,7 +462,6 @@ pub const Surface = struct { }, .size = .{ .width = 800, .height = 600 }, .cursor_pos = .{ .x = -1, .y = -1 }, - .keymap_state = .{}, }; // Add ourselves to the list of surfaces on the app. @@ -992,6 +797,13 @@ pub const Surface = struct { }; } + pub fn preeditCallback(self: *Surface, preedit_: ?[]const u8) void { + _ = self.core_surface.preeditCallback(preedit_) catch |err| { + log.err("error in preedit callback err={}", .{err}); + return; + }; + } + pub fn textCallback(self: *Surface, text: []const u8) void { _ = self.core_surface.textCallback(text) catch |err| { log.err("error in key callback err={}", .{err}); @@ -1082,7 +894,6 @@ pub const Inspector = struct { surface: *Surface, ig_ctx: *cimgui.c.ImGuiContext, backend: ?Backend = null, - keymap_state: input.Keymap.State = .{}, content_scale: f64 = 1, /// Our previous instant used to calculate delta time for animations. @@ -1328,11 +1139,13 @@ pub const CAPI = struct { const KeyEvent = extern struct { action: input.Action, mods: c_int, + consumed_mods: c_int, keycode: u32, text: ?[*:0]const u8, + unshifted_codepoint: u32, composing: bool, - /// Convert to surface key event. + /// Convert to Zig key event. fn keyEvent(self: KeyEvent) App.KeyEvent { return .{ .action = self.action, @@ -1340,8 +1153,13 @@ pub const CAPI = struct { input.Mods.Backing, @truncate(@as(c_uint, @bitCast(self.mods))), )), + .consumed_mods = @bitCast(@as( + input.Mods.Backing, + @truncate(@as(c_uint, @bitCast(self.consumed_mods))), + )), .keycode = self.keycode, .text = if (self.text) |ptr| std.mem.sliceTo(ptr, 0) else null, + .unshifted_codepoint = self.unshifted_codepoint, .composing = self.composing, }; } @@ -1447,15 +1265,7 @@ pub const CAPI = struct { app: *App, event: KeyEvent, ) bool { - var buf: [128]u8 = undefined; - const core_event = app.coreKeyEvent( - &buf, - .app, - event.keyEvent(), - ) catch |err| { - log.warn("error processing key event err={}", .{err}); - return false; - } orelse { + const core_event = event.keyEvent().core() orelse { log.warn("error processing key event", .{}); return false; }; @@ -1701,20 +1511,7 @@ pub const CAPI = struct { surface: *Surface, event: KeyEvent, ) bool { - var buf: [128]u8 = undefined; - const core_event = surface.app.coreKeyEvent( - &buf, - // Note: this "app" target here looks like a bug, but it is - // intentional. coreKeyEvent uses the target only as a way to - // trigger preedit callbacks for keymap translation and we don't - // want to trigger that here. See the todo item in coreKeyEvent - // for a long term solution to this and removing target altogether. - .app, - event.keyEvent(), - ) catch |err| { - log.warn("error processing key event err={}", .{err}); - return false; - } orelse { + const core_event = event.keyEvent().core() orelse { log.warn("error processing key event", .{}); return false; }; @@ -1733,6 +1530,16 @@ pub const CAPI = struct { surface.textCallback(ptr[0..len]); } + /// Set the preedit text for the surface. This is used for IME + /// composition. If the length is 0, then the preedit text is cleared. + export fn ghostty_surface_preedit( + surface: *Surface, + ptr: [*]const u8, + len: usize, + ) void { + surface.preeditCallback(if (len == 0) null else ptr[0..len]); + } + /// Returns true if the surface currently has mouse capturing /// enabled. export fn ghostty_surface_mouse_captured(surface: *Surface) bool { From ded9be39c03df45e6a3b81f5f4d53935563ae737 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 17 Apr 2025 13:36:15 -0700 Subject: [PATCH 2/7] macOS: handle preedit text changes outside of key event --- .../Sources/Ghostty/SurfaceView_AppKit.swift | 48 +++++++++++++------ 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index 9e5dc0559..52314f534 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -954,20 +954,7 @@ extension Ghostty { // If we have marked text, we're in a preedit state. The order we // do this and the key event callbacks below doesn't matter since // we control the preedit state only through the preedit API. - if markedText.length > 0 { - let str = markedText.string - let len = str.utf8CString.count - if len > 0 { - markedText.string.withCString { ptr in - // Subtract 1 for the null terminator - ghostty_surface_preedit(surface, ptr, UInt(len - 1)) - } - } - } else if markedTextBefore { - // If we had marked text before but don't now, we're no longer - // in a preedit state so we can clear it. - ghostty_surface_preedit(surface, nil, 0) - } + syncPreedit(clearIfNeeded: markedTextBefore) if let list = keyTextAccumulator, list.count > 0 { // If we have text, then we've composed a character, send that down. @@ -1466,10 +1453,21 @@ extension Ghostty.SurfaceView: NSTextInputClient { default: print("unknown marked text: \(string)") } + + // If we're not in a keyDown event, then we want to update our preedit + // text immediately. This can happen due to external events, for example + // changing keyboard layouts while composing: (1) set US intl (2) type ' + // to enter dead key state (3) + if keyTextAccumulator == nil { + syncPreedit() + } } func unmarkText() { - self.markedText.mutableString.setString("") + if self.markedText.length > 0 { + self.markedText.mutableString.setString("") + syncPreedit() + } } func validAttributesForMarkedText() -> [NSAttributedString.Key] { @@ -1608,6 +1606,26 @@ extension Ghostty.SurfaceView: NSTextInputClient { print("SEL: \(selector)") } + + /// Sync the preedit state based on the markedText value to libghostty + private func syncPreedit(clearIfNeeded: Bool = true) { + guard let surface else { return } + + if markedText.length > 0 { + let str = markedText.string + let len = str.utf8CString.count + if len > 0 { + markedText.string.withCString { ptr in + // Subtract 1 for the null terminator + ghostty_surface_preedit(surface, ptr, UInt(len - 1)) + } + } + } else if clearIfNeeded { + // If we had marked text before but don't now, we're no longer + // in a preedit state so we can clear it. + ghostty_surface_preedit(surface, nil, 0) + } + } } // MARK: Services From 34ddd3d9e5a8f12191306a1207692d8fd38a9ca8 Mon Sep 17 00:00:00 2001 From: Bryan Lee <38807139+liby@users.noreply.github.com> Date: Fri, 18 Apr 2025 23:34:18 +0800 Subject: [PATCH 3/7] Refine Chinese translations for daily usage consistency --- po/zh_CN.UTF-8.po | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/po/zh_CN.UTF-8.po b/po/zh_CN.UTF-8.po index 795a93585..cdb4c3873 100644 --- a/po/zh_CN.UTF-8.po +++ b/po/zh_CN.UTF-8.po @@ -36,14 +36,14 @@ msgstr "确认" #: src/apprt/gtk/ui/1.5/config-errors-dialog.blp:5 msgid "Configuration Errors" -msgstr "设置错误" +msgstr "配置错误" #: src/apprt/gtk/ui/1.5/config-errors-dialog.blp:6 msgid "" "One or more configuration errors were found. Please review the errors below, " "and either reload your configuration or ignore these errors." msgstr "" -"加载设置时发现了以下错误。请仔细阅读错误信息,并选择忽略或重新加载设置文件。" +"加载配置时发现了以下错误。请仔细阅读错误信息,并选择忽略或重新加载配置文件。" #: src/apprt/gtk/ui/1.5/config-errors-dialog.blp:9 msgid "Ignore" @@ -53,7 +53,7 @@ msgstr "忽略" #: src/apprt/gtk/ui/1.0/menu-surface-context_menu.blp:97 #: src/apprt/gtk/ui/1.0/menu-window-titlebar_menu.blp:95 msgid "Reload Configuration" -msgstr "重新加载设置" +msgstr "重新加载配置" #: src/apprt/gtk/ui/1.0/menu-surface-context_menu.blp:6 #: src/apprt/gtk/ui/1.0/menu-window-titlebar_menu.blp:6 @@ -69,7 +69,7 @@ msgstr "粘贴" #: src/apprt/gtk/ui/1.0/menu-surface-context_menu.blp:18 #: src/apprt/gtk/ui/1.0/menu-window-titlebar_menu.blp:73 msgid "Clear" -msgstr "清除界面" +msgstr "清除屏幕" #: src/apprt/gtk/ui/1.0/menu-surface-context_menu.blp:23 #: src/apprt/gtk/ui/1.0/menu-window-titlebar_menu.blp:78 @@ -137,16 +137,16 @@ msgstr "关闭窗口" #: src/apprt/gtk/ui/1.0/menu-surface-context_menu.blp:89 msgid "Config" -msgstr "设置" +msgstr "配置" #: src/apprt/gtk/ui/1.0/menu-surface-context_menu.blp:92 #: src/apprt/gtk/ui/1.0/menu-window-titlebar_menu.blp:90 msgid "Open Configuration" -msgstr "打开设置文件" +msgstr "打开配置文件" #: src/apprt/gtk/ui/1.0/menu-window-titlebar_menu.blp:85 msgid "Terminal Inspector" -msgstr "终端检视器" +msgstr "终端调试器" #: src/apprt/gtk/ui/1.0/menu-window-titlebar_menu.blp:102 #: src/apprt/gtk/Window.zig:960 @@ -160,13 +160,13 @@ msgstr "退出" #: src/apprt/gtk/ui/1.5/ccw-osc-52-read.blp:6 #: src/apprt/gtk/ui/1.5/ccw-osc-52-write.blp:6 msgid "Authorize Clipboard Access" -msgstr "剪切板访问授权" +msgstr "剪贴板访问授权" #: src/apprt/gtk/ui/1.5/ccw-osc-52-read.blp:7 msgid "" "An application is attempting to read from the clipboard. The current " "clipboard contents are shown below." -msgstr "一个应用正在试图从剪切板读取内容。剪切板目前的内容如下:" +msgstr "一个应用正在试图从剪贴板读取内容。剪贴板目前的内容如下:" #: src/apprt/gtk/ui/1.5/ccw-osc-52-read.blp:10 #: src/apprt/gtk/ui/1.5/ccw-osc-52-write.blp:10 @@ -182,7 +182,7 @@ msgstr "允许" msgid "" "An application is attempting to write to the clipboard. The current " "clipboard contents are shown below." -msgstr "一个应用正在试图向剪切板写入内容。剪切板目前的内容如下:" +msgstr "一个应用正在试图向剪贴板写入内容。剪贴板目前的内容如下:" #: src/apprt/gtk/ui/1.5/ccw-paste.blp:6 msgid "Warning: Potentially Unsafe Paste" @@ -196,11 +196,11 @@ msgstr "将以下内容粘贴至终端内将可能执行有害命令。" #: src/apprt/gtk/inspector.zig:144 msgid "Ghostty: Terminal Inspector" -msgstr "Ghostty 终端检视器" +msgstr "Ghostty 终端调试器" #: src/apprt/gtk/Surface.zig:1243 msgid "Copied to clipboard" -msgstr "已复制至剪切板" +msgstr "已复制至剪贴板" #: src/apprt/gtk/CloseDialog.zig:47 msgid "Close" @@ -253,7 +253,7 @@ msgstr "⚠️ Ghostty 正在以调试模式运行!性能将大打折扣。" #: src/apprt/gtk/Window.zig:725 msgid "Reloaded the configuration" -msgstr "已重新加载设置" +msgstr "已重新加载配置" #: src/apprt/gtk/Window.zig:941 msgid "Ghostty Developers" From 793c727986c005a61103973e31a6f598448e69b5 Mon Sep 17 00:00:00 2001 From: Tristan Partin Date: Fri, 18 Apr 2025 12:41:59 -0500 Subject: [PATCH 4/7] apprt/gtk: refactor action callbacks to reduce code duplication It was getting very monotonous reading the code. Signed-off-by: Tristan Partin --- src/apprt/gtk/Window.zig | 86 ++++++++++------------------------------ 1 file changed, 20 insertions(+), 66 deletions(-) diff --git a/src/apprt/gtk/Window.zig b/src/apprt/gtk/Window.zig index 5fcb0d42b..129c149e7 100644 --- a/src/apprt/gtk/Window.zig +++ b/src/apprt/gtk/Window.zig @@ -811,16 +811,19 @@ fn gtkWindowUpdateScaleFactor( }; } -// Note: we MUST NOT use the GtkButton parameter because gtkActionNewTab -// sends an undefined value. -fn gtkTabNewClick(_: *gtk.Button, self: *Window) callconv(.c) void { +/// Perform a binding action on the window's action surface. +fn performBindingAction(self: *Window, action: input.Binding.Action) void { const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .new_tab = {} }) catch |err| { + _ = surface.performBindingAction(action) catch |err| { log.warn("error performing binding action error={}", .{err}); return; }; } +fn gtkTabNewClick(_: *gtk.Button, self: *Window) callconv(.c) void { + self.performBindingAction(.{ .new_tab = {} }); +} + /// Create a new tab from the AdwTabOverview. We can't copy gtkTabNewClick /// because we need to return an AdwTabPage from this function. fn gtkNewTabFromOverview(_: *adw.TabOverview, self: *Window) callconv(.c) *adw.TabPage { @@ -1007,11 +1010,7 @@ fn gtkActionNewWindow( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .new_window = {} }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .new_window = {} }); } fn gtkActionNewTab( @@ -1019,8 +1018,7 @@ fn gtkActionNewTab( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - // We can use undefined because the button is not used. - gtkTabNewClick(undefined, self); + self.performBindingAction(.{ .new_tab = {} }); } fn gtkActionCloseTab( @@ -1028,11 +1026,7 @@ fn gtkActionCloseTab( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .close_tab = {} }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .close_tab = {} }); } fn gtkActionSplitRight( @@ -1040,11 +1034,7 @@ fn gtkActionSplitRight( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .new_split = .right }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .new_split = .right }); } fn gtkActionSplitDown( @@ -1052,11 +1042,7 @@ fn gtkActionSplitDown( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .new_split = .down }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .new_split = .down }); } fn gtkActionSplitLeft( @@ -1064,11 +1050,7 @@ fn gtkActionSplitLeft( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .new_split = .left }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .new_split = .left }); } fn gtkActionSplitUp( @@ -1076,11 +1058,7 @@ fn gtkActionSplitUp( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .new_split = .up }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .new_split = .right }); } fn gtkActionToggleInspector( @@ -1088,11 +1066,7 @@ fn gtkActionToggleInspector( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .inspector = .toggle }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .inspector = .toggle }); } fn gtkActionCopy( @@ -1100,11 +1074,7 @@ fn gtkActionCopy( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .copy_to_clipboard = {} }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .copy_to_clipboard = {} }); } fn gtkActionPaste( @@ -1112,11 +1082,7 @@ fn gtkActionPaste( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .paste_from_clipboard = {} }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .paste_from_clipboard = {} }); } fn gtkActionReset( @@ -1124,11 +1090,7 @@ fn gtkActionReset( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .reset = {} }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .reset = {} }); } fn gtkActionClear( @@ -1136,11 +1098,7 @@ fn gtkActionClear( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .clear_screen = {} }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .clear_screen = {} }); } fn gtkActionPromptTitle( @@ -1148,11 +1106,7 @@ fn gtkActionPromptTitle( _: ?*glib.Variant, self: *Window, ) callconv(.C) void { - const surface = self.actionSurface() orelse return; - _ = surface.performBindingAction(.{ .prompt_surface_title = {} }) catch |err| { - log.warn("error performing binding action error={}", .{err}); - return; - }; + self.performBindingAction(.{ .prompt_surface_title = {} }); } /// Returns the surface to use for an action. From edb861634186936c677d48c79c95937929ad0d02 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 18 Apr 2025 14:25:34 -0700 Subject: [PATCH 5/7] macos: translationMods should be used for consumed mods calculation Fixes #7131 Regression from #7121 Our consumed mods should not include "alt" if `macos-option-as-alt` is set. To do this, we need to calculate our consumed mods based on the actual translation event mods (if available, only available during keyDown). --- macos/Sources/Ghostty/NSEvent+Extension.swift | 12 ++++++++++-- macos/Sources/Ghostty/SurfaceView_AppKit.swift | 13 ++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/macos/Sources/Ghostty/NSEvent+Extension.swift b/macos/Sources/Ghostty/NSEvent+Extension.swift index 5c13003b3..041a5a199 100644 --- a/macos/Sources/Ghostty/NSEvent+Extension.swift +++ b/macos/Sources/Ghostty/NSEvent+Extension.swift @@ -6,7 +6,13 @@ extension NSEvent { /// /// This will not set the "text" or "composing" fields since these can't safely be set /// with the information or lifetimes given. - func ghosttyKeyEvent(_ action: ghostty_input_action_e) -> ghostty_input_key_s { + /// + /// The translationMods should be set to the modifiers used for actual character + /// translation if available. + func ghosttyKeyEvent( + _ action: ghostty_input_action_e, + translationMods: NSEvent.ModifierFlags? = nil, + ) -> ghostty_input_key_s { var key_ev: ghostty_input_key_s = .init() key_ev.action = action key_ev.keycode = UInt32(keyCode) @@ -22,7 +28,9 @@ extension NSEvent { // so far: control and command never contribute to the translation of text, // assume everything else did. key_ev.mods = Ghostty.ghosttyMods(modifierFlags) - key_ev.consumed_mods = Ghostty.ghosttyMods(modifierFlags.subtracting([.control, .command])) + key_ev.consumed_mods = Ghostty.ghosttyMods( + (translationMods ?? modifierFlags) + .subtracting([.control, .command])) // Our unshifted codepoint is the codepoint with no modifiers. We // ignore multi-codepoint values. diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index 52314f534..e182e210f 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -961,13 +961,19 @@ extension Ghostty { // These never have "composing" set to true because these are the // result of a composition. for text in list { - _ = keyAction(action, event: translationEvent, text: text) + _ = keyAction( + action, + event: event, + translationEvent: translationEvent, + text: text + ) } } else { // We have no accumulated text so this is a normal key event. _ = keyAction( action, - event: translationEvent, + event: event, + translationEvent: translationEvent, text: translationEvent.ghosttyCharacters, composing: markedText.length > 0 ) @@ -1165,12 +1171,13 @@ extension Ghostty { private func keyAction( _ action: ghostty_input_action_e, event: NSEvent, + translationEvent: NSEvent? = nil, text: String? = nil, composing: Bool = false ) -> Bool { guard let surface = self.surface else { return false } - var key_ev = event.ghosttyKeyEvent(action) + var key_ev = event.ghosttyKeyEvent(action, translationMods: translationEvent?.modifierFlags) key_ev.composing = composing if let text { return text.withCString { ptr in From 18d6faf597c60dd646b2bb0278692e87d2054482 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 18 Apr 2025 15:07:27 -0700 Subject: [PATCH 6/7] macOS: translation mods should never have "control" This also lets us get rid of our `C-/` special handling to prevent a system beep. --- macos/Sources/Ghostty/NSEvent+Extension.swift | 2 +- .../Sources/Ghostty/SurfaceView_AppKit.swift | 10 ------ src/input/key.zig | 36 ++++++++++++------- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/macos/Sources/Ghostty/NSEvent+Extension.swift b/macos/Sources/Ghostty/NSEvent+Extension.swift index 041a5a199..d3c052f64 100644 --- a/macos/Sources/Ghostty/NSEvent+Extension.swift +++ b/macos/Sources/Ghostty/NSEvent+Extension.swift @@ -11,7 +11,7 @@ extension NSEvent { /// translation if available. func ghosttyKeyEvent( _ action: ghostty_input_action_e, - translationMods: NSEvent.ModifierFlags? = nil, + translationMods: NSEvent.ModifierFlags? = nil ) -> ghostty_input_key_s { var key_ev: ghostty_input_key_s = .init() key_ev.action = action diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index e182e210f..574c88044 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -1046,16 +1046,6 @@ extension Ghostty { let equivalent: String switch (event.charactersIgnoringModifiers) { - case "/": - // Treat C-/ as C-_. We do this because C-/ makes macOS make a beep - // sound and we don't like the beep sound. - if (!event.modifierFlags.contains(.control) || - !event.modifierFlags.isDisjoint(with: [.shift, .command, .option])) { - return false - } - - equivalent = "_" - case "\r": // Pass C- through verbatim // (prevent the default context menu equivalent) diff --git a/src/input/key.zig b/src/input/key.zig index f9db4a04a..ec65170f2 100644 --- a/src/input/key.zig +++ b/src/input/key.zig @@ -150,21 +150,25 @@ pub const Mods = packed struct(Mods.Backing) { /// 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.os.tag.isDarwin()) return self; + var result = 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, + // Control is never used for translation. + result.ctrl = false; + + // macos-option-as-alt for darwin + if (comptime builtin.target.os.tag.isDarwin()) alt: { + // Alt has to be set only on the correct side + switch (option_as_alt) { + .false => break :alt, + .true => {}, + .left => if (self.sides.alt == .right) break :alt, + .right => if (self.sides.alt == .left) break :alt, + } + + // Unset alt + result.alt = false; } - // Unset alt - var result = self; - result.alt = false; return result; } @@ -186,6 +190,14 @@ pub const Mods = packed struct(Mods.Backing) { ); } + test "translation removes control" { + const testing = std.testing; + + const mods: Mods = .{ .ctrl = true }; + const result = mods.translation(.true); + try testing.expectEqual(Mods{}, result); + } + test "translation macos-option-as-alt" { if (comptime !builtin.target.os.tag.isDarwin()) return error.SkipZigTest; From e4a37dd3836994a9ac7448ab17418b9224f1300b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 18 Apr 2025 15:14:14 -0700 Subject: [PATCH 7/7] macOS: only set unshifted codepoint on keyDown/Up events Other event types trigger an AppKit assertion that doesn't crash the app but logs some nasty stuff. --- macos/Sources/Ghostty/NSEvent+Extension.swift | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/macos/Sources/Ghostty/NSEvent+Extension.swift b/macos/Sources/Ghostty/NSEvent+Extension.swift index d3c052f64..754bb7a3a 100644 --- a/macos/Sources/Ghostty/NSEvent+Extension.swift +++ b/macos/Sources/Ghostty/NSEvent+Extension.swift @@ -35,10 +35,12 @@ extension NSEvent { // Our unshifted codepoint is the codepoint with no modifiers. We // ignore multi-codepoint values. key_ev.unshifted_codepoint = 0 - if let charactersIgnoringModifiers, - let codepoint = charactersIgnoringModifiers.unicodeScalars.first - { - key_ev.unshifted_codepoint = codepoint.value + if type == .keyDown || type == .keyUp { + if let charactersIgnoringModifiers, + let codepoint = charactersIgnoringModifiers.unicodeScalars.first + { + key_ev.unshifted_codepoint = codepoint.value + } } return key_ev