Skip to content
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

wsl vim is horribly broken after #5181 #5291

Closed
DHowett-MSFT opened this issue Apr 9, 2020 · 2 comments · Fixed by #5294
Closed

wsl vim is horribly broken after #5181 #5291

DHowett-MSFT opened this issue Apr 9, 2020 · 2 comments · Fixed by #5294
Assignees
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. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Conpty For console issues specifically related to conpty Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Blocking We won't ship a release like this! No-siree.
Milestone

Comments

@DHowett-MSFT
Copy link
Contributor

image

tap output:

~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K~␛[K

no cursor movement after the EL :/

@DHowett-MSFT DHowett-MSFT added Product-Conpty For console issues specifically related to conpty 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. Severity-Blocking We won't ship a release like this! No-siree. Priority-0 Bugs that we consider release-blocking/recall-class (P0) labels Apr 9, 2020
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.0 milestone Apr 9, 2020
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Apr 9, 2020
@zadjii-msft zadjii-msft self-assigned this Apr 9, 2020
@zadjii-msft
Copy link
Member

looking now

@ghost ghost added the In-PR This issue has a related PR label Apr 9, 2020
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Apr 9, 2020
@ghost ghost closed this as completed in #5294 Apr 15, 2020
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Apr 15, 2020
ghost pushed a commit that referenced this issue Apr 15, 2020
## Summary of the Pull Request

When WSL vim prints the initial empty buffer (the one that's just a bunch of '\~'s), it prints this by doing the following:
* Print '\~' followed by enough spaces to clear the line
* Use CUP (`^[[H`) to move the cursor to the start of the next line
* repeat until the buffer is full

When we'd get the line of "\~     "... in conhost, we'd mark that line as wrapped. 

Logically, it doesn't really make any sense that when we follow that up by moving the cursor, the line is wrapped. However, this is just how conhost is right now. 
This wasn't ever a problem in just conhost before, because we really didn't care if lines in the alt buffer were "wrapped" or not. Plus, when vim would get resized, it would just reprint it's own buffer anyways. Nor was this a problem in conpty before this year (2020). We've only just recently added logic to conpty to try and preserve wrapped lines. 

Initially, I tried fixing this by breaking the line manually when the cursor was moved. This seemed to work great, except for the win32 vim.exe. Vim.exe doesn't emit a newline or a CUP to get to the next line. It just _goes for it_ and keeps printing. So there's _no way_ for us to know the line broke, because they're essentially just printing one long line, assuming we'll automatically move the cursor.

So instead, I'm making sure to emit the proper number of spaces at the end of a line when the line is wrapped. We won't do any funny business in that scenario and try to optimize for them, we'll _just print the spaces_.

## References

* #5181 - This change regressed this
* #4415 - Actually implemented wrapped lines in conpty

## PR Checklist
* [x] Closes #5291
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated

## Validation Steps Performed
* Wrote a unittest first and foremost
* Checked vtpipeterm to make sure vim still works
* checked Terminal to make sure vim still works
@ghost
Copy link

ghost commented Apr 22, 2020

🎉This issue was addressed in #5294, which has now been successfully released as Windows Terminal Preview v0.11.1121.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Conpty For console issues specifically related to conpty Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants