GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-132439: Fix REPL swallowing characters entered with AltGr on cmd.exe by chris-eibl · Pull Request #132440 · python/cpython

Conversation

@chris-eibl

@chris-eibl chris-eibl commented Apr 12, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

  • Issue: New REPL on Windows swallows characters entered via AltGr #132439

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
cos4ni2s reacted with confused emoji
All reactions
  • 😕 1 reaction
chris-eibl added 2 commits April 12, 2025 16:01
@chris-eibl
do not swallow characters that have been entered via AltGr
59d71b0
@chris-eibl
blurb it
cca6155
@chris-eibl
chris-eibl requested review from ambv, lysnikolaou and pablogsal as code owners April 12, 2025 14:31
@bedevere-app bedevere-app Bot added the awaiting review label Apr 12, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Apr 12, 2025
New REPL on Windows swallows characters entered via AltGr #132439
Closed
chris-eibl
chris-eibl commented Apr 12, 2025
View reviewed changes
Comment thread Lib/_pyrepl/windows_console.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@chris-eibl chris-eibl added OS-windows 3.13 only security fixes 3.14 bugs and security fixes topic-repl Related to the interactive shell labels Apr 12, 2025
@chris-eibl
unrelated stuff worth to do (?)
1e61540
chris-eibl
chris-eibl commented Apr 12, 2025
View reviewed changes
Comment thread Lib/_pyrepl/windows_console.py Outdated
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)

@chris-eibl chris-eibl Apr 12, 2025

Copy link
Copy Markdown
Member Author

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@chris-eibl chris-eibl Apr 20, 2025

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@sergey-miryanov sergey-miryanov Apr 20, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@chris-eibl chris-eibl Apr 20, 2025

Copy link
Copy Markdown
Member Author

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@sergey-miryanov sergey-miryanov Apr 21, 2025

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
chris-eibl reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
chris-eibl
chris-eibl commented Apr 12, 2025
View reviewed changes
Comment thread Lib/_pyrepl/windows_console.py Outdated
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)

@chris-eibl chris-eibl Apr 12, 2025

Copy link
Copy Markdown
Member Author

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@chris-eibl chris-eibl Apr 20, 2025

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@chris-eibl chris-eibl Apr 20, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
chris-eibl
chris-eibl commented Apr 12, 2025
View reviewed changes
Comment thread Lib/_pyrepl/windows_console.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@chris-eibl
chris-eibl requested a review from vstinner April 12, 2025 14:49
@chris-eibl

chris-eibl commented Apr 12, 2025

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@chris-eibl

chris-eibl commented Apr 12, 2025

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz picnixz added needs backport to 3.13 only security fixes and removed 3.13 only security fixes 3.14 bugs and security fixes labels Apr 12, 2025
@chris-eibl

chris-eibl commented Apr 12, 2025

Copy link
Copy Markdown
Member Author

The failing of Ubuntu (free-threading) is definitely unrelated, since this is a Windows specific change.
tomasr8 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tomasr8

tomasr8 commented Apr 17, 2025

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@chris-eibl

chris-eibl commented Apr 17, 2025

Copy link
Copy Markdown
Member Author

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

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

to be able to test

cpython/Lib/_pyrepl/windows_console.py

Lines 426 to 432 in b87189d

def get_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."""
while self.event_queue.empty():
rec = self._read_input(block)

AFAICT, all the existing tests just mock get_event, but for this, IMHO get_event itself must be tested.
Mocking _read_input seems to be the best way forward.

WDYT?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tomasr8

tomasr8 commented Apr 17, 2025

Copy link
Copy Markdown
Member

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
chris-eibl added 3 commits April 19, 2025 08:30
@chris-eibl
Merge remote-tracking branch 'origin/main' into fix_altgr
c60a727
@chris-eibl
fix missing con.restore() in test_multiline_ctrl_z
ed035e5
@chris-eibl
add WindowsConsoleGetEventTests
69e8779
chris-eibl
chris-eibl commented Apr 20, 2025
View reviewed changes
Comment thread Lib/test/test_pyrepl/test_windows_console.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Apr 24, 2025
Pasting in new REPL is slow #130328
Closed
GH-130328: pasting in new REPL is slow on Windows #132884
Merged
chris-eibl
chris-eibl commented Apr 25, 2025
View reviewed changes
Comment thread Lib/test/test_pyrepl/test_windows_console.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
chris-eibl and others added 6 commits April 25, 2025 21:40
@chris-eibl @StanFromIreland
Apply suggestions from code review
e67452b 
Co-authored-by: Stan Ulbrych <[email protected]>
@chris-eibl
Update Lib/test/test_pyrepl/test_windows_console.py
396f48e
@chris-eibl
Merge branch 'main' into fix_altgr
40b339a
@chris-eibl
revert change in virtual terminal path
1fbe240 
because it will be part of 131901, anywayy
@chris-eibl
Merge remote-tracking branch 'origin/main' into fix_altgr
2d59563
@chris-eibl
remove raw part of Event in get_event() in case of windows_console
adb89ba 
because it is unused
chris-eibl
chris-eibl commented May 5, 2025
View reviewed changes
Comment thread Lib/test/test_pyrepl/test_windows_console.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ambv ambv changed the title GH-132439: REPL on Windows swallows characters entered via AltGr GH-132439: Fix REPL swallowing characters entered with AltGr May 5, 2025
ambv
ambv approved these changes May 5, 2025
View reviewed changes

@ambv ambv left a comment

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
hugovk and MajorTanya reacted with hooray emoji chris-eibl, sergey-miryanov, and MajorTanya reacted with heart emoji hugovk reacted with rocket emoji
All reactions
  • 🎉 2 reactions
  • ❤️ 3 reactions
  • 🚀 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels May 5, 2025
@ambv
ambv merged commit 07f416a into python:main May 5, 2025
@miss-islington-app

miss-islington-app Bot commented May 5, 2025

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label May 5, 2025
@miss-islington-app

miss-islington-app Bot commented May 5, 2025

Copy link
Copy Markdown

Sorry, @chris-eibl and @ambv, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 07f416a3f063db6b91b8b99ff61a51b64b0503f1 3.13
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app miss-islington-app Bot assigned ambv May 5, 2025
@ambv ambv changed the title GH-132439: Fix REPL swallowing characters entered with AltGr GH-132439: Fix REPL swallowing characters entered with AltGr on cmd.exe May 5, 2025
@sergey-miryanov

sergey-miryanov commented May 5, 2025

Copy link
Copy Markdown
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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ambv

ambv commented May 5, 2025

Copy link
Copy Markdown
Contributor

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
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ambv pushed a commit to ambv/cpython that referenced this pull request May 5, 2025
@chris-eibl @StanFromIreland @ambv
[3.13] pythonGH-132439: Fix REPL swallowing characters entered with A…
3662540 
…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]>
@bedevere-app

bedevere-app Bot commented May 5, 2025

Copy link
Copy Markdown

GH-133460 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label May 5, 2025
ambv added a commit that referenced this pull request May 5, 2025
@ambv @chris-eibl @StanFromIreland
[3.13] GH-132439: Fix REPL swallowing characters entered with AltGr o…
e4d6848 
…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]>
@chris-eibl
chris-eibl deleted the fix_altgr branch May 5, 2025 18:14
@eendebakpt eendebakpt mentioned this pull request Jun 3, 2025
Dead keys on Windows with German keyboard layout #135105
Closed
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@chris-eibl @StanFromIreland @Pranjal095
pythonGH-132439: Fix REPL swallowing characters entered with AltGr on…
bace14d 
… cmd.exe (pythonGH-132440)

Co-authored-by: Stan Ulbrych <[email protected]>
@chris-eibl chris-eibl mentioned this pull request Mar 13, 2026
Event.raw being added to itself instead of to another event's .raw in UnixConsole.getpending #145886
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StanFromIreland StanFromIreland StanFromIreland left review comments
@ambv ambv ambv approved these changes
@sergey-miryanov sergey-miryanov sergey-miryanov approved these changes
@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
@vstinner vstinner Awaiting requested review from vstinner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@ambv ambv

Labels

OS-windows topic-repl Related to the interactive shell

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@chris-eibl @tomasr8 @sergey-miryanov @ambv @StanFromIreland @picnixz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.