Fix chord shortcuts on Windows non-Latin keyboard layouts#9476
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @landkirk on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm starting a first review of this pull request. I requested changes on this pull request and posted feedback. Comment I'm re-reviewing this pull request in response to a review request. I reviewed this pull request and requested human review from: @vorporeal. I left feedback as a comment so a maintainer can approve. Comment I reviewed this pull request and requested human review from: @zachbai. Comment You can view the conversation on Warp. I reviewed this pull request and requested human review from: @vorporeal. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds a Windows-only fallback that maps non-ASCII logical character keys with Ctrl/Super held back to their US-QWERTY physical-key characters so shortcut matching can work on non-Latin layouts.
Concerns
- The fallback also applies when Windows reports AltGr/right-Alt text entry as Ctrl+Alt, which can turn typed non-ASCII AltGr characters into Ctrl+Alt ASCII keystrokes and let shortcuts intercept text input. No security concerns were identified in the changed code.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @landkirk on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
b161c5f to
5bac90d
Compare
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds a Windows-only fallback that maps non-ASCII logical keys back to their US-QWERTY physical-key characters when Ctrl/Super chord shortcuts are pressed, plus unit coverage for the mapping helper.
Concerns
- No blocking correctness or security concerns found in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
There was a problem hiding this comment.
Overview
This PR adds a Windows-only US-QWERTY physical-key fallback for Ctrl/Super chord shortcuts when the active layout reports a non-ASCII logical key, plus unit coverage for the helper.
Concerns
- Shifted digit/punctuation chords are not preserved by the fallback:
Ctrl+Shift+]on a non-Latin layout becomesctrl-shift-]instead of the US-QWERTY shiftedctrl-shift-}, so default shifted-symbol bindings can still fail. - No security concerns found in the changed key-event mapping code.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
handing this off to @acarl005, who is our main windows guy |
On Windows, non-Latin keyboard layouts (Cyrillic, Greek, Arabic, etc.) translate the physical key to a non-ASCII character even when Ctrl/Cmd is held, so bindings like ctrl-c / ctrl-v fail to match. Detect that case and substitute the US-QWERTY character the physical key would produce, mirroring how VS Code, JetBrains, and Chromium handle it. Fixes warpdotdev#9036.
5bac90d to
86f3071
Compare
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds a Windows-only fallback that maps non-ASCII logical keys from Ctrl/Super chord events back to US-QWERTY physical-key characters, with unit coverage for the helper mappings and fallthrough cases.
Concerns
- No blocking correctness findings found in the changed diff.
- Security pass: no security concerns identified.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @vorporeal. Comment Powered by Oz |
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR adds a Windows-only fallback that maps non-ASCII Ctrl/Super chord characters back to their US-QWERTY physical-key equivalent, with tests for the helper mapping and unmapped keys. I did not find correctness or security issues in the changed lines.
Concerns
- None.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Fixes #9036.
Description
On Windows, non-Latin keyboard layouts (Cyrillic, Greek, Arabic, etc.) translate the physical key to a non-ASCII character even when Ctrl/Cmd is held. That makes bindings like
ctrl-c/ctrl-v(and the terminal's copy/paste shortcuts) fail to match — users have to switch their system layout to English just to copy or paste in Warp.This change detects that situation in
convert_keyboard_input_eventand substitutes the US-QWERTY character that the physical key would produce, so chord shortcuts work regardless of the active OS layout. Same approach used by VS Code, JetBrains, and Chromium.The fallback is gated on
cfg(windows)and on Ctrl/Super being held with a non-ASCII character — typed text on non-Latin layouts is unaffected.Testing
Unit tests in
key_events_tests.rscover the newus_qwerty_fallback_for_chordhelper:KeyA→a, etc.)Nonefor keys outside the chord-shortcut set (function keys, modifiers, navigation), so the originallogical_keyis preservedNoneforPhysicalKey::UnidentifiedManually verified on Windows:
Server API dependencies
No server API changes.
Agent Mode
CHANGELOG-BUG-FIX: Fixed Ctrl/Cmd shortcuts (e.g. copy, paste) failing on Windows when a non-Latin keyboard layout was active.