Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ambv merged 13 commits intopython:mainfromchris-eibl:fix_altgr
May 5, 2025

Conversation

chris-eibl
Copy link
Member

@chris-eiblchris-eibl commentedApr 12, 2025•
edited
Loading

E.g. on my keyboard with German layout,{ is usually entered via pressing theAltGr 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.

cos4ni2s reacted with confused emoji
@chris-eiblchris-eibl added OS-windows 3.13bugs and security fixes 3.14bugs and security fixes topic-replRelated to the interactive shell labelsApr 12, 2025
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)
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

A missed opportunity whenmain was merged during development of#124119.

Copy link
MemberAuthor

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 ofhere as part of#131901.

So maybe I should remove this one, but I'd really like to keep the other tworaw_key changes (not only because I'd have to adapt almost all tests :)

Copy link
Contributor

@sergey-miryanovsergey-miryanovApr 20, 2025•
edited
Loading

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 :)

Copy link
MemberAuthor

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?

Copy link
Contributor

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.

chris-eibl reacted with thumbs up emoji
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)
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looking at thediff, previously

returnEvent(evt="key",data=code,raw=rec.Event.KeyEvent.uChar.UnicodeChar)

was used, which in the new code should have beenraw_key?

Copy link
MemberAuthor

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 theraw member is not used in the whole code base except

defgetpending(self):

where the twogetpending implementations dutifully respecte.raw += e.raw :)

Copy link
MemberAuthor

@chris-eiblchris-eiblApr 20, 2025•
edited
Loading

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 forgetpending I found

defgetpending(self)->Event:
"""Return the characters that have been typed but not yet
processed."""
returnEvent("key","",b"")

which clearly seems to be a bug to me? Because even thoughWindowsConsole._read_input() only reads one WindowsINPUT_RECORD per call,get_event could have put something intoself.event_queue.

I've addressed this inhttps://github.com/python/cpython/pull/132889/files#r2059235071.

@chris-eibl
Copy link
MemberAuthor

Adding@vstinner,@eendebakpt and@paulie4 , since they were involved in#128389 and already talked about AltGr.

@chris-eibl
Copy link
MemberAuthor

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 :)

@picnixzpicnixz added needs backport to 3.13bugs and security fixes and removed 3.13bugs and security fixes 3.14bugs and security fixes labelsApr 12, 2025
@chris-eibl
Copy link
MemberAuthor

The failing of Ubuntu (free-threading) is definitely unrelated, since this is a Windows specific change.

tomasr8 reacted with thumbs up emoji

@tomasr8
Copy link
Member

Do you think it'd be possible to add some tests for this change to ensure we don't accidentally regress again?

@chris-eibl
Copy link
MemberAuthor

Yeah, I've already thought about it, but am still unsure how to best do it.

ATM, I am thinking of mocking

def_read_input(self,block:bool=True)->INPUT_RECORD|None:

to be able to test
defget_event(self,block:bool=True)->Event|None:
"""Return an Event instance. Returns None if |block| is false
and there is no event pending, otherwise waits for the
completion of an event."""
whileself.event_queue.empty():
rec=self._read_input(block)

AFAICT, all the existing tests just mockget_event, but for this, IMHOget_event itself must be tested.
Mocking_read_input seems to be the best way forward.

WDYT?

@tomasr8
Copy link
Member

Agreed that we shouldn't be mockingget_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.

@ambvambv changed the titleGH-132439: REPL on Windows swallows characters entered via AltGrGH-132439: Fix REPL swallowing characters entered with AltGrMay 5, 2025
Copy link
Contributor

@ambvambv left a 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.

hugovk and MajorTanya reacted with hooray emojichris-eibl, sergey-miryanov, and MajorTanya reacted with heart emojihugovk reacted with rocket emoji
@ambvambv merged commit07f416a intopython:mainMay 5, 2025
52 checks passed
@miss-islington-app
Copy link

Thanks@chris-eibl for the PR, and@ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry,@chris-eibl and@ambv, I could not cleanly backport this to3.13 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker 07f416a3f063db6b91b8b99ff61a51b64b0503f1 3.13

@ambvambv changed the titleGH-132439: Fix REPL swallowing characters entered with AltGrGH-132439: Fix REPL swallowing characters entered with AltGr on cmd.exeMay 5, 2025
@sergey-miryanov
Copy link
Contributor

Will repeat here too - problem with backport because VT support was not backported#130805 (comment) (for completeness)

chris-eibl reacted with thumbs up emoji

@ambv
Copy link
Contributor

ambv commentedMay 5, 2025

Yeah, I am backporting the VT support too because it's the only way we can have a sensibly similar codebase for bug fixes.

sergey-miryanov and chris-eibl reacted with heart emoji

ambv pushed a commit to ambv/cpython that referenced this pull requestMay 5, 2025
…ltGr on cmd.exe (pythonGH-132440)(cherry picked from commit07f416a)Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@bedevere-app
Copy link

GH-133460 is a backport of this pull request to the3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13bugs and security fixes labelMay 5, 2025
ambv added a commit that referenced this pull requestMay 5, 2025
…n cmd.exe (GH-132440) (GH-133460)(cherry picked from commit07f416a)Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@chris-eiblchris-eibl deleted the fix_altgr branchMay 5, 2025 18:14
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@StanFromIrelandStanFromIrelandStanFromIreland left review comments

@ambvambvambv approved these changes

@sergey-miryanovsergey-miryanovsergey-miryanov approved these changes

@pablogsalpablogsalAwaiting requested review from pablogsalpablogsal is a code owner

@lysnikolaoulysnikolaouAwaiting requested review from lysnikolaoulysnikolaou is a code owner

@vstinnervstinnerAwaiting requested review from vstinner

Assignees

@ambvambv

Labels
OS-windowstopic-replRelated to the interactive shell
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@chris-eibl@tomasr8@sergey-miryanov@ambv@StanFromIreland@picnixz
[8]ページ先頭
©2009-2025 Movatter.jp