-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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 Terminal Line Endings (the "exact wrap" bug) #3088
Comments
I discovered this yesterday and wanted to report today, LOL. There's more to the story. First, let's note that most shells redraw their prompt after a resize, potentially overwriting some of the previously printed Then, let's observe that if you have a shorter line than the width, e.g. shorter by one:
(followed by some other lines), and then you narrow the window, this line wraps one step too early (e.g. in this case immediately as you shrink the window by 1), introducing an unnecessary empty line below (which in turn copy-pastes as tons of spaces). Another thing I noticed is that double clicking stops the selection at the current physical row boundary, rather than selecting an entire word. I haven't dug into the code yet, but I have a guts feeling that we might be seeing a conceptionally unusual and problematic handling (or rather: lack thereof) of newlines, rather than a simple off-by-one. I have the feeling (and apologies if I'm wrong, I'm planning to explore the code soon) that it's not remembered whether a row ends in a newline or not; instead, the presence or absence of a trailing speical character ("unused space" or so), which occupies its own cell, is used for that. Or something similar. What most terminals do, and I believe is the right thing, is that they remember for each physical row whether it ended in an explicit newline or an overflow. This bit belongs to the row itself, rather than a particular cell thereof, and is obviously not directly visible (you could somehow display it in some debug mode). This information is used for multiple things:
Probably there's no official specification anywhere how exactly the line ending status needs to be set. It's mostly common sense, with the only nontrivial bit: a NL character does not set the line to end in an explicit newline, it leaves its state unchanged. My BiDi proposal also provides an overview of this, and then refines the behavior for some special cases. I think that going with this traditional, well proven behavior of tracking newlines is the key to address multiple issues, e.g. rewrapping's dropping of a newline and off-by-one as reported here, proper spaces vs. newlines at copy-pasting (e.g. #2944), proper semantical behavior on double click, and to prepare for other cool features WT is about to have. |
Just dropping in to say: yeah, we don't do wrapping correctly in Windows Terminal. Since it's a client of conhost, and conhost does wrapping somewhat okay, WT isn't yet enlightened here. There's a lot of intricacies in the whole ConPTY layer -- it should almost certainly un-wrap wrapped lines before it sends them, and let the terminal emulator on the other end wrap them, but we can't do that until #780... WT won't even wrap right now (evidenced by #2588 #3094). Curious thing about newlines in the traditional console is that using Also of interest: #2089 (related to how we measure the righthand side of the screen). |
For pseudo terminal (PTY) such as screen pseudo terminal slave (PTS) works properly. |
Incidentally, this is probably the same root cause as the issue with powershell where when you type off the edge of the screen it wraps one line early but the cursor jumps to the wrong place, and then when you backspace over it it destroys the known universe. |
On a new Windows build 18363.592 this issue got even weirder after increasing size of the terminal. Per every additional column of the terminal triggers duplication of the next line. I tried even explicit escaping, but without any differences. |
## Summary of the Pull Request Changes how conpty emits text to preserve line-wrap state, and additionally adds rudimentary support to the Windows Terminal for wrapped lines. ## References * Does _not_ fix (!) #3088, but that might be lower down in conhost. This makes wt behave like conhost, so at least there's that * Still needs a proper deferred EOL wrap implementation in #780, which is left as a todo * #4200 is the mega bucket with all this work * MSFT:16485846 was the first attempt at this task, which caused the regression MSFT:18123777 so we backed it out. * #4403 - I made sure this worked with that PR before I even sent #4403 ## PR Checklist * [x] Closes #405 * [x] Closes #3367 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments I started with the following implementation: When conpty is about to write the last column, note that we wrapped this line here. If the next character the vt renderer is told to paint get is supposed to be at the start of the following line, then we know that the previous line had wrapped, so we _won't_ emit the usual `\r\n` here, and we'll just continue emitting text. However, this isn't _exactly_ right - if someone fills the row _exactly_ with text, the information that's available to the vt renderer isn't enough to know for sure if this line broke or not. It is possible for the client to write a full line of text, with a `\n` at the end, to manually break the line. So, I had to also add the `lineWrapped` param to the `IRenderEngine` interface, which is about half the files in this changelist. ## Validation Steps Performed * Ran tests * Checked how the Windows Terminal behaves with these changes * Made sure that conhost/inception and gnome-terminal both act as you'd expect with wrapped lines from conpty
Subjectively speaking, this commit makes 3 improvements: * Most importantly, it now would work with arbitrary Unicode text. (No more `IsGlyphFullWidth` or DBCS handling during reflow.) * Due to the simpler implementation it hopefully makes review of future changes and maintenance simpler. (~3x less LOC.) * It improves perf. by 1-2 orders of magnitude. (At 120x9001 with a full buffer I get 60ms -> 2ms.) Unfortunately, I'm not confident that the new code replicates the old code exactly, because I failed to understand it. During development I simply tried to match its behavior with what I think reflow should do. Closes #797 Closes #3088 Closes #4968 Closes #6546 Closes #6901 Closes #15964 Closes MSFT:19446208 Related to #5800 and #8000 ## Validation Steps Performed * Unit tests ✅ * Feature tests ✅ * Reflow with a scrollback ✅ * Reflowing the cursor cell causes a forced line-wrap ✅ (Even at the end of the buffer. ✅) * `color 8f` and reflowing retains the background color ✅ * Enter alt buffer, Resize window, Exit alt buffer ✅
Subjectively speaking, this commit makes 3 improvements: * Most importantly, it now would work with arbitrary Unicode text. (No more `IsGlyphFullWidth` or DBCS handling during reflow.) * Due to the simpler implementation it hopefully makes review of future changes and maintenance simpler. (~3x less LOC.) * It improves perf. by 1-2 orders of magnitude. (At 120x9001 with a full buffer I get 60ms -> 2ms.) Unfortunately, I'm not confident that the new code replicates the old code exactly, because I failed to understand it. During development I simply tried to match its behavior with what I think reflow should do. Closes #797 Closes #3088 Closes #4968 Closes #6546 Closes #6901 Closes #15964 Closes MSFT:19446208 Related to #5800 and #8000 ## Validation Steps Performed * Unit tests ✅ * Feature tests ✅ * Reflow with a scrollback ✅ * Reflowing the cursor cell causes a forced line-wrap ✅ (Even at the end of the buffer. ✅) * `color 8f` and reflowing retains the background color ✅ * Enter alt buffer, Resize window, Exit alt buffer ✅ (cherry picked from commit 7474839) Service-Card-Id: 90642727 Service-Version: 1.19
Put into terminal and then increase the window size:
printf "%*s\n" "$(tput cols)" "" | tr " " "X"
Line endings acts like they disappeared if the string length matches exactly the window columns.
The terminal has different behavior for columns +- 1.
The behavior does not depend on the type of shell.
The text was updated successfully, but these errors were encountered: