-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
VT sequence support for EraseInLine, EraseInDisplay, DeleteCharacter and InsertCharacter #2144
Conversation
nit: title. rephrase to be something imperative, as though you were completing the sentence:
|
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.
I would love to see some tests added for these guys, but I understand that there's probably not time to add them now.
I've got a couple nit-level questions I want answered, but otherwise great work!
…ddressed reviewer comments
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.
Alright I'm happy with this
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.
🚀
while (fDoBackUp) | ||
{ | ||
coordEndOfText.Y--; | ||
pCurrRow = &GetRowByOffset(coordEndOfText.Y); | ||
// We need to back up to the previous row if this line is empty, AND there are more rows | ||
|
||
coordEndOfText.X = static_cast<short>(pCurrRow->GetCharRow().MeasureRight()) - 1; | ||
fDoBackUp = (coordEndOfText.X < 0 && coordEndOfText.Y > 0); | ||
fDoBackUp = (coordEndOfText.X < 0 && coordEndOfText.Y > viewportTop); |
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.
Technically, should this make sure that X
is within viewportRight
? Or something equivalent?
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.
It seems like X will be -1 if there was no non-space text in the row (based on what MeasureRight) is doing, so I don't think its relative to where the viewport is
Summary of the Pull Request
Implemented the following VT sequences:
(Minor: implemented CursorUp and CursorBackward in TerminalDispatch with existing APIs)
References
Related to #1883
PR Checklist
Detailed Description of the Pull Request / Additional comments
Without these VT sequences, non-Conhost connections behave strangely (since they do not go through conpty for VT parsing).
Validation Steps Performed
Compared behaviour with putty/other terminals and imitated them