fix(tui): allow AltGr printable keys on Windows (#5922) #380
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[cfg(target_os = "windows")] | ||
| KeyEvent { | ||
| code: KeyCode::Char(c), | ||
| modifiers, | ||
| .. | ||
| } if (modifiers == (KeyModifiers::CONTROL | KeyModifiers::ALT) | ||
| || modifiers == (KeyModifiers::CONTROL | ||
| | KeyModifiers::ALT | ||
| | KeyModifiers::SHIFT)) | ||
| && !c.is_ascii_control() | ||
| && !c.is_ascii_alphabetic() => | ||
| { | ||
| self.insert_str(&c.to_string()); |
There was a problem hiding this comment.
This branch makes TextArea::input treat Control+Alt key chords as printable on Windows, so the chat composer now works with AltGr. However, the terminal overlay still filters out every KeyCode::Char whenever any of CONTROL, ALT, or SUPER modifiers are set (terminal_handle_pending_key keeps returning handled = true without inserting the char). Pressing AltGr in terminal cells therefore remains impossible even after this change. If the goal is to permit AltGr everywhere, the terminal handler needs the same exception; otherwise the bug persists for terminal input.
Useful? React with 👍 / 👎.
zemaj
force-pushed
the
issue-5922-altgr
branch
from
b3c23fe to
3ccd483
Compare
|
Works on my machine (win 11, wt, ger keyboard) |
zemaj
left a comment
zemaj
left a comment
There was a problem hiding this comment.
Pulled the branch and exercised the new AltGr path. Highlights:
- Verified the Control+Alt insertion branch only fires for non-alphabetic chars so Ctrl+Alt shortcuts (e.g. delete-word) still route through the existing handlers.
- Added non-Windows TextArea + ComposerInput regression tests to prove Ctrl+Alt symbol chords stay suppressed on macOS/Linux while the Windows AltGr tests continue to pass.
- Ran the targeted suites (, ) and Cache bucket: main-0d6e4079e367-63a036970bd9 (branch/worktree)
Using rustup toolchain: 1.90.0
rustc 1.90.0 (1159e78c4 2025-09-14)
Building code binary (dev-fast mode)...
Building bins: code
✅ Build successful!
Binary location: /Users/zemaj/www/just-every/code/.code/working/_target-cache/code/main-0d6e4079e367-63a036970bd9/code-rs/dev-fast/code
Binary Hash: 8147106423b34c147e0e55765644fd2f7ddc35bc0da4469b4ac0f4f330183a83 (101M); all clean.
Remaining risk: Windows-only coverage still depends on CI/manual validation, so give the change a real keyboard smoke on Windows before merging.
zemaj
left a comment
zemaj
left a comment
There was a problem hiding this comment.
Follow-up to avoid shell substitution noise in the earlier review comment.
- Verified the Control+Alt insertion branch only fires for non-alphabetic chars so Ctrl+Alt shortcuts (e.g. delete-word) still route through the existing handlers.
- Added non-Windows TextArea + ComposerInput regression tests to prove Ctrl+Alt symbol chords stay suppressed on macOS/Linux while the Windows AltGr tests continue to pass.
- Ran the targeted suites (
cargo test -p code-tui --features test-helpers --test non_windows_shortcuts,cargo test -p code-tui --features test-helpers --test windows_altgr) and./build-fast.sh; all clean.
Remaining risk: Windows-only coverage still depends on CI/manual validation, so give the change a real keyboard smoke on Windows before merging.
Summary
/,@, and other symbols insert correctly in the composer and terminal cellsTextAreaunit tests and a ComposerInput integration-style test to prevent future regressionsTesting
cc1.2.41 is missing generated modules; clear/update the registry and rerun on Windows)Closes openai#5922.