| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
| 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.
A missed opportunity when main was merged during development of #124119.
Sorry, something went wrong.
There was a problem hiding this comment.
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 :)
Sorry, something went wrong.
There was a problem hiding this comment.
IMHO you should merge my branch here :)
Sorry, something went wrong.
There was a problem hiding this comment.
I can undo the change - then you won't have conflicts. I don't think we should (partially) merge between our two PRs?
Sorry, something went wrong.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
| self.event_queue.insert(Event(evt="key", data=key, raw=raw_key)) | ||
| 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.
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?
Sorry, something went wrong.
There was a problem hiding this comment.
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
Sorry, something went wrong.
There was a problem hiding this comment.
When searching in the code base for getpending I found
cpython/Lib/_pyrepl/windows_console.py
Lines 518 to 521 in 5d8e432
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.
Sorry, something went wrong.
|
Adding @vstinner, @eendebakpt and @paulie4 , since they were involved in #128389 and already talked about AltGr. |
Sorry, something went wrong.
|
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 :) |
Sorry, something went wrong.
|
The failing of Ubuntu (free-threading) is definitely unrelated, since this is a Windows specific change. |
Sorry, something went wrong.
|
Do you think it'd be possible to add some tests for this change to ensure we don't accidentally regress again? |
Sorry, something went wrong.
|
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 get_event, but for this, IMHO get_event itself must be tested. WDYT? |
Sorry, something went wrong.
|
Agreed that we shouldn't be mocking get_event since that's where we're making changes. _read_input seems like a good option, we can capture some real inputs and then replay them with it. |
Sorry, something went wrong.
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
because it will be part of 131901, anywayy
because it is unused
There was a problem hiding this comment.
Excellent work again! Very well thought through.
Sorry, something went wrong.
|
Thanks @chris-eibl for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, something went wrong.
|
Sorry, @chris-eibl and @ambv, I could not cleanly backport this to 3.13 due to a conflict. cherry_picker 07f416a3f063db6b91b8b99ff61a51b64b0503f1 3.13 |
Sorry, something went wrong.
|
Will repeat here too - problem with backport because VT support was not backported #130805 (comment) (for completeness) |
Sorry, something went wrong.
|
Yeah, I am backporting the VT support too because it's the only way we can have a sensibly similar codebase for bug fixes. |
Sorry, something went wrong.
…ltGr on cmd.exe (pythonGH-132440) (cherry picked from commit 07f416a) Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
|
GH-133460 is a backport of this pull request to the 3.13 branch. |
Sorry, something went wrong.
… cmd.exe (pythonGH-132440) Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
| Back | FazBrowse Home | New Git URL |
E.g. on my keyboard with German layout, { is usually entered via pressing the AltGr key and 7, i.e. AltGr+7.
Likewise, }, [, ], \ and some more can only be entered via AltGr.
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.