Cursor jumps to next line after running command same width as window. #17013
Labels
Area-Output
Related to output processing (inserting text into buffer, retrieving buffer text, etc.)
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Needs-Tag-Fix
Doesn't match tag requirements
Priority-3
A description (P3)
Product-Conpty
For console issues specifically related to conpty
Milestone
Comments
e82eric
added
Issue-Bug
|
This is a conpty issue - it doesn't occur in conhost/openconsole. What's meant to be happening is the cursor should move to the next line when you type a character on the last column of the page. Powershell actually makes a I think it may have to do with the vtengine's understanding of the terminal/src/renderer/base/renderer.cpp Lines 746 to 751 in 67ae9f6 Which in turn sets the terminal/src/renderer/vt/paint.cpp Lines 611 to 621 in 67ae9f6 Which concludes with us determining that the line was already wrapped by the text output, so there is no need to move the cursor. terminal/src/renderer/vt/XtermEngine.cpp Lines 260 to 266 in 67ae9f6 But in this case the conpty client hasn't wrapped, because of the VT delayed wrap functionality, so we do need to emit something to force it onto the next line. This problem can be fixed just by removing the above code, but I'm not sure if there are legitimate cases where we actually do need to avoid a cursor movement here. However, I think it's possible that this code may have been mistakenly introduced to compensate for the fact that the original Windows Terminal VT interpreter didn't handle delayed wrap correctly. Since that's been fixed, this may no longer be necessary. I'm not positive about that though. |
|
@e82eric I forgot to add that the code you pointed out is also incorrect, and commenting it out will solve the problem in your particular case (i.e. when using powershell), but the underlying bug in the vtengine can still be triggered in other ways. For example, the cmd shell uses the legacy console API to write its output, and that doesn't pass through |
|
@j4james thank you! that makes sense. Feel free to close this issue. (I had noticed it and wanted to see if I could sort out what was going on). I tried commenting this out, and it did fix the issue, it does look like it has a side affect where when the window is resized the cursor moved and another line was added (I guess that is why those lines are needed). terminal/src/renderer/vt/XtermEngine.cpp Lines 260 to 266 in 67ae9f6
|
I think it may be worth keeping this open, because there's definitely a bug here which is independent of issue #15602, and I don't think it's tracked anywhere else.
I haven't checked what exactly is going on there, but it's possible that's just a bug in the powershell readline implementation, and it only appears to be working correctly now because of the conpty bug. |
|
@j4james sounds good. I ran a time travel trace to see if I could find anything on why it did the weird jump with those lines commented out. It looked like it called Cursor::SetPosition twice. Once after Terminal::Reflow to x:12, y:5 and once from VirtualTerminal::StateMachine::ProcessString x:43 y:4 from .[m.[2;44H (I think windbg mangled the .[m. part). Maybe a combination of Terminal::Reflow moving to line 3 and then something with PSReadLine moving it back to the correct line with a weird x offset. |
carlos-zamora
added
Product-Conpty
This was referenced May 25, 2024
github-merge-queue bot
pushed a commit
that referenced
this issue
May 30, 2024
## 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 `PaintCursor` call 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 `_wrappedRow` field before the `_MoveCursor` call in the `PaintCursor` method. ## 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 `ConptyRoundtripTests` that were expecting the terminal row to be marked as wrapped when writing a wrapped line in two parts using `WriteCharsLegacy`. This is because the legacy 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 - [x] Closes #17013 - [x] Closes #17270 - [x] Tests added/passed
microsoft-github-policy-service
bot
added
the
Needs-Tag-Fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Windows Terminal version
67ae9f6
Windows build number
No response
Other Software
No response
Steps to reproduce
When a command that is the same length as the window is run, it causes the cursor to jump to the next line 2 characters into typing the next command.
Commenting out these 4 line seemed to fix the issue.
terminal/src/terminal/adapter/adaptDispatch.cpp
Lines 144 to 149 in 67ae9f6
Expected Behavior
The cursor not to jump to the next line while typing the next command.
Actual Behavior
The cursor jumped to the next line while typing the next command
The text was updated successfully, but these errors were encountered: