From d30eb31c822b0d5f2cc4cbc96e9f36daacc80503 Mon Sep 17 00:00:00 2001 From: "Dongmin, Yu" Date: Tue, 29 Sep 2026 00:20:18 +0900 Subject: [PATCH] Match Cmd shortcuts on the ASCII-capable layout when the input source is not Latin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On macOS, the key of a keystroke came from charactersIgnoringModifiers, which reports the active keyboard layout's own character. Under a layout that is not ASCII-capable, such as Korean 2-Set, the I key reports ㅑ, so Cmd+I became `cmd-ㅑ` and never matched a binding written as `cmd-i`, so the Terminal/Agent mode toggle did nothing until the user switched back to an English input source. The same NSEvent reports `characters` as `i`; only the modifier-ignoring string carries the Hangul letter. For a Cmd keystroke whose reported character is not ASCII, the key now comes from translating the key code through the current ASCII-capable keyboard layout, with Shift applied. That reproduces what the English input source reports for the same key, including shifted symbols (`I`, `!`, `{`). The lookup returns nothing when the current layout is already ASCII-capable, so Latin layouts keep their own characters (`cmd-ö` on German stays `cmd-ö`). Named keys, keystrokes without Cmd, and ASCII characters take the same path as before, so typing and Option combinations are unchanged. The decision lives in a pure keystroke_key function with unit tests; the key code translation is a new keyCodeToAsciiCapableChar beside keyCodeToChar. Ctrl keystrokes are left alone here. Closes #8547. --- crates/warpui/src/platform/mac/event.rs | 48 ++++++++++++--- crates/warpui/src/platform/mac/event_tests.rs | 60 +++++++++++++++++++ crates/warpui/src/platform/mac/keycode.rs | 38 +++++++++--- crates/warpui/src/platform/mac/objc/keycode.m | 31 ++++++++++ 4 files changed, 160 insertions(+), 17 deletions(-) create mode 100644 crates/warpui/src/platform/mac/event_tests.rs diff --git a/crates/warpui/src/platform/mac/event.rs b/crates/warpui/src/platform/mac/event.rs index b9495194765..8c20c487043 100644 --- a/crates/warpui/src/platform/mac/event.rs +++ b/crates/warpui/src/platform/mac/event.rs @@ -35,6 +35,34 @@ fn native_key_code_to_key_code(native_key_code: u16) -> Option { } } +/// The key a keystroke carries, from the characters AppKit reported for the key without +/// modifiers. `None` when AppKit reported no character at all. +/// +/// A keyboard layout that is not ASCII-capable, such as Korean 2-Set, reports its own +/// letters here (ㅑ for the I key, even with Command held), so a Command keystroke would +/// never match a binding written as `cmd-i`. For a Command keystroke on a non-ASCII +/// character this asks `ascii_capable_key` for the key on the ASCII-capable layout +/// instead, which is what the same key reports when that layout is active. Every other +/// keystroke keeps the layout's own character, so typing and Option combinations are +/// unchanged. +fn keystroke_key( + unmodified_chars: &str, + cmd: bool, + ascii_capable_key: impl FnOnce() -> Option, +) -> Option { + let first_char = unmodified_chars.chars().next()?; + if let Some(named_key) = unicode_char_to_key(first_char as u16) { + return Some(named_key.to_owned()); + } + if cmd + && !first_char.is_ascii() + && let Some(key) = ascii_capable_key() + { + return Some(key); + } + Some(unmodified_chars.to_owned()) +} + /// # Safety /// This code is only unsafe since it requires interfacing with platform code. /// Creates an event from a native event, taking in the current window_height and whether this is @@ -78,19 +106,19 @@ pub unsafe fn from_native( .to_str() .ok()?; - let unmodified_chars = if let Some(first_char) = unmodified_chars.chars().next() { - unicode_char_to_key(first_char as u16).unwrap_or(unmodified_chars) - } else { - return None; - }; + let cmd = native_modifiers.contains(NSEventModifierFlags::Command); + let shift = native_modifiers.contains(NSEventModifierFlags::Shift); + let key = keystroke_key(unmodified_chars, cmd, || { + Keycode(native_event.keyCode()).try_to_ascii_capable_key_name(shift) + })?; let keystroke = Keystroke { ctrl: native_modifiers.contains(NSEventModifierFlags::Control), alt: native_modifiers.contains(NSEventModifierFlags::Option), - shift: native_modifiers.contains(NSEventModifierFlags::Shift), - cmd: native_modifiers.contains(NSEventModifierFlags::Command), + shift, + cmd, meta: false, /* handled separately */ - key: unmodified_chars.into(), + key, }; let characters = native_event.characters(); @@ -253,3 +281,7 @@ pub unsafe fn from_native( } } } + +#[cfg(test)] +#[path = "event_tests.rs"] +mod tests; diff --git a/crates/warpui/src/platform/mac/event_tests.rs b/crates/warpui/src/platform/mac/event_tests.rs new file mode 100644 index 00000000000..e1b3923c39c --- /dev/null +++ b/crates/warpui/src/platform/mac/event_tests.rs @@ -0,0 +1,60 @@ +use super::keystroke_key; + +fn not_consulted() -> Option { + panic!("the ASCII-capable layout should not be consulted for this keystroke") +} + +#[test] +fn a_command_keystroke_on_a_non_latin_letter_takes_the_ascii_capable_key() { + // Korean 2-Set reports ㅑ for the I key; Cmd+I must still be `cmd-i`. + assert_eq!( + keystroke_key("ㅑ", true, || Some("i".to_owned())).as_deref(), + Some("i") + ); + // Shift is part of the translation, the way it is on the ASCII-capable layout. + assert_eq!( + keystroke_key("ㅑ", true, || Some("I".to_owned())).as_deref(), + Some("I") + ); +} + +#[test] +fn a_keystroke_without_command_keeps_the_layouts_character() { + assert_eq!( + keystroke_key("ㅑ", false, not_consulted).as_deref(), + Some("ㅑ") + ); +} + +#[test] +fn an_ascii_character_is_left_alone() { + assert_eq!( + keystroke_key("i", true, not_consulted).as_deref(), + Some("i") + ); + assert_eq!( + keystroke_key("!", true, not_consulted).as_deref(), + Some("!") + ); +} + +#[test] +fn a_named_key_keeps_its_name() { + // AppKit reports the up arrow as the private-use character U+F700. + assert_eq!( + keystroke_key("\u{F700}", true, not_consulted).as_deref(), + Some("up") + ); +} + +#[test] +fn a_layout_that_is_already_ascii_capable_keeps_its_own_letters() { + // German reports ö for its own key, and the ASCII-capable layout is German itself, + // so the lookup returns nothing and the keystroke stays `cmd-ö`. + assert_eq!(keystroke_key("ö", true, || None).as_deref(), Some("ö")); +} + +#[test] +fn no_characters_means_no_key() { + assert_eq!(keystroke_key("", true, not_consulted), None); +} diff --git a/crates/warpui/src/platform/mac/keycode.rs b/crates/warpui/src/platform/mac/keycode.rs index 04fa39ecda9..e7b791dac17 100644 --- a/crates/warpui/src/platform/mac/keycode.rs +++ b/crates/warpui/src/platform/mac/keycode.rs @@ -16,6 +16,7 @@ pub const CONTROL_KEY: u16 = 4096; unsafe extern "C" { fn charToKeyCodes(keyChar: id) -> id; fn keyCodeToChar(keyCode: NSUInteger, shifted: BOOL) -> id; + fn keyCodeToAsciiCapableChar(keyCode: u16, shifted: BOOL) -> id; } pub struct Keycode(pub u16); @@ -28,16 +29,19 @@ impl Keycode { // But clippy isn't smart enough to know that so we silence it here for now. #[allow(clippy::useless_conversion)] let key = keyCodeToChar(self.0 as u64, shift_key_pressed.into()); + nsstring_to_string(key) + } + } - if key.is_null() { - return None; - } - - let key = &*key.cast::(); - let cstr = key.UTF8String() as *const u8; - std::str::from_utf8(slice::from_raw_parts(cstr, key.len())) - .ok() - .map(|s| s.to_string()) + /// The key's name on the current ASCII-capable keyboard layout, the Latin layout the + /// user switches to alongside a layout such as Korean 2-Set. `None` when the current + /// layout is already ASCII-capable, or when the key has no printable character there. + pub fn try_to_ascii_capable_key_name(self, shift_key_pressed: bool) -> Option { + unsafe { + // See `try_to_key_name` for why the conversion is needed. + #[allow(clippy::useless_conversion)] + let key = keyCodeToAsciiCapableChar(self.0, shift_key_pressed.into()); + nsstring_to_string(key) } } @@ -62,6 +66,22 @@ impl Keycode { } } +/// # Safety +/// `key` must be null or point to a live `NSString`. +unsafe fn nsstring_to_string(key: id) -> Option { + if key.is_null() { + return None; + } + + unsafe { + let key = &*key.cast::(); + let cstr = key.UTF8String() as *const u8; + std::str::from_utf8(slice::from_raw_parts(cstr, key.len())) + .ok() + .map(|s| s.to_string()) + } +} + // Convert modifier flags to Carbon style modifier key mask. pub fn modifier_code(keystroke: &Keystroke) -> u16 { let mut code = 0; diff --git a/crates/warpui/src/platform/mac/objc/keycode.m b/crates/warpui/src/platform/mac/objc/keycode.m index 32b2dc59588..439ec53fe9f 100644 --- a/crates/warpui/src/platform/mac/objc/keycode.m +++ b/crates/warpui/src/platform/mac/objc/keycode.m @@ -158,6 +158,37 @@ UniChar TranslatedUnicodeCharFromKeyCode(CFDataRef layout_data, UInt16 key_code, } } +// Convert keycode to its character on the ASCII-capable keyboard layout, the Latin layout +// the user switches to alongside the current one. Returns nil when the current keyboard layout +// is already ASCII-capable, so the caller keeps the character AppKit reported. A layout +// such as Korean 2-Set reports its own letters from charactersIgnoringModifiers (ㅑ for +// the I key), which no Command binding is written in. +NSString* keyCodeToAsciiCapableChar(UInt16 keyCode, BOOL shifted) { + TISInputSourceRef current_layout = TISCopyCurrentKeyboardLayoutInputSource(); + CFBooleanRef is_ascii_capable = current_layout + ? (CFBooleanRef)TISGetInputSourceProperty(current_layout, + kTISPropertyInputSourceIsASCIICapable) + : NULL; + BOOL current_is_ascii_capable = is_ascii_capable && CFBooleanGetValue(is_ascii_capable); + if (current_layout) CFRelease(current_layout); + if (current_is_ascii_capable) return nil; + + TISInputSourceRef ascii_layout = TISCopyCurrentASCIICapableKeyboardLayoutInputSource(); + if (!ascii_layout) return nil; + CFDataRef layout_data = + (CFDataRef)(TISGetInputSourceProperty(ascii_layout, kTISPropertyUnicodeKeyLayoutData)); + + // The shift key representation in Carbon is 1 << 9; see keyCodeToChar. + UInt32 modifier_key_state = shifted ? 1 << 1 : 0; + UniChar translated_char = + TranslatedUnicodeCharFromKeyCode(layout_data, keyCode, modifier_key_state, LMGetKbdLast()); + // layout_data belongs to ascii_layout, so release only after translating. + CFRelease(ascii_layout); + + if (!layout_data || IsUnicodeControl(translated_char)) return nil; + return [NSString stringWithFormat:@"%C", translated_char]; +} + NSArray* charToKeyCodes(NSString* keyChar) { if (keycodeDict == nil) { keycodeDict = [[NSMutableDictionary alloc] init];