Issue with line-wrapped tabstops after resizing the window #4669
Comments
ghost
added
Needs-Triage
|
If this is what I think it is, it's a bug in conhost. The problem is that the VT tab stops are initialized at startup (more or less) in just enough positions to fill the width of the buffer. If you make the screen wider, there won't be any tab stops in the newly revealed area. Now it's not just a matter of reinitializing the tab stops after a resize, because we need to preserve the state of any tab stops which have been added and removed by VT escape sequences. And when the screen is made narrower, we even need to remember the state of tab stops that have gone off screen in case the screen is made wider again (at which point they will need to be reapplied). The bottom line is it's probably not a simple fix. I have an item on my TODO list to put together a proposal for the tab management code that would address this, but I just haven't gotten around to it yet. Part of that was also moving the tab management out of the If this considered a priority, though, you may want to consider a simpler patch of the existing code, even if it doesn't perfectly address all the edge cases. |
Yeah I just ran a quick test and this looks like a good explanation. I was able to reproduce my issue with just (a) a small initial window size, (b) a resize to make the window larger, and (c) printing a line with a literal tab that falls beyond the original width of the window. I thought the issue only appeared if you first caused a tab to appear on the second+ line of a wrapped line first, before resizing... but that's not the case after all. As to priorities: This is a real pain in the neck when you have a CLI program that uses tabs in its output, as if your window started too small for your output there's no way to resize it to make it work. (The workaround is to start a new Terminal tab after resizing—which is at best inconvenient—or to pipe the output through a formatter which converts tabs to spaces.) |
|
I'm curious, actually. This is a big of an annoying ask, so feel free to shoot me down. I can't reproduce it over here, so: If you grab the latest .msixbundle from our releases page and unzip it (it's just a zip file), then further unzip the "x64" .msix inside it (also just a zip file) and run OpenConsole.exe... do you still see this behavior? OpenConsole is the console host (conhost.exe), but built out of this repository. That'll help us narrow down whether it's conhost or the terminal that's doing something wrong here. |
DHowett-MSFT
added
Area-VT
DHowett-MSFT
removed
the
Needs-Triage
Yes, I've just replicated it using
The space between square brackets is a tab. |
|
And, because why not, here's the file I tested with. |
IANAEITE (I Am Not An Expert In Terminal Emulation)... but that seems like a strange choice in and of itself. I'd have thought the only stateful tab stops would be the user-specified ones. Is there a benefit to pre-calculating the rest? (Also, I'd just like to say: I am constantly and profoundly impressed with this project, this team, and this community. You guys rock! This project is keeping Windows my preferred development environment.) |
ghost
added
Resolution-Fix-Committed
## Summary of the Pull Request This is essentially a rewrite of the VT tab stop functionality, implemented entirely within the `AdaptDispatch` class. This significantly simplifies the `ConGetSet` interface, and should hopefully make it easier to share the functionality with the Windows Terminal VT implementation in the future. By removing the dependence on the `SCREEN_INFORMATION` class, it fixes the problem of the the tab state not being preserved when switching between the main and alternate buffers. And the new architecture also fixes problems with the tabs not being correctly initialized when the screen is resized. ## References This fixes one aspect of issue #3545. It also supersedes the fix for #411 (PR #2816). I'm hoping the simplification of `ConGetSet` will help with #3849. ## PR Checklist * [x] Closes #4669 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [x] Tests added/passed * [ ] Requires documentation to be updated * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Detailed Description of the Pull Request / Additional comments In the new tab architecture, there is now a `vector<bool>` (__tabStopColumns_), which tracks whether any particular column is a tab stop or not. There is also a __initDefaultTabStops_ flag indicating whether the default tab stop positions need to be initialised when the screen is resized. The way this works, the vector is initially empty, and only initialized (to the current width of the screen) when it needs to be used. When the vector grows in size, the __initDefaultTabStops_ flag determines whether the new columns are set to false, or if every 8th column is set to true. By default we want the latter behaviour - newly revealed columns should have default tab stops assigned to them - so __initDefaultTabStops_ is set to true. However, after a `TBC 3` operation (i.e. we've cleared all tab stops), there should be no tab stops in any newly revealed columns, so __initDefaultTabStops_ is set to false. Note that the __tabStopColumns_ vector is never made smaller when the window is shrunk, and that way it can preserve the state of tab stops that are off screen, but which may come into range if the window is made bigger again. However, we can can still reset the vector completely after an `RIS` or `TBC 3` operation, since the state can then be reconstructed automatically based on just the __initDefaultTabStops_ flag. ## Validation Steps Performed The original screen buffer tests had to be rewritten to set and query the tab stop state using escape sequences rather than interacting with the `SCREEN_INFORMATION` class directly, but otherwise the structure of most tests remained largely the same. However, the alt buffer test was significantly rewritten, since the original behaviour was incorrect, and the initialization test was dropped completely, since it was no longer applicable. The adapter tests were also dropped, since they were testing the `ConGetSet` interface which has now been removed. I also had to make an addition to the method setup of the screen buffer tests (making sure the viewport was appropriately initialized), since there were some tests (unrelated to tab stops) that were previously dependent on the state being set in the tab initialization test which has now been removed. I've manually tested the issue described in #4669 and confirmed that the tabs now produce the correct spacing after a resize.
|
🎉This issue was addressed in #5173, which has now been successfully released as Handy links: |
Environment
Steps to reproduce
Expected behavior
The second printing of the text is printed in a single line, with the tab producing the expected amount of horizontal whitespace.
Actual behavior
The line of text is corrupted, with the tab consuming too much horizontal whitespace (= the full remaining width of the window - 1), causing the line to sill be wrapped across multiple lines as well.
I would assume this is related to the various problems listed in #4200, but I don't see this specifically mentioned (though some of the issues listed there are beyond my ability to grok).
Fully fleshed-out example:
Assuming an initial window width of approximately 80 characters, here's an example command that will print a suitable line:
On a smallish window, that renders something like this:
If you then resize the window wide enough that the output line is reflowed onto a single line...
... and then re-run the echo you get something clearly quite wrong:
The text was updated successfully, but these errors were encountered: