Repository: microsoft/terminal (language: cpp)

Issue:
Don't push the buffer contents into the scrollback on session restore
### 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. https://github.com/microsoft/terminal/pull/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 https://github.com/microsoft/terminal/issues/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
1. Install Windows Terminal Preview Version: 1.21.1272.0 from the MS app store.
2. Launch Terminal Preview.
3. Ensure that `Settings -> Startup -> When Terminal starts` is set to `Open windows from a previous session`.
4. Run some commands in a tab. In my case I'm just testing with a PowerShell 7 tab with no profile customizations and running `Get-Process`.
5. Close Terminal Preview.
6. Launch Preview.

### 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:

> Buffer snapshots are stored in human-readable (albeit UTF-16) VT-encoded text in your package's local state directory, next to your settings.

I do see a file named `buffer_<guid>.txt` alongside my `settings.json` and `state.json` files in the Terminal Preview installation's `LocalState` folder (where such a file doesn't exist in the production Terminal app's `LocalState` folder). And that file does contain the output of `Get-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>]`.


Discussion / hints:
So, this may sound silly, but did you scroll up after the buffer was restored/? The old buffer contents are restored into the scrollback _above_ the new viewport

![image](https://github.com/microsoft/terminal/assets/18356694/13cc9622-fdff-4b3d-be93-0f281861f40a)



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 `[Restored <DateTime>]` events, which are actually a nice feature IMO, as they clearly log when the console was restarted.

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.

Fix patch:
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index ca86bb80a37..064d341b816 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -2994,15 +2994,6 @@ namespace winrt::TerminalApp::implementation
 
         const auto content = _manager.CreateCore(settings.DefaultSettings(), settings.UnfocusedSettings(), connection);
         const TermControl control{ content };
-
-        if (const auto id = settings.DefaultSettings().SessionId(); id != winrt::guid{})
-        {
-            const auto settingsDir = CascadiaSettings::SettingsDirectory();
-            const auto idStr = Utils::GuidToPlainString(id);
-            const auto path = fmt::format(FMT_COMPILE(L"{}\\buffer_{}.txt"), settingsDir, idStr);
-            control.RestoreFromPath(path);
-        }
-
         return _SetupControl(control);
     }
 
@@ -3102,7 +3093,10 @@ namespace winrt::TerminalApp::implementation
             return nullptr;
         }
 
-        auto connection = existingConnection ? existingConnection : _CreateConnectionFromSettings(profile, controlSettings.DefaultSettings(), false);
+        const auto sessionId = controlSettings.DefaultSettings().SessionId();
+        const auto hasSessionId = sessionId != winrt::guid{};
+
+        auto connection = existingConnection ? existingConnection : _CreateConnectionFromSettings(profile, controlSettings.DefaultSettings(), hasSessionId);
         if (existingConnection)
         {
             connection.Resize(controlSettings.DefaultSettings().InitialRows(), controlSettings.DefaultSettings().InitialCols());
@@ -3124,6 +3118,14 @@ namespace winrt::TerminalApp::implementation
 
         const auto control = _CreateNewControlAndContent(controlSettings, connection);
 
+        if (hasSessionId)
+        {
+            const auto settingsDir = CascadiaSettings::SettingsDirectory();
+            const auto idStr = Utils::GuidToPlainString(sessionId);
+            const auto path = fmt::format(FMT_COMPILE(L"{}\\buffer_{}.txt"), settingsDir, idStr);
+            control.RestoreFromPath(path);
+        }
+
         auto paneContent{ winrt::make<TerminalPaneContent>(profile, _terminalSettingsCache, control) };
 
         auto resultPane = std::make_shared<Pane>(paneContent);
diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
index c4ad4b0799d..d82bfac365c 100644
--- a/src/cascadia/TerminalControl/ControlCore.cpp
+++ b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -1787,12 +1787,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             // * prints "  [Restored <date> <time>]  <spaces until end of line>  "
             // * resets the color ("\x1b[m")
             // * newlines
-            // * clears the screen ("\x1b[2J")
-            // The last step is necessary because we launch ConPTY without PSEUDOCONSOLE_INHERIT_CURSOR by default.
-            // This will cause ConPTY to emit a \x1b[2J sequence on startup to ensure it and the terminal are in-sync.
-            // If we didn't do a \x1b[2J ourselves as well, the user would briefly see the last state of the terminal,
-            // before it's quickly scrolled away once ConPTY has finished starting up, which looks weird.
-            message = fmt::format(FMT_COMPILE(L"\x1b[100;37m  [{} {} {}]\x1b[K\x1b[m\r\n\x1b[2J"), msg, date, time);
+            message = fmt::format(FMT_COMPILE(L"\x1b[100;37m  [{} {} {}]\x1b[K\x1b[m\r\n"), msg, date, time);
         }
 
         wchar_t buffer[32 * 1024];
@@ -1830,6 +1825,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             }
 
             _terminal->Write(message);
+
+            // Show 3 lines of scrollback to the user, so they know it's there. Otherwise, in particular with the well
+            // hidden touch scrollbars in WinUI 2 and later, there's no indication that there's something to scroll up to.
+            //
+            // We only show 3 lines because ConPTY doesn't know about our restored buffer contents initially,
+            // and so ReadConsole calls will return whitespace.
+            //
+            // We also avoid using actual newlines or similar here, because if we ever change our text buffer implementation
+            // to actually track the written contents, we don't want this to be part of the next buffer snapshot.
+            const auto cursorPosition = _terminal->GetCursorPosition();
+            const auto y = std::max(0, cursorPosition.y - 4);
+            _terminal->SetViewportPosition({ 0, y });
         }
     }
 


Test patch: