Consider not wrapping the alternate screen on resize #3493
Assignees
Labels
Area-Output
Related to output processing (inserting text into buffer, retrieving buffer text, etc.)
Area-TerminalControl
Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.)
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Priority-1
A description (P1)
Product-Conpty
For console issues specifically related to conpty
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Comments
egmontkob
added
the
Issue-Feature
ghost
added
Needs-Triage
zadjii-msft
added
Product-Conpty
16 tasks
4 tasks
DHowett
pushed a commit
that referenced
this issue
Apr 12, 2022
This PR allows the Terminal to actually use the alt buffer
appropriately. Currently, we just render the alt buffer state into the
main buffer and that is wild. It means things like `vim` will let the
user scroll up to see the previous history (which it shouldn't).
Very first thing this PR does: updates the
`{Trigger|Invalidate}Circling` methods to instead be
`{Trigger|Invalidate}Flush(bool circling)`. We need this so that when an
app requests the alt buffer in conpty, we can immediately flush the
frame before asking the Terminal side to switch to the other buffer. The
`Circling` methods was a great place to do this, but we don't actually
want to set the circled flag in VtRenderer when that happens just for a
flush.
The Terminal's implementation is a little different than conhost's.
Conhost's implementation grew organically, so I had it straight up
create an entire new screen buffer for the alt buffer. The Terminal
doesn't need all that! All we need to do is have a separate `TextBuffer`
for the alt buffer contents. This makes other parts easier as well - we
don't really need to do anything with the `_mutableViewport` in the alt
buffer, because it's always in the same place. So, we can just leave it
alone and when we come back to the main buffer, there it is. Helper
methods have been updated to account for this.
* [x] Closes #381
* [x] Closes #3492
* #3686, #3082, #3321, #3493 are all good follow-ups here.
ghost
pushed a commit
that referenced
this issue
Apr 15, 2022
VTE only rewraps the contents of the (normal screen + its scrollback buffer) on a resize event. It doesn't rewrap the contents of the alternate screen. The alternate screen is used by applications which repaint it after a resize event. So, it doesn't really matter. However, in that short time window, after resizing the terminal but before the application catches up, this prevents vertical lines It was really hard to get a gif of this where it happened and was small enough to upload to GH, but there is one in #12719. There's something in this branch that fixes a scrolling issue in the parent PR. I'm partially filing this so I can look at the diffs here and try and figure out what that is. I kinda want to just take all 3 alt buffer PRs as a single atomic unit, but splitting them up made sense from a review standpoint. Closes #3493
ghost
added
the
Resolution-Fix-Committed
|
🎉This issue was addressed in #12719, which has now been successfully released as Handy links: |
This issue was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description of the new feature/enhancement
VTE only rewraps the contents of the (normal screen + its scrollback buffer) on a resize event. It doesn't rewrap the contents of the alternate screen.
Rationale:
The alternate screen is used by applications which repaint it after a resize event. So it doesn't really matter.
However, in that short time window, after resizing the terminal but before the application catches up, this prevents vertical lines (e.g. the user interface of
mc) from becoming ugly tilted.The text was updated successfully, but these errors were encountered: