Position the conpty cursor correctly when wrappedRow is set #17290
Conversation
|
In an ideal world the wrapping code needs a major rewrite, so if that's something someone is already planning to do, you may want to ignore this and wait for a proper fix. But if you want a quick resolution to #17270, this may be the least worst option for now. And I think having the cursor correctly positioned is way more important than regressing a few reflow wrapping cases that were never fully working to begin with. |
|
Before I say anything else: This is great! I think we should merge this because alternatives won't arrive any time soon.
FYI coincidentally I did start writing a ConPTY variant today that does straight conversions to VT and has no VtEngine at all. Basic usage of pwsh and wsl already works. I believe the main complexity of the work will be the rewrite of cooked read in terms of VT sequences without direct |
Summary of the Pull Request
If the VT render engine is moving the cursor to the start of a row, and
the previous row was marked as wrapped, it will assume that it doesn't
need to do anything, because the next output should automatically move
the cursor to the correct position anyway.
However, if that cursor movement is coming from the final
PaintCursorcall for the frame, there isn't going to be any more output, so the
cursor will be left in the wrong position.
This PR fixes that issue by clearing the
_wrappedRowfield before the_MoveCursorcall in thePaintCursormethod.Validation Steps Performed
I've confirmed that this fixes all the test cases mentioned in issue
#17270, and issue #17013, and I've added a unit test to check the new
behavior is working as expected.
However, this change does break a couple of
ConptyRoundtripTeststhatwere expecting the terminal row to be marked as wrapped when writing a
wrapped line in two parts using
WriteCharsLegacy. This is because thelegacy way of wrapping a line isn't the same as a VT delayed wrap, so it
has to be emulated with cursor movement, and that can end up resetting
the wrap flag.
It's possible that could be fixed, but it's already broken in a number
of other ways, so I don't think this makes things much worse. For now,
I've just made the affected test cases skip the wrapping check.
PR Checklist