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-131878: Fix input of unicode characters with two or more code points in new pyrepl on Windows by sergey-miryanov · Pull Request #131901 · python/cpython

Conversation

@sergey-miryanov

@sergey-miryanov sergey-miryanov commented Mar 30, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

If unicode characters with two or more codepoints (ñ or é) typed or pasted, then it should be converted to bytes before passing to eventqueue.push.

Also fixed handling of exceptions while decoding buffer. If exception occurred then buffer should be flushed to prevent mixing it with following control commands (for example "\x1b[201").

  • Issue: New REPL on Windows exits when accented character is pasted/typed #131878

Sorry, something went wrong.

Uh oh!

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

All reactions
@sergey-miryanov
Fix input of long unicode characters
04812d4
- with two codepoints or more
@sergey-miryanov
sergey-miryanov requested review from ambv, lysnikolaou and pablogsal as code owners March 30, 2025 12:01
@bedevere-app bedevere-app Bot added the awaiting review label Mar 30, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Mar 30, 2025
New REPL on Windows exits when accented character is pasted/typed #131878
Closed
sergey-miryanov added 2 commits March 30, 2025 19:49
@sergey-miryanov
Add new entry
9203df4
@sergey-miryanov
Do not change simple_interact
9d7e3e5
@picnixz picnixz mentioned this pull request Mar 30, 2025
gh-131878: Handle top level exceptions in new pyrepl and prevent of closing it #131910
Merged
@python-cla-bot

python-cla-bot Bot commented Apr 6, 2025

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

All reactions

Sorry, something went wrong.

Uh oh!

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

@chris-eibl

chris-eibl commented Apr 14, 2025

Copy link
Copy Markdown
Member

LGTM. I cannot do a thorough review, but at least confirm, that with this PR I can enter äÄöÖüÜ, etc, again 🚀

sergey-miryanov and tomasr8 reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

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

@chris-eibl chris-eibl mentioned this pull request Apr 20, 2025
GH-132439: Fix REPL swallowing characters entered with AltGr on cmd.exe #132440
Merged
tomasr8
tomasr8 approved these changes Apr 26, 2025
View reviewed changes

@tomasr8 tomasr8 left a comment

Copy link
Copy Markdown
Member

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.

Tested with a Czech keyboard and everything works. Thanks!

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/_pyrepl/base_eventqueue.py Resolved

Uh oh!

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

Comment thread Lib/test/test_pyrepl/test_eventqueue.py Resolved

Uh oh!

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

@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Apr 26, 2025
sergey-miryanov and others added 2 commits April 26, 2025 21:31
@sergey-miryanov @tomasr8
Do not use bytearray to convert char to bytes
7fd3418
Co-authored-by: Tomas R. <[email protected]>
@sergey-miryanov
Merge branch 'main' into pythongh-131878-fix-unicode-input-in-new-pyr…
a2fb7ee
…epl-for-long-unicode-chars
chris-eibl
chris-eibl reviewed Apr 26, 2025
View reviewed changes
Comment thread Lib/_pyrepl/base_eventqueue.py Resolved

Uh oh!

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

chris-eibl
chris-eibl reviewed Apr 26, 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 commented Apr 26, 2025

Copy link
Copy Markdown
Member

LGTM. I cannot do a thorough review, but at least confirm, that with this PR I can enter äÄöÖüÜ, etc, again 🚀

Had some time now: this needs fixes for Linux. Otherwise looks good, thanks @sergey-miryanov!

All reactions

Sorry, something went wrong.

Uh oh!

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

@tomasr8

tomasr8 commented Apr 27, 2025

Copy link
Copy Markdown
Member

This breaks Linux, because only one char is inserted in UnixConsole.get_event() [...]

Since this wasn't caught by the tests, we should also probably add a test for that.

All reactions

Sorry, something went wrong.

Uh oh!

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

@chris-eibl

chris-eibl commented Apr 27, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

I have suggested the def test_push_unicode_character_two_bytes(self): which tests exactly that.

All reactions

Sorry, something went wrong.

Uh oh!

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

sergey-miryanov and others added 4 commits April 27, 2025 21:52
@sergey-miryanov @chris-eibl
Push char one by one in windows_console
b1fa88d
Co-authored-by: Chris Eibl <[email protected]>
@sergey-miryanov
Apply Chris Eibl suggestionы and use only int and bytes in eventqueue…
925f3a2
….push
@sergey-miryanov
Do not use finally to reset keymap
aa56581
@sergey-miryanov
Rename tests along Chris Eibl suggestions
1eaeca3
@sergey-miryanov

sergey-miryanov commented Apr 27, 2025

Copy link
Copy Markdown
Contributor Author

@chris-eibl Please take a look! I would like to keep tests for paste mode to be sure that escape sequences not mixed with input if error occurred. But I'm OK to remove them if you think it is not necessary.

All reactions

Sorry, something went wrong.

Uh oh!

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

@chris-eibl

chris-eibl commented Apr 28, 2025

Copy link
Copy Markdown
Member

LGTM. Sure, more tests are always better. Though, AFAIU, there is nothing special about paste mode here:

  • test_push_unicode_character_two_bytes_in_paste_mode just asserts that a valid "two byte char" within a sequence of "one byte chars" is working as expected.
  • likewise, test_push_unicode_character_as_str_in_paste_mode asserts that when an exception during a sequence of feeding bytes via push is caught, we can continue pushing afterwards.

IMHO, both are reasonable, but "paste mode" makes them look too special about pasting?

So I'd rename them and use arbitrary "one byte chars" in there.

All reactions

Sorry, something went wrong.

Uh oh!

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

@sergey-miryanov

sergey-miryanov commented Apr 28, 2025

Copy link
Copy Markdown
Contributor Author

Sequence of "one byte chars" is a control command to enable "paste mode". Originally in gh-131878 problem aroise from case where the wrongly passed unicode string puts to the buffer and mixed with following control command that disables paste mode and asserted in those code path. So, I want to check that we don't "corrupt" chars buffer.

All reactions

Sorry, something went wrong.

Uh oh!

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

@chris-eibl

chris-eibl commented Apr 28, 2025

Copy link
Copy Markdown
Member

Yupp, I know. But as said, there is nothing special about bracketed pasting here. From the perspective of the eventqueue test, these are just arbitrary bytes. Whether bracketed pasting is working correctly, is tested in

cpython/Lib/test/test_pyrepl/test_pyrepl.py

Line 1201 in 1b7470f

def test_bracketed_paste(self):

If you'd like to showcase that sequences of "one byte chars" interleaved with "multi byte chars" work correctly via "paste mode", then maybe just add a comment?

And small nit: ~ is missing in the escape sequences, see

cpython/Lib/test/test_pyrepl/test_pyrepl.py

Lines 1227 to 1228 in 1b7470f

paste_start = "\x1b[200~"
paste_end = "\x1b[201~"

To me, those two tests have merit, but are too special about "paste mode".

All reactions

Sorry, something went wrong.

Uh oh!

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

sergey-miryanov added 2 commits April 28, 2025 23:03
@sergey-miryanov
Fix tests
68d978e
@sergey-miryanov
Add comment
7f8fb7f
@sergey-miryanov

sergey-miryanov commented Apr 28, 2025

Copy link
Copy Markdown
Contributor Author

@chris-eibl It seems that I did not correctly assume how paste-mode works (shame on me!). I removed one test because the same code path is covered by test_push_unicode_character_two_bytes. And renamed and simplified a bit test_push_single_chars_and_unicode_character_as_str (not sure the name is ok though).

All reactions

Sorry, something went wrong.

Uh oh!

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

chris-eibl
chris-eibl reviewed Apr 28, 2025
View reviewed changes
Comment thread Lib/test/test_pyrepl/test_eventqueue.py Resolved

Uh oh!

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

@chris-eibl

chris-eibl commented Apr 28, 2025

Copy link
Copy Markdown
Member

I don't have a better name - the comment in there clarifies what we intend to test (not much tbh).
Just a small nit on the comment.

Otherwise LGTM. Thanks for bearing with me!

All reactions

Sorry, something went wrong.

Uh oh!

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

@sergey-miryanov @chris-eibl
Update comments in the test
9519208
Co-authored-by: Chris Eibl <[email protected]>
@sergey-miryanov

sergey-miryanov commented Apr 29, 2025

Copy link
Copy Markdown
Contributor Author

@chris-eibl Thanks for the review and patience!

All reactions

Sorry, something went wrong.

Uh oh!

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

chris-eibl
chris-eibl approved these changes Apr 29, 2025
View reviewed changes

@chris-eibl chris-eibl left a comment

Copy link
Copy Markdown
Member

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.

Thank you @sergey-miryanov!

Sorry, something went wrong.

Uh oh!

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

All reactions
tomasr8
tomasr8 reviewed May 4, 2025
View reviewed changes
Comment thread Lib/_pyrepl/base_eventqueue.py Resolved

Uh oh!

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

tomasr8
tomasr8 approved these changes May 4, 2025
View reviewed changes

@tomasr8 tomasr8 left a comment

Copy link
Copy Markdown
Member

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.

Tested again after the simplifications. WFM on both Windows and Linux :)

Sorry, something went wrong.

Uh oh!

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

All reactions
@sergey-miryanov @tomasr8
Use to_bytes
a855ba8
Co-authored-by: Tomas R. <[email protected]>
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.

Gentlemen, excellent work. Not only does it fix the bug, but it makes the codebase simpler and more elegant. If not for the new test, the net number of lines added here would be negative. Impressive!

Sorry, something went wrong.

Uh oh!

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

hugovk reacted with hooray emoji chris-eibl, sergey-miryanov, and hugovk reacted with heart emoji hugovk reacted with rocket emoji
All reactions
  • 🎉 1 reaction
  • ❤️ 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 0c5151b into python:main May 5, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label May 5, 2025
@ambv ambv added the needs backport to 3.13 only security fixes label May 5, 2025
@miss-islington-app

miss-islington-app Bot commented May 5, 2025

Copy link
Copy Markdown

Thanks @sergey-miryanov 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.

@miss-islington-app

miss-islington-app Bot commented May 5, 2025

Copy link
Copy Markdown

Sorry, @sergey-miryanov 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 0c5151bc81ec8e8588bef4389df12a9ab50e9fa0 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
@sergey-miryanov

sergey-miryanov commented May 5, 2025

Copy link
Copy Markdown
Contributor Author

@ambv Please take a look - this is the same problem with backport as in earlier PR - #130805 (comment)

All 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
@sergey-miryanov @tomasr8
@chris-eibl @ambv
[3.13] pythongh-131878: Fix input of unicode characters with two or m…
1fcec66
…ore code points in new pyrepl on Windows (pythongh-131901)

(cherry picked from commit 0c5151b)

Co-authored-by: Sergey Miryanov <[email protected]>
Co-authored-by: Tomas R. <[email protected]>
Co-authored-by: Chris Eibl <[email protected]>
@bedevere-app

bedevere-app Bot commented May 5, 2025

Copy link
Copy Markdown

GH-133468 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 @sergey-miryanov
@tomasr8 @chris-eibl
[3.13] gh-131878: Fix input of unicode characters with two or more co…
891232f
…de points in new pyrepl on Windows (gh-131901) (gh-133468)

(cherry picked from commit 0c5151b)

Co-authored-by: Sergey Miryanov <[email protected]>
Co-authored-by: Tomas R. <[email protected]>
Co-authored-by: Chris Eibl <[email protected]>
@bedevere-bot

bedevere-bot commented May 5, 2025

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian root 3.13 (tier-1) has failed when building commit 891232f.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1441/builds/1169) and take a look at the build logs.
  4. Check if the failure is related to this commit (891232f) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1441/builds/1169

Summary of the results of the build (if available):

Click to see traceback logs
remote: Enumerating objects: 26, done.        
remote: Counting objects:   3% (1/26)        
remote: Counting objects:   7% (2/26)        
remote: Counting objects:  11% (3/26)        
remote: Counting objects:  15% (4/26)        
remote: Counting objects:  19% (5/26)        
remote: Counting objects:  23% (6/26)        
remote: Counting objects:  26% (7/26)        
remote: Counting objects:  30% (8/26)        
remote: Counting objects:  34% (9/26)        
remote: Counting objects:  38% (10/26)        
remote: Counting objects:  42% (11/26)        
remote: Counting objects:  46% (12/26)        
remote: Counting objects:  50% (13/26)        
remote: Counting objects:  53% (14/26)        
remote: Counting objects:  57% (15/26)        
remote: Counting objects:  61% (16/26)        
remote: Counting objects:  65% (17/26)        
remote: Counting objects:  69% (18/26)        
remote: Counting objects:  73% (19/26)        
remote: Counting objects:  76% (20/26)        
remote: Counting objects:  80% (21/26)        
remote: Counting objects:  84% (22/26)        
remote: Counting objects:  88% (23/26)        
remote: Counting objects:  92% (24/26)        
remote: Counting objects:  96% (25/26)        
remote: Counting objects: 100% (26/26)        
remote: Counting objects: 100% (26/26), done.        
remote: Compressing objects:  33% (1/3)        
remote: Compressing objects:  66% (2/3)        
remote: Compressing objects: 100% (3/3)        
remote: Compressing objects: 100% (3/3), done.        
remote: Total 14 (delta 12), reused 12 (delta 11), pack-reused 0 (from 0)        
From https://github.com/python/cpython
 * branch                    3.13       -> FETCH_HEAD
Note: switching to '891232f3386dd8b20a216a473954c1b01cede7ec'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 891232f3386 [3.13] gh-131878: Fix input of unicode characters with two or more code points in new pyrepl on Windows (gh-131901) (gh-133468)
Switched to and reset branch '3.13'

configure: WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)
configure: WARNING: pkg-config is missing. Some dependencies may not be detected correctly.

find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
make: [Makefile:3116: clean-retain-profile] Error 1 (ignored)
All reactions

Sorry, something went wrong.

Uh oh!

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

@chris-eibl chris-eibl mentioned this pull request May 6, 2025
gh-131878: Fix input of unicode characters with two or more code points in the REPL on Windows in vt mode #133030
Closed
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@sergey-miryanov @tomasr8
@chris-eibl @Pranjal095
pythongh-131878: Fix input of unicode characters with two or more cod…
ac6b151
…e points in new pyrepl on Windows (pythongh-131901)

Co-authored-by: Tomas R. <[email protected]>
Co-authored-by: Chris Eibl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chris-eibl chris-eibl chris-eibl approved these changes
@ambv ambv ambv approved these changes
@tomasr8 tomasr8 tomasr8 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

Uh oh!

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

Assignees

@ambv ambv

Labels

None yet

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.

5 participants

@sergey-miryanov @chris-eibl @tomasr8 @bedevere-bot @ambv
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 01402503 — 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.