-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
GH-132439: Fix REPL swallowing characters entered with AltGr on cmd.exe #132440
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
Lib/_pyrepl/windows_console.py
Outdated
if block: | ||
continue | ||
|
||
return None | ||
elif self.__vt_support: | ||
# If virtual terminal is enabled, scanning VT sequences | ||
self.event_queue.push(rec.Event.KeyEvent.uChar.UnicodeChar) | ||
self.event_queue.push(raw_key) |
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.
A missed opportunity when main was merged during development of #124119.
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.
Which @sergey-miryanov takes care of here as part of #131901.
So maybe I should remove this one, but I'd really like to keep the other two raw_key
changes (not only because I'd have to adapt almost all tests :)
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.
IMHO you should merge my branch here :)
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.
I can undo the change - then you won't have conflicts. I don't think we should (partially) merge between our two PRs?
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.
Yeah, I think you are right - we shouldn't merge our PRs. You can keep your changes - I'm OK if here will be conflict.
Lib/_pyrepl/windows_console.py
Outdated
return Event(evt="key", data="\033") # keymap.py uses this for meta | ||
return Event(evt="key", data=key, raw=key) | ||
return Event(evt="key", data=key, raw=raw_key) |
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.
Looking at the diff , previously
return Event(
evt="key", data=code, raw=rec.Event.KeyEvent.uChar.UnicodeChar
)
was used, which in the new code should have been raw_key
?
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.
Actually, I was wondering why this change did not break anything, but AFAICT the raw
member is not used in the whole code base except
cpython/Lib/_pyrepl/unix_console.py
Line 513 in 5d8e432
def getpending(self): |
where the two
getpending
implementations dutifully respect e.raw += e.raw
:)
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.
When searching in the code base for getpending
I found
cpython/Lib/_pyrepl/windows_console.py
Lines 518 to 521 in 5d8e432
def getpending(self) -> Event: | |
"""Return the characters that have been typed but not yet | |
processed.""" | |
return Event("key", "", b"") |
which clearly seems to be a bug to me? Because even though WindowsConsole._read_input()
only reads one Windows INPUT_RECORD
per call, get_event
could have put something into self.event_queue
.
I've addressed this in https://github.com/python/cpython/pull/132889/files#r2059235071.
Adding @vstinner, @eendebakpt and @paulie4 , since they were involved in #128389 and already talked about AltGr. |
PS: switching to an english keyboard layout, I can use the AltGr key like right-Alt, since it is not used in this keyboard layout :) |
The failing of Ubuntu (free-threading) is definitely unrelated, since this is a Windows specific change. |
Do you think it'd be possible to add some tests for this change to ensure we don't accidentally regress again? |
Yeah, I've already thought about it, but am still unsure how to best do it. ATM, I am thinking of mocking cpython/Lib/_pyrepl/windows_console.py Line 411 in b87189d
to be able to test cpython/Lib/_pyrepl/windows_console.py Lines 426 to 432 in b87189d
AFAICT, all the existing tests just mock WDYT? |
Agreed that we shouldn't be mocking |
Misc/NEWS.d/next/Library/2025-04-12-16-29-42.gh-issue-132439.3twrU6.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Stan Ulbrych <[email protected]>
because it will be part of 131901, anywayy
because it is unused
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.
Excellent work again! Very well thought through.
Thanks @chris-eibl for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, @chris-eibl and @ambv, I could not cleanly backport this to
|
Will repeat here too - problem with backport because VT support was not backported #130805 (comment) (for completeness) |
Yeah, I am backporting the VT support too because it's the only way we can have a sensibly similar codebase for bug fixes. |
…ltGr on cmd.exe (pythonGH-132440) (cherry picked from commit 07f416a) Co-authored-by: Chris Eibl <[email protected]> Co-authored-by: Stan Ulbrych <[email protected]>
GH-133460 is a backport of this pull request to the 3.13 branch. |
…n cmd.exe (GH-132440) (GH-133460) (cherry picked from commit 07f416a) Co-authored-by: Chris Eibl <[email protected]> Co-authored-by: Stan Ulbrych <[email protected]>
E.g. on my keyboard with German layout,
{
is usually entered via pressing the AltGr key and7
, i.e.AltGr+7
.Likewise,
}
,[
,]
,\
and some more can only be entered viaAltGr
.But since #128388 / #128389 these are swallowed by the REPL on Windows and can no longer be entered.
This happens in legacy Windows terminals, where the virtual terminal mode is turned off (e.g.
cmd.exe
).In virtual terminal mode there are other issues, see #131878.