-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Fix TUI issues with Alt-Gr on Windows #6799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
nornagon-openai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic looks correct but i'd prefer this get integrated into key_hint.rs instead of one-off'd in various places to ensure consistent behavior.
|
OK, I moved the |
nornagon-openai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah sure, i think i actually slightly misunderstood this and thought there was a specific AltGr code you were trying to catch (or avoid catching), but this is on arbitrary chars. lgtm.
|
There's some long standing work on AltGr in crossterm that has a bit more context on this problem. The main part of solving it in the followup PRs was that doing something that works on keyboards that do have AltGr keys, broke things on keyboards without it. I haven't looked at the solution here (only the description), but it's worth noting that it may have a similar problem where fixing the problem for international keyboards introduces a problem for non international keyboards. It may be that we don't have this problem at the app layer however. crossterm-rs/crossterm#820 (not a blocker - merely awareness / context) |
This PR fixes keyboard handling for the Right Alt (aka "Alt-Gr") key on Windows. This key appears on keyboards in Central and Eastern Europe. Codex has effectively never worked for Windows users in these regions because the code didn't properly handle this key, which is used for typing common symbols like
\and@.A few days ago, I merged a community-authored PR that supplied a partial fix for this issue. Upon closer inspect, that PR was 1) too broad (not scoped to Windows only) and 2) incomplete (didn't fix all relevant code paths, so paste was still broken).
This improvement is based on another community-provided PR by @marektomas-cz. He submitted it back in September and later closed it because it didn't receive any attention.
This fix addresses the following bugs: #5922, #3046, #3092, #3519, #5684, #5843.