-
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
Don't push the buffer contents into the scrollback on session restore #17274
Comments
Oh lord. You're right. It's there and I just never scrolled up 🤦 I suppose I was expecting the window to look exactly the same as the way I left it (i.e. at the same scrolling position). Well I'm certainly happy that it works (been desperately wanting this feature for a long time). I guess my feedback is that I would prefer an option to additionally restore the scrolling position. However I recognize that that doesn't necessarily make sense (at least in the current implementation), because the restored buffer is not actually 1:1 with the buffer from before the restoration due to the injection of the Thanks for your response. |
You're not the first person to bring this flaw up. We've talked about fixing the issue before, and I'm mostly leaning towards doing so now. However, this will increase the startup cost quite a bit, as the cursor position after session restoration needs to be synced up, etc. |
I do think if that "fix" is added, it would be best as an optional setting (maybe even non-default), as I can definitely see people (perhaps even myself at times) preferring the current behavior. I'm guessing that would also mean people could opt into/out of the additional startup cost, maybe. Just my 2 cents. |
First, this makes use of `PSEUDOCONSOLE_INHERIT_CURSOR` to stop ConPTY from emitting a CSI 2 J on startup. Then, it uses `Terminal::SetViewportPosition` to fake-scroll the viewport down so that only 3 lines of scrollback are visible. It avoids printing actual newlines because if we later change the text buffer to actually track the written contents, we don't want those newlines to end up in the next buffer snapshot. Closes #17274 ## Validation Steps Performed * Restore cmd multiple times * There's always exactly 3 lines visible ✅
Windows Terminal version
Windows Terminal Preview Version: 1.21.1272.0
Windows build number
10.0.19045.0
Other Software
No response
Summary
I installed Windows Terminal Preview to make use of the new buffer restoration feature, noted as the first bullet here: https://github.com/microsoft/terminal/releases/tag/v1.21.1272.0 (i.e. #16598), but it doesn't seem to behave any differently than previous versions, out of the box. Not sure if the feature is broken, or I'm doing something wrong, or what.
While searching related issues, I saw #17179, but it seems to be related more to special cases while restoring after a reboot due to Windows updates. In my case, I can't get the functionality to work even under the simplest condition of manually closing and reopening Terminal.
Steps to reproduce
Settings -> Startup -> When Terminal starts
is set toOpen windows from a previous session
.Get-Process
.Expected Behavior
Original tabs, titles, and contents are restored. In the example case, the single PowerShell 7 tab should have the output of the previously-run
Get-Process
command displayed in the buffer content, along with however much further buffer history is expected by this feature.Actual Behavior
Original tabs and tab titles are restored (as in previous versions), but buffer contents are empty (as in previous versions).
Additional info
According to the release notes linked above:
I do see a file named
buffer_<guid>.txt
alongside mysettings.json
andstate.json
files in the Terminal Preview installation'sLocalState
folder (where such a file doesn't exist in the production Terminal app'sLocalState
folder). And that file does contain the output ofGet-Process
, as expected. But It's apparently just not being pulled into Terminal on a restart. I even see a couple lines in the buffer file that look like[Restored <DateTime>]
.The text was updated successfully, but these errors were encountered: