Match Cmd shortcuts on the ASCII-capable layout under non-Latin input sources - #16192
AndrewDongminYoo wants to merge 1 commit into
Conversation
… is not Latin
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 warpdotdev#8547.
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR updates macOS key event conversion so Command shortcuts under non-Latin input sources can fall back to the current ASCII-capable keyboard layout, while preserving existing behavior for named keys, non-Command input, ASCII characters, and ASCII-capable layouts. The change includes focused unit coverage for the new keystroke-key decision logic.
Concerns
- No blocking correctness, security, comment-quality, test-quality, or spec-drift concerns found in the annotated diff.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /warp-agent-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
Sorry for the confusion. Issue #8547 should've been marked as a duplicate and it's certainly not |
|
Thanks for the clear answer, and no problem. I'll stay out of the keybinding area. In case it is useful to whoever picks this up internally (it overlaps with the draft #15197 in the same
I'll leave the branch on my fork in case any of it helps. |
Description
On macOS, a Cmd shortcut that is not also a menu item did nothing while a keyboard layout that is not ASCII-capable was active. #8547 reports it for the Terminal/Agent input mode toggle (⌘I) under the Korean 2-Set input source; it works again as soon as the user switches to an English input source.
The key of a keystroke came from
NSEvent.charactersIgnoringModifiers, which reports the active layout's own character even with Cmd held. Measured with locally constructedNSEvents under Korean 2-Set, Cmd+I reportscharacters = "i"butcharactersIgnoringModifiers = "ㅑ", so the keystroke becamecmd-ㅑand never matchedcmd-i.from_nativeincrates/warpui/src/platform/mac/event.rsis the only place this conversion happens, and bothkeyDown:and theperformKeyEquivalent:binding checks go through it.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 (
TISCopyCurrentASCIICapableKeyboardLayoutInputSource), with Shift applied. That reproduces what the English input source reports for the same key, including shifted symbols: under Korean 2-Set the translation givesi,I,!and{for Cmd+I, Cmd+Shift+I, Cmd+Shift+1 and Cmd+Shift+[, the same keys the ABC layout reports. Everything else takes the path it took before:cmd-öon German stayscmd-ö).The decision is a pure
keystroke_keyfunction with unit tests in a siblingevent_tests.rs; the key code translation is a newkeyCodeToAsciiCapableCharbesidekeyCodeToCharinkeycode.m.Scope and related work
us_qwerty_fallback_for_chord) does. The modifier gate here is a single condition, so the two can be reconciled whichever lands first.keyDownImplinhost_view.minserts IME-committed text only when the key was not handled. This already happens today with ⌘↵, which matched under Korean before this change (type안녕하and press ⌘↵: only안녕remains). This PR extends the set of Cmd shortcuts that work under Korean, and so the set that can hit it; fixing it means inserting committed text before the shortcut runs, which changes the event order for all IME input, so I would rather do it separately.cmd-ㅑstops matching it.details.key_without_modifiersstill reports the layout character (the kitty keyboard protocol path, see macOS: kitty keyboard protocol never reports the base layout key, so Ctrl+<letter> is unmatchable on non-Latin layouts #15646), and global hotkeys resolve throughcharToKeyCodes, which is built from the layout active at its first call.The hint half of #8547
The issue also reports a ⌘Space hint that stays visible after turning off "Show input hint text". Neither the installed stable build nor this branch shows a ⌘Space hint any more; the message line now reads
⌘↵ new /agent conversation, derived from the binding. That line is controlled by the "terminal input message line" setting, not "Show input hint text", which only controls the input placeholder. So the remaining behavior is two separate settings rather than a bug, and I have not changed it here.Linked Issue
Closes #8547.
ready-to-implement.Testing
cargo nextest run -p warpui --lib -E 'test(/platform::mac::event::tests/)': 6 passed. With the fallback disabled,a_command_keystroke_on_a_non_latin_letter_takes_the_ascii_capable_keyfails, as it should.cargo fmt -p warpui -- --check,cargo clippy -p warpui --all-targets --tests -- -D warnings,./script/check_no_inline_test_modules: passed.keycode.mwas formatted by hand to match the surrounding code, sinceclang-formatis not installed on this machine../script/presubmitwas not run in full for the same reason (it also needswgslfmtandpwsh).Manual testing with
./script/run, logged in, macOS with the ABC and Korean 2-Set input sources:하is still composing toggles the mode and drops하(the pre-existing limitation above)../script/runScreenshots / Videos
The recording follows the four manual steps under Testing, in order. The input source indicator in the menu bar shows which source is active in each segment: the installed stable Warp first, then the
WarpOssbuild of this branch.Keystrokes are not overlaid on screen; in each segment the key pressed is the one named in the matching Testing step (⌘I, or ⌘T in step 3), and the mode or tab change that follows is its result.
Screen.Recording.2026-09-29.at.9.31.22.AM.mov
Agent Mode
CHANGELOG-BUG-FIX: Cmd keyboard shortcuts such as ⌘I now work on macOS while a non-Latin input source such as Korean 2-Set is active.