Repository navigation
Keep AltGr text out of terminal application shortcuts - #681
Merged
Merged
Conversation
Windows reports AltGr as Ctrl+Alt. TerminalSurface.onKey decided whether a key was an application shortcut from Ctrl and Shift alone, so AltGr+O, AltGr+J and AltGr+comma (and Ctrl+Alt+PageUp/PageDown, and Ctrl+Alt+[ or ] with Shift) ran the Ctrl shortcut instead of typing the layout's character. isApplicationShortcut now also takes Alt and never matches a Ctrl+Alt chord. Also narrow comments that claimed more than is implemented: the key encoder documents that only application cursor keys is exercised, that releases never reach it, and that only numpad Enter is told apart among keypad keys. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com>
3 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Windows, AltGr reaches the shell as Ctrl+Alt.
TerminalSurface.onKeydecided whether a key was an application shortcut from Ctrl and Shift alone, so AltGr+O, AltGr+J and AltGr+comma ran the plain-Ctrl shortcut (open folder, jump, settings) instead of typing the layout's character. This keeps every Ctrl+Alt chord except Ctrl+Alt+Tab out of the terminal application shortcuts, so its text reaches the terminal. It also narrows two code comments that claimed more than is implemented. It is one of a small series that follows up the review of #676, #677 and #679; it touches no mouse, paste or focus code.Verified against the code before changing it:
TerminalKeyEncoding.encodesAltChordalready left Ctrl+Alt alone, butisApplicationShortcut(TerminalSurface.zig) took onlyctrlandshift, soonKeycalled the shortcut route first and returned before the WM_CHAR text path mattered.Changes
isApplicationShortcuttakesaltand returns false for any Ctrl+Alt chord;onKeypasses it. Ctrl+Alt+PageUp/PageDown stay with the window's own accelerators (handled before the key reaches the terminal); a Ctrl+Alt+PageUp that does reach the terminal now goes to the key encoder instead of selecting the previous tab (the bytes the encoder produces for it are not asserted by any test).isApplicationShortcutstill returns on VK_TAB before it looks at Alt, andonKeyroutes Alt+Tab separately, so Ctrl+Alt+Tab remains an application shortcut. Windows itself normally takes Alt+Tab before the terminal sees it, and no layout types text on it.TerminalKeyEncoding.zigdoc comments no longer claim the encoder consumes Kitty keyboard flags or modifyOtherKeys: they now say only application cursor keys is tested, releases never reach the encoder, and of the keypad keys only numpad Enter is told apart.onKeycarries a matching one-line note on key releases. No behavior change from the comment edits.What each test proves
AltGr (Ctrl+Alt) never triggers an application shortcut, so its text reaches the terminal(TerminalSurface.zig): calls the productionTerminalSurface.onKeycallback directly (not the window or accelerator path) with Ctrl+Alt on ten virtual keys modelled on Polish (O, A, L), German (Q, 7, E), French (0, J), Spanish (2) and an AltGr+comma layout, asserts the shortcut callback is not called and nothing is queued, then callsonTextwith the layout's character and asserts exactly those bytes are queued. The WM_CHAR text is supplied by the test, not produced by a layout.Ctrl+Alt held on every application-shortcut key leaves the key to the terminal: the same for every keyisApplicationShortcutaccepts (O, J, comma, PageUp, PageDown, Ctrl+Shift+[ and ]), with and without Shift; and plain Ctrl on the same keys still reaches the shortcut route.TerminalSurface.isApplicationShortcut rejects every shortcut key once Alt is held with Ctrl (AltGr): the predicate alone.live terminal keyboard: AltGr characters reach the shell and never run a shell shortcut(App.zig): a real Winghostty surface window; a plain WM_KEYDOWN with Ctrl and Alt set viaSetKeyboardStategoes throughMainWindow.dispatchMessageand the production accelerator table, thenWM_CHARis sent to the surface window; asserts no shortcut ran and the character's UTF-8 bytes arrive in the terminal input queue (6 cases). Limit: it does not prove that the window accelerators declined the chords. Any charactersTranslateMessagequeued are discarded and the layout character is injected by the test, and the test observes only the surface callback and the input queue, not accelerator or app state. The accelerator table has no Ctrl+Alt entry for these keys (only Ctrl+Alt+PageUp/PageDown), which is read from the table, not asserted by this test.Which tests were RED before the fix (run against the unfixed
onKey): both newTerminalSurface.zigbehavior tests and the live test. The predicate test passes on the unfixedonKey, because it calls the already-changed predicate directly; it is a unit test of the new signature, not a RED test.What is NOT tested
TranslateMessageoutput is discarded and the layout's character is posted by hand. A real AltGr key sequence, including the fake left Ctrl key event Windows sends first, is not exercised.USERPROFILE,APPDATA,LOCALAPPDATA,TEMP,TMPandGRAPHCODE_SUPPORT_DIRin a scratch profile; no installed daemon or per-user zmx was used.Test plan
RED: zig test src\TerminalSurface.zig --test-filter "Ctrl+Alt" (pinned target and link flags, unfixed onKey) -> 0 passed, 2 failed: Polish AltGr vk 0x4f: 1 shortcut calls; expected 0, found 1 (and expected 0, found 12 for every shortcut key); the live App test also failed with AltGr vk 0x4f ran shortcut key 79
GREEN: zig test src\App.zig --test-filter "AltGr" (pinned flags, with the fix) -> All 4 tests passed; zig test src\TerminalSurface.zig --test-filter "Ctrl+Alt" -> the two behavior tests pass
REGRESSION: zig test for TerminalKeys, TerminalVt, TerminalKeyEncoding, TerminalSelection, Clipboard, TerminalSurface and App roots (pinned flags) -> 8, 22, 36, 35, 4, 223 and 1001 tests passed, 0 failed (baseline before the change: TerminalSurface 220, App 997)
Checklist
git commit -s) per the DCOmake test) - macOS target, not run: this change is Windows-only; the Windows roots above were run insteadmake check) - macOS lint target, not run: Windows-only change in Zig