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-128067: Fix pyrepl overriding printed output without newlines#138732

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 10 commits intopython:mainfromJanEricNitschke:fix-pyrepl
Jan 2, 2026

Conversation

@JanEricNitschke
Copy link
Contributor

@JanEricNitschkeJanEricNitschke commentedSep 10, 2025•
edited by bedevere-appbot
Loading

I tried a bit to debug this and narrowed it down to these two lines for windows and unix terminals respectively:

Because while properly running the repl in unix doesnt cause the issue, running the test i added also fails there before these changes.

I am not 100% sure if this is the base and 100% correct way, but it causes the test to pass on unix and fixes the issue on windows for me.

I also attempted to try to be able to run these tests for windows (because thats where we actually saw the issue) but didnt manage unfortunately.

PyReplFixWSLPyReplFixWindows

With the fix in the unix console:

 python -m coverage run --pylib --branch --source=pyrepl Lib/test/regrtest.py  test_pyrepl -m test_no_newlineUsing random seed: 41267882060:00:00 load avg: 0.17 Run 1 test sequentially in a single process0:00:00 load avg: 0.17 [1/1] test_pyrepl0:00:04 load avg: 0.17 [1/1] test_pyrepl passed== Tests result: SUCCESS ==1 test OK.Total duration: 4.7 secTotal tests: run=1 (filtered)Total test files: run=1/1 (filtered)Result: SUCCESS

Without:

 python -m coverage run --pylib --branch --source=pyrepl Lib/test/regrtest.py  test_pyrepl -m test_no_newlineUsing random seed: 38325641220:00:00 load avg: 0.30 Run 1 test sequentially in a single process0:00:00 load avg: 0.30 [1/1] test_pyrepltest test_pyrepl failed -- Traceback (most recent call last):  File "/mnt/d/Programming/Projects/cpython/Lib/test/support/__init__.py", line 2923, in wrapper    return func(*args, **kwargs)  File "/mnt/d/Programming/Projects/cpython/Lib/test/test_pyrepl/test_pyrepl.py", line 1819, in test_no_newline    self.assertIn(expected_output_sequence, cleaned_output)    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^AssertionError: 'Something pretty long>>> exit()' not found in 'print(\'Something pretty long\', end=\'\')\r\nexit()\r\nPython 3.15.0a0 (heads/main:c919d02ede, Sep  6 2025, 15:55:27) [GCC 13.3.0] on linux\r\nType "help", "copyright", "credits" or "license" for more information.\r\n\x1b[1A\n>>> print(\'Something pretty long\', end=\'\')\x1b[42D\n\rSomething pretty long\x1b[1A\n>>> exit()\x1b[10D\n\r'0:00:02 load avg: 0.30 [1/1/1] test_pyrepl failed (1 failure)== Tests result: FAILURE ==1 test failed:    test_pyreplTotal duration: 2.0 secTotal tests: run=1 (filtered) failures=1Total test files: run=1/1 (filtered) failed=1Result: FAILURE

@bedevere-app
Copy link

Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply theskip news label instead.

@bedevere-app
Copy link

Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply theskip news label instead.

@bedevere-app
Copy link

Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply theskip news label instead.

@chris-eibl
Copy link
Member

As written in the issue, this works fine on

  • Windows 10 (in a legacy console and the new Windows terminal )
  • WSL Ubuntu-24.04 (again in a legacy console and the new Windows terminal)
  • native Ubuntu 24.0.4.3 LTS

self._move_relative(0, len(self.screen) - 1)
self.__write("\n")
if self.screen:
self._move_relative(0, len(self.screen) - 1)
Copy link
Member

Choose a reason for hiding this comment

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

Spot on,len(self.screen) - 1 should not get negative here.

@chris-eibl
Copy link
Member

Please add a News entry, seeQuick reference #8 in the devguide.

@chris-eibl
Copy link
Member

Please merge with latest default, because the pyrepl Windows tests are not run otherwise (#138801).

@JanEricNitschke
Copy link
ContributorAuthor

Please merge with latest default, because the pyrepl Windows tests are not run otherwise (#138801).

So merge with current main and "merge" not "rebase", right? So basically just clicking the "Update branch" button here.

chris-eibl reacted with thumbs up emoji

@chris-eibl
Copy link
Member

I suggest to add tests like

deftest_no_newline(self,_os_write):code="1"events=code_to_events(code)_,con=handle_events_unix_console(events)self.assertNotIn(call(ANY,b'\n'),_os_write.mock_calls)con.restore()deftest_newline(self,_os_write):code="\n"events=code_to_events(code)_,con=handle_events_unix_console(events)_os_write.assert_any_call(ANY,b"\n")con.restore()

intest_unix_console.py andtest_windows_console.py. Otherwise, for Windows we won't have any test coverage.

I think they are best placed beforetest_simple_addition?

Both tests fail before the PR and succeed with the PR.

So these tests might be enough, but of course your current test is more explict. But since Windows doesn't have thepty (Pseudo-terminal utilities), I don't know how to do a similar test there.

The filtering of the many escape sequences might induce some maintenance burden, but I don't think it is a big issue.

@chris-eibl
Copy link
Member

Please merge with latest default, because the pyrepl Windows tests are not run otherwise (#138801).

So merge with current main and "merge" not "rebase", right? So basically just clicking the "Update branch" button here.

Correct. For reviewers it is easier to follow without squashing.

For the final merge from your fork to the upstream branch, squashing is used, anyway.

@JanEricNitschke
Copy link
ContributorAuthor

Added the tests. I put the news entry under Windows because the main user visible effect here is on windows, although we did make changes to both unix and windows consoles.

chris-eibl reacted with thumbs up emojichris-eibl reacted with heart emoji

Copy link
Member

@chris-eiblchris-eibl left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you@JanEricNitschke.

@chris-eibl
Copy link
Member

I just found out, that inclear we can now change from


to

like inunix_console.py (and like it is initialized inprepare in both implementations).

@JanEricNitschke
Copy link
ContributorAuthor

JanEricNitschke commentedSep 14, 2025•
edited
Loading

Changed this for the windows console to be the same as for the unix one.

chris-eibl reacted with heart emoji

@JanEricNitschke
Copy link
ContributorAuthor

Added myself to ACKS after the review/approval, hope thats alright.

chris-eibl reacted with thumbs up emoji

self.__write(CLEAR)
self.posxy = 0, 0
self.screen = [""]
self.screen = []
Copy link
Contributor

Choose a reason for hiding this comment

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

The "" here before was a clear attempt at working around the same issue that you're solving here, but I agree theif statement is cleaner and more explicit.

Inunix_console.pyclear() is settingself.screen to an empty list, but as you noticed this wasn't triggering any bug because the__gone_tall boolean would prevent this path being taken inrefresh(). In unit tests that wasn't the case and so they were failing before your fix.

I'm okay with being defensive here, especially that this makes the behavior symmetrical between Windows and Unix.

@ambv
Copy link
Contributor

ambv commentedJan 2, 2026

Sorry this took me so long to get to. I had to understand what's going on here before pushing the green button. Confirmed this is the correct fix and tested it resolves the issue on Windows 11 while maintaining behavior on macOS and Ubuntu 22.04 LTS. Thank you so much for your work,@JanEricNitschke and@chris-eibl for your careful review.

chris-eibl and JanEricNitschke reacted with hooray emoji

@ambvambv added needs backport to 3.13bugs and security fixes needs backport to 3.14bugs and security fixes labelsJan 2, 2026
@ambvambv merged commit8a2deea intopython:mainJan 2, 2026
58 checks passed
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestJan 2, 2026
pythonGH-138732)(cherry picked from commit8a2deea)Co-authored-by: Jan-Eric Nitschke <47750513+JanEricNitschke@users.noreply.github.com>Co-authored-by: Łukasz Langa <lukasz@langa.pl>
@bedevere-app
Copy link

GH-143350 is a backport of this pull request to the3.14 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestJan 2, 2026
pythonGH-138732)(cherry picked from commit8a2deea)Co-authored-by: Jan-Eric Nitschke <47750513+JanEricNitschke@users.noreply.github.com>Co-authored-by: Łukasz Langa <lukasz@langa.pl>
@bedevere-appbedevere-appbot removed the needs backport to 3.14bugs and security fixes labelJan 2, 2026
@bedevere-app
Copy link

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

@bedevere-appbedevere-appbot removed the needs backport to 3.13bugs and security fixes labelJan 2, 2026
@bedevere-bot
Copy link

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

Hi! The buildbotAMD64 CentOS9 NoGIL 3.x (tier-1) has failed when building commit8a2deea.

What do you need to do:

  1. Don't panic.
  2. Checkthe 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/1609/builds/4528) and take a look at the build logs.
  4. Check if the failure is related to this commit (8a2deea) 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/1609/builds/4528

Failed tests:

  • test_pyrepl

Failed subtests:

  • test_no_newline - test.test_pyrepl.test_pyrepl.TestMain.test_no_newline

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

==

Click to see traceback logs
Traceback (most recent call last):  File"/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/Lib/test/support/__init__.py", line3022, inwrapperreturn func(*args,**kwargs)  File"/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/Lib/test/test_pyrepl/test_pyrepl.py", line1884, intest_no_newlineself.assertIn(expected_output_sequence, cleaned_output)~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^AssertionError:'Something pretty long>>> exit()' not found in 'print(\'Something pretty long\', end=\'\')\r\nexit()\r\nPython 3.15.0a3+ free-threading build (heads/main:8a2deea1fc7, Jan  2 2026, 13:14:38) [GCC 11.5.0 20240719 (Red Hat 11.5.0-14)] on linux\r\nType "help", "copyright", "credits" or "license" for more information.\r\n>>> \x1b[34hprint(\'Something pretty long\', end=\'\')\x1b[42D\n\rSomething pretty long>>> \x1b[34hexit()\x1b[10D\n\r'

@ambv
Copy link
Contributor

ambv commentedJan 2, 2026

The buildbot failure is related. I'm on it.

JanEricNitschke reacted with heart emoji

ambv added a commit that referenced this pull requestJan 2, 2026
…es (GH-138732) (GH-143350)(cherry picked from commit8a2deea)Co-authored-by: Jan-Eric Nitschke <47750513+JanEricNitschke@users.noreply.github.com>Co-authored-by: Łukasz Langa <lukasz@langa.pl>
ambv added a commit that referenced this pull requestJan 2, 2026
…es (GH-138732) (GH-143351)(cherry picked from commit8a2deea)Co-authored-by: Jan-Eric Nitschke <47750513+JanEricNitschke@users.noreply.github.com>Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@ambvambvambv left review comments

@chris-eiblchris-eiblchris-eibl approved these changes

@pablogsalpablogsalAwaiting requested review from pablogsalpablogsal is a code owner

@lysnikolaoulysnikolaouAwaiting requested review from lysnikolaoulysnikolaou is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JanEricNitschke@chris-eibl@ambv@bedevere-bot
[8]ページ先頭
©2009-2026 Movatter.jp