From 416b617de90903da7204e46398fe1941b35604e9 Mon Sep 17 00:00:00 2001 From: Aaron Ruan Date: Tue, 25 Mar 2025 22:46:14 +0800 Subject: [PATCH 1/2] fix canonicalizing some zh locales (#6885) resolve #6870 --------- Signed-off-by: Aaron Ruan --- src/os/i18n.zig | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/os/i18n.zig b/src/os/i18n.zig index a21a26be8..baae73e46 100644 --- a/src/os/i18n.zig +++ b/src/os/i18n.zig @@ -107,6 +107,9 @@ pub fn canonicalizeLocale( buf: []u8, locale: []const u8, ) error{NoSpaceLeft}![:0]const u8 { + // Fix zh locales for macOS + if (fixZhLocale(locale)) |fixed| return fixed; + // Buffer must be 16 or at least as long as the locale and null term if (buf.len < @max(16, locale.len + 1)) return error.NoSpaceLeft; @@ -125,6 +128,30 @@ pub fn canonicalizeLocale( return buf[0..slice.len :0]; } +/// Handles some zh locales canonicalization because internal libintl +/// canonicalization function doesn't handle correctly in these cases. +fn fixZhLocale(locale: []const u8) ?[:0]const u8 { + var it = std.mem.splitScalar(u8, locale, '-'); + const name = it.next() orelse return null; + if (!std.mem.eql(u8, name, "zh")) return null; + + const script = it.next() orelse return null; + const region = it.next() orelse return null; + + if (std.mem.eql(u8, script, "Hans")) { + if (std.mem.eql(u8, region, "SG")) return "zh_SG"; + return "zh_CN"; + } + + if (std.mem.eql(u8, script, "Hant")) { + if (std.mem.eql(u8, region, "MO")) return "zh_MO"; + if (std.mem.eql(u8, region, "HK")) return "zh_HK"; + return "zh_TW"; + } + + return null; +} + /// This can be called at any point a compile-time-known locale is /// available. This will use comptime to verify the locale is supported. pub fn staticLocale(comptime v: [*:0]const u8) [*:0]const u8 { @@ -159,6 +186,12 @@ test "canonicalizeLocale darwin" { try testing.expectEqualStrings("zh_CN", try canonicalizeLocale(&buf, "zh-Hans")); try testing.expectEqualStrings("zh_TW", try canonicalizeLocale(&buf, "zh-Hant")); + try testing.expectEqualStrings("zh_CN", try canonicalizeLocale(&buf, "zh-Hans-CN")); + try testing.expectEqualStrings("zh_SG", try canonicalizeLocale(&buf, "zh-Hans-SG")); + try testing.expectEqualStrings("zh_TW", try canonicalizeLocale(&buf, "zh-Hant-TW")); + try testing.expectEqualStrings("zh_HK", try canonicalizeLocale(&buf, "zh-Hant-HK")); + try testing.expectEqualStrings("zh_MO", try canonicalizeLocale(&buf, "zh-Hant-MO")); + // This is just an edge case I want to make sure we're aware of: // canonicalizeLocale does not handle encodings and will turn them into // underscores. We should parse them out before calling this function. From 67f47a6e226cdfb52570ff0222246170f16ccdd5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 25 Mar 2025 14:53:14 -0700 Subject: [PATCH 2/2] macos: remove special-case cmd+period handling Fixes #5522 This commit re-dispatches command inputs that are unhandled by our macOS app so they can be encoded to the pty and handled by the core libghostty key callback system. We've had a special case `cmd+period` handling in Ghostty for a very long time (since well into the private beta). `cmd+period` by default binds to "cancel" in macOS, so it doesn't encode to the pty. We don't handle "cancel" in any meaningful way in Ghostty, so we special-cased it to encode properly to the pty. However, as shown in #5522, if the user rebinds `cmd+period` at the system level to some other operation, then this is ignored and we encode it still. This isn't desirable, we just want to work around not caring about "cancel." The callback path that AppKit takes for key events is a bit convoluted. For command keys, it first calls `performKeyEquivalent`. If this returns false (we want to continue standard processing), then it calls EITHER `keyDown` or `doCommand(by:)`. It calls the latter if there is a standard system command that matches the key event. For `cmd+period` by default, this is "cancel." Unfortunately, from `doCommand` we can't say "oops, we don't want to handle this, please continue processing." Its too late. So, this commit stores the last command key event from `performKeyEquivalent` and if we reach `doCommand` for it without having called `keyDown`, we re-dispatch the event and send it to keyDown. I'm honestly pretty sus about this whole logic but it is scoped to only command-keys and I couldn't trigger any adverse behavior in my testing. It also definitely fixed #5522 as far as I could reproduce it before. --- .../Sources/Ghostty/SurfaceView_AppKit.swift | 77 +++++++++++++++++-- 1 file changed, 70 insertions(+), 7 deletions(-) diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index e4c9072f3..60a573c37 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -922,6 +922,33 @@ extension Ghostty { _ = keyAction(GHOSTTY_ACTION_RELEASE, event: event) } + /// Records the timestamp of the last event to performKeyEquivalent that had a command key active. + /// + /// For command+key inputs, the AppKit input stack calls performKeyEquivalent to give us a chance + /// to handle them first. If we return "false" then it goes through the standard AppKit responder chain. + /// For an NSTextInputClient, that may redirect some commands _before_ our keyDown gets called. + /// Concretely: Command+Period will do: performKeyEquivalent, doCommand ("cancel:"). In doCommand, + /// we need to know that we actually want to handle that in keyDown, so we send it back through the + /// event dispatch system and use this timestamp as an identity to know to actually send it to keyDown. + /// + /// Why not send it to keyDown always? Because if the user rebinds a command to something we + /// actually handle then we do want the standard response chain to handle the key input. Unfortunately, + /// we can't know what a command is bound to at a system level until we let it flow through the system. + /// That's the crux of the problem. + /// + /// So, we have to send it back through if we didn't handle it. + /// + /// The next part of the problem is comparing NSEvent identity seems pretty nasty. I couldn't + /// find a good way to do it. I originally stored a weak ref and did identity comparison but that + /// doesn't work and for reasons I couldn't figure out the value gets mangled (fields don't match + /// before/after the assignment). I suspect it has something to do with the fact an NSEvent is wrapping + /// a lower level event pointer and its just not surviving the Swift runtime somehow. I don't know. + /// + /// The best thing I could find was to store the event timestamp which has decent granularity + /// and compare that. To further complicate things, some events are synthetic and have a zero + /// timestamp so we have to protect against that. Fun! + var lastCommandEvent: TimeInterval? + /// Special case handling for some control keys override func performKeyEquivalent(with event: NSEvent) -> Bool { switch (event.type) { @@ -975,15 +1002,41 @@ extension Ghostty { equivalent = "\r" - case ".": - if (!event.modifierFlags.contains(.command)) { + default: + // It looks like some part of AppKit sometimes generates synthetic NSEvents + // with a zero timestamp. We never process these at this point. Concretely, + // this happens for me when pressing Cmd+period with default bindings. This + // binds to "cancel" which goes through AppKit to produce a synthetic "escape". + // + // Question: should we be ignoring all synthetic events? Should we be finding + // synthetic escape and ignoring it? I feel like Cmd+period could map to a + // escape binding by accident, but it hasn't happened yet... + if event.timestamp == 0 { return false } - equivalent = "." + // All of this logic here re: lastCommandEvent is to workaround some + // nasty behavior. See the docs for lastCommandEvent for more info. - default: - // Ignore other events + // Ignore all other non-command events. This lets the event continue + // through the AppKit event systems. + if (!event.modifierFlags.contains(.command)) { + // Reset since we got a non-command event. + lastCommandEvent = nil + return false + } + + // If we have a prior command binding and the timestamp matches exactly + // then we pass it through to keyDown for encoding. + if let lastCommandEvent { + self.lastCommandEvent = nil + if lastCommandEvent == event.timestamp { + equivalent = event.characters ?? "" + break + } + } + + lastCommandEvent = event.timestamp return false } @@ -1480,9 +1533,19 @@ extension Ghostty.SurfaceView: NSTextInputClient { } } + /// This function needs to exist for two reasons: + /// 1. Prevents an audible NSBeep for unimplemented actions. + /// 2. Allows us to properly encode super+key input events that we don't handle override func doCommand(by selector: Selector) { - // This currently just prevents NSBeep from interpretKeyEvents but in the future - // we may want to make some of this work. + // If we are being processed by performKeyEquivalent with a command binding, + // we send it back through the event system so it can be encoded. + if let lastCommandEvent, + let current = NSApp.currentEvent, + lastCommandEvent == current.timestamp + { + NSApp.sendEvent(current) + return + } print("SEL: \(selector)") }