Skip to content

Conversation

@hyongtao-code
Copy link
Contributor

@hyongtao-code hyongtao-code commented Jan 28, 2026

Long log:

Windows VT terminals do not consistently wrap the cursor when a line
exactly fills the terminal width.
Previously we assumed a wrap always happened, which could desynchronize
the logical cursor from the real console cursor and break subsequent
cursor movement.
This change queries the real cursor position and updates posxy
accordingly, and adds regression tests for both wrap and no-wrap cases.

Video with the patch

fix-issue-144259.mp4

Signed-off-by: Yongtao Huang yongtaoh2022@gmail.com

Windows VT terminals do not consistently wrap the cursor when a line
exactly fills the terminal width.
Previously we assumed a wrap always happened, which could desynchronize
the logical cursor from the real console cursor and break subsequent
cursor movement.
This change queries the real cursor position and updates posxy
accordingly, and adds regression tests for both wrap and no-wrap cases.

Signed-off-by: Yongtao Huang <yongtaoh2022@gmail.com>
Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
@hyongtao-code hyongtao-code changed the title gh-144259: Fix Windows VT EOL wrap by syncing real console cursor gh-144259: Fix Windows EOL wrap by syncing real console cursor Feb 1, 2026
@hyongtao-code
Copy link
Contributor Author

Thanks for the review! I agree with your point, so I’ve removed the VT-specific references and updated the tests to exercise both VT and legacy console modes.

Fall back gracefully when querying the console cursor fails with
an invalid handle, instead of raising an exception.

Failed link: https://github.com/python/cpython/actions/runs/21563046527/job/62130029686?pr=144297
@chris-eibl
Copy link
Member

As you've seen, many Windows tests fail now, because in headless mode GetConsoleScreenBufferInfo() fails with The handle is invalid.

Instead of swallowing the error, maybe introduce something like

_HasWrappedToNextRow(self, y)

Then, we can simply mock this for most of the tests.

And you can also write an explicit test for _HasWrappedToNextRow by mocking the "bare" GetConsoleScreenBufferInfo() just for this one test?

@hyongtao-code
Copy link
Contributor Author

Thanks for the suggestion — these failed cases were indeed quite tricky. I decided to follow your proposed approach to fix the failing cases.

Comment on lines 260 to 264
if not GetConsoleScreenBufferInfo(OutHandle, info):
err = get_last_error()
if err == 6: # ERROR_INVALID_HANDLE
return None
raise WinError(err)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if not GetConsoleScreenBufferInfo(OutHandle, info):
err = get_last_error()
if err == 6: # ERROR_INVALID_HANDLE
return None
raise WinError(err)
if not GetConsoleScreenBufferInfo(OutHandle, info):
raise WinError(get_last_error())

Let's raise again? We can handle failing CI like for

console._getscrollbacksize = MagicMock(42)

by adding

console._has_wrapped_to_next_row = MagicMock(False)

That's what I had in mind in #144297 (comment)

Instead of swallowing the error [...] we can simply mock this for most of the tests.

In case you like this suggestion, please also adapt the annotation and docstring. test_returns_none_on_invalid_handle and test_raises_on_unexpected_error are then no longer needed.

Thanks for bearing with me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem at all, thanks for the clarification and your help — I’ll keep working on these over the next day or two.

hyongtao-code and others added 2 commits February 2, 2026 00:40
Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants