Text Wrapping with ConPTY #405
Comments
|
I'm sure there's another issue tracking this lying around somewhere... There was actually a prototype of doing this for a few insider's builds but that caused some other regressions in behavior late in RS5. (For internal reference, see task 16485846 and bug 18123777). This is definitely something we need to figure out how to get right. |
zadjii-msft
added
Product-Conpty
|
Thanks for the notes! I did have a search for an existing issue before opening though didn't spot one. Please do close this issue if a duplicate turns up! |
|
Interesting. This feature request dovetails nicely with my recent remarks on MSBuild supporting colors but intermediary programs like PowerShell blocking colors: dotnet/msbuild#4299 I am still wrapping my head around all of this, so apologies to those who don't see the same connection I see, which is that API consumers don't control output, API providers do. I think this is a fundamental problem. |
miniksa
added
Area-Interaction
|
This is one of the major issues VS Code has with ConPTY as we have to guess which lines wrap (based on a naive heuristic) to try to avoid issues like microsoft/vscode#43664 |
|
@Tyriar Well, that heuristic doesn’t work well at all with a PowerLine prompt that has right‑aligned blocks. Also, are you sure you didn’t mean to say WinPTY? Because that is what the comment in the linked code mentions. |
|
@ExE-Boss its primary uses are link detection and error detection so that doesn't matter too much. Problems arise for example when errors have unfortunately placed spaces at the end of a wrapped line, leading to flaky detection. |
|
winpty acts in the same way as conpty, the workaround is for both. |
|
Per microsoft/vscode#81328, this is breaking problem matchers in VS Code in the case where the user has not already opened the terminal. |
|
API Consumers need to control output formatting, not API Producers. |
|
Okay, we're not disagreeing that this should be fixed. However, no one on the team has had the time to get to it quite yet. If a particularly passionate community member is interested in contributing, we'd be happy to review a PR. |
|
Which source file or project or function handle the text wrapping in normal CMD + ConHost combo? |
|
@Biswa96 this is probably less about the wrapping within conhost itself, but more about how ConPTY renders wrapped text. I'd probably take a look at In my original prototype, I believe I added a flag |
ghost
added
Resolution-Fix-Committed
## 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
|
@zadjii-msft in what Windows version should this be expected? I think I'll need to disable the special compatibility mode in VS Code that guesses wrapped lines based on the build number. |
|
@Tyriar the version what comes after 19041. |
|
Thanks very much! Thrilled to see this. I guess until stable release I'll also be able to enjoy it on insiders ;) |
This PR adds support for "Resize with Reflow" to the Terminal. In conhost, `ResizeWithReflow` is the function that's responsible for reflowing wrapped lines of text as the buffer gets resized. Now that #4415 has merged, we can also implement this in the Terminal. Now, when the Terminal is resized, it will reflow the lines of it's buffer in the same way that conhost does. This means, the terminal will no longer chop off the ends of lines as the buffer is too small to represent them. As a happy side effect of this PR, it also fixed #3490. This was a bug that plagued me during the investigation into this functionality. The original #3490 PR, #4354, tried to fix this bug with some heavy conpty changes. Turns out, that only made things worse, and far more complicated. When I really got to thinking about it, I realized "conhost can handle this right, why can't the Terminal?". Turns out, by adding resize with reflow, I was also able to fix this at the same time. Conhost does a little bit of math after reflowing to attempt to keep the viewport in the same relative place after a reflow. By re-using that logic in the Terminal, I was able to fix #3490. I also included that big ole test from #3490, because everyone likes adding 60 test cases in a PR. ## References * #4200 - this scenario * #405/#4415 - conpty emits wrapped lines, which was needed for this PR * #4403 - delayed EOL wrapping via conpty, which was also needed for this * #4354 - we don't speak of this PR anymore ## PR Checklist * [x] Closes #1465 * [x] Closes #3490 * [x] Closes #4771 * [x] Tests added/passed ## EDIT: Changes to this PR on 5 March 2020 I learned more since my original version of this PR. I wrote that in January, and despite my notes that say it was totally working, it _really_ wasn't. Part of the hard problem, as mentioned in #3490, is that the Terminal might request a resize to (W, H-1), and while conpty is preparing that frame, or before the terminal has received that frame, the Terminal resizes to (W, H-2). Now, there aren't enough lines in the terminal buffer to catch all the lines that conpty is about to emit. When that happens, lines get duplicated in the buffer. From a UX perspective, this certainly looks a lot worse than a couple lost lines. It looks like utter chaos. So I've introduced a new mode to conpty to try and counteract this behavior. This behavior I'm calling "quirky resize". The **TL;DR** of quirky resize mode is that conpty won't emit the entire buffer on a resize, and will trust that the terminal is prepared to reflow it's buffer on it's own. This will enable the quirky resize behavior for applications that are prepared for it. The "quirky resize" is "don't `InvalidateAll` when the terminal resizes". This is added as a quirk as to not regress other terminal applications that aren't prepared for this behavior (gnome-terminal, conhost in particular). For those kinds of terminals, when the buffer is resized, it's just going to lose lines. That's what currently happens for them. When the quirk is enabled, conpty won't repaint the entire buffer. This gets around the "duplicated lines" issue that requesting multiple resizes in a row can cause. However, for these terminals that are unprepared, the conpty cursor might end up in the wrong position after a quirky resize. The case in point is maximizing the terminal. For maximizing (height->50) from a buffer that's 30 lines tall, with the cursor on y=30, this is what happens: * With the quirk disabled, conpty reprints the entire buffer. This is 60 lines that get printed. This ends up blowing away about 20 lines of scrollback history, as the terminal app would have tried to keep the text pinned to the bottom of the window. The term. app moved the viewport up 20 lines, and then the 50 lines of conpty output (30 lines of text, and 20 blank lines at the bottom) overwrote the lines from the scrollback. This is bad, but not immediately obvious, and is **what currently happens**. * With the quirk enabled, conpty doesn't emit any lines, but the actual content of the window is still only in the top 30 lines. However, the terminal app has still moved 20 lines down from the scrollback back into the viewport. So the terminal's cursor is at y=50 now, but conpty's is at 30. This means that the terminal and conpty are out of sync, and there's not a good way of re-syncing these. It's very possible (trivial in `powershell`) that the new output will jump up to y=30 override the existing output in the terminal buffer. The Windows Terminal is already prepared for this quirky behavior, so it doesn't keep the output at the bottom of the window. It shifts it's viewport down to match what conpty things the buffer looks like. What happens when we have passthrough mode and WT is like "I would like quirky resize"? I guess things will just work fine, cause there won't be a buffer behind the passthrough app that the terminal cares about. Sure, in the passthrough case the Terminal could _not_ quirky resize, but the quirky resize won't be wrong.
|
@davidhewitt in case you're interested, I implemented the xterm.js (== VS Code) "naïve heuristic" in Alacritty here: This change resolves the issue with copying (e.g.,) |
|
@DHowett-MSFT Hi, I wonder if the fix (#4415) applies to conhost.exe on Windows newer than 19041? I've tested it on 19044 with ConPty APIs, and got I've briefly checked the code of #4415 and it seems that all changes relates to the WindowsTerminal part, rather than the conhost part. But I'm not familar with the whole architecture of the source tree. So if it does changed the behaviour of conhost, what sould I do if I want to decide whether a line is wrapped, or just happens to holds a line with same width to the window and a following hard break? Actually this is from alacritty/alacritty#2093 . I also tested with latest alacritty on 19044 and found it still gives breaked lines when copying content from the window, such as ssh public keys. Update: this seems to be already fix on Win11. Output files are attached to https://github.com/wesley800/rust-conpty-test/tree/master/output . Update2: A pre-compiled binary is added to the repo for those not familiar with rust toolchain. |
How did you perform this test? Just using a latest Alacritty build on stock Windows 11? What version of Windows 11? |
I don't know, probably the latest one. I don't have access to that machine in fact. I've confirmed that both latest Alacritty and rust-conpty-test shows no CR or LF at line wrapping points. |
|
@zadjii-msft In case no one is still watching this issue... |
|
Sorry, still catching up on mail after the holidays. I'd assume just based on the dates that #4415 would be in at least 19042 - it's at around this point in time that tracking code changes across win10 versions vs win11 versions starts getting a little more confusing... I can try and drill in and find where the commit to the OS repo is, but this should definitely be in 19044 and Windows 11, that I'm sure of. That code change is mostly a Conhost / ConPTY change - the bulk of the real change is in the VT renderer, which is what conpty uses to write the buffer to a connected terminal application[1] We do have a number of ongoing issues with line wrapping across the repo. #6901 (comment) has a collection of issues we've been seeing. It's hard to tease those apart to individual root causes, unfortunately. My headcannon is that there's some renderer state that's not getting set/reset correctly, resulting in the incorrectness of it all, but it's been hard to get exact repros. Maybe there's something to do with the volume of text written in a single frame? hard to say. [1] this is a vast oversimplification for the sake of brevity. |
|
Thank you for your time and detailed replies to my each question and hope I didn't break your vacation much. Actually I ran the test on Win10 LTSC 2022 which is 19044 by Also I might be wrong in the expectation of #4415, which seems to be mainly focused on rendering (as you've stated). From my perspective, changes are mostly among adding |
Nah it's fine, I'm just still slow coming back off paternity and only working like, half time. It's generally the holidays, so things are just slow here 😅 sorry for any delays! You might have something here. We should probably take a look through the history here. Why is I'll need to double tap on this. That also being said - it looks like on windows 11 the test works fine? Perhaps the fixes I'm thinking of didn't actually land till 22000. This might just be another point in favor of having a proper ConPTY nuget package, that folks can use to consume the updated ConPTY version (regardless of OS version). |
|
@zadjii-msft Any chance this could get fixed in Win10 19044/19045? The regular editions are supported at least till 2025-10-14, with IoT Enterprise LTSC (for which volume licencing is newly available) supported until 2032-01-13. It seems like a bug that should be fixed as part of a cumulative update. |
I agree with you! Unfortunately, the Windows organization doesn't necessarily agree. Changes like this -- especially ones that are in vanishingly rare cases that come with a high likelihood of destabilizing a working product -- are very difficult to get into a servicing update without a strong "business justiciation." The way we're tackling getting ConPTY bug fixes out to people on a quicker cadence than Windows releases is over in #15065: a NuGet package for folks to depend upon. 😄 |
I'm hitting problems using the Alacritty terminal. alacritty/alacritty#4993 Is there anyway I can get the latest version to make that bug go away? |
There's an invisible entry in the Alacritty FAQ that explains what you need to do. And if you search the Alacritty repo for conpty.dll openconsole.exe you'll find several comments from users explaining how they've got it to work. Here is one recent example: alacritty/alacritty#6704 (comment) |
It seems to me that output is line-wrapped by conhost before being sent through the ConPTY stdout pipe. This makes sense, though it has the unfortunate downside that in Alacritty we can't do the line-wrapping ourselves.
This wouldn't matter that much, but when selecting and copying text in Alacritty which has been line-wrapped by conhost, Alacritty isn't aware that the text was originally a single line and inserts a newline into the content placed on the clipboard.
Is it possible to disable the line wrapping by conhost?
(Perhaps this would be something that would naturally be in the "passthrough" mode I've seen mentioned elsewhere in this issue tracker?)
The text was updated successfully, but these errors were encountered: