microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
94.95k stars 8.22k forks source link

megathread: Window State Persistence #9800

Open zadjii-msft opened 3 years ago

zadjii-msft commented 3 years ago
[Original thread: #766] [Spec: {none}] [Initial PR: #10972] [Multiple windows PR: #11083]

This thread is being used to track all the component work for restoring the window state. Additionally related threads:

Settings UI Mockup (programmer art warning) ![mockup](https://user-images.githubusercontent.com/18356694/130969227-73eca7ec-3f9b-4c27-be2d-cb68c6ba8f08.png)

2.0 Bugs

these are all presuming that #10972 merges basically as is

Rosefield commented 3 years ago

Re

How do we persist something like wt -- cmd.exe? That won't have a profile, only a NewTerminalArgs, that we don't cache at runtime

I can answer that concretely, just fine (the TerminalSettings on a control has all of these properties)

{
    ...
    "tabLayout" : 
    [
        {
            "action" : "newTab",
            "colorScheme" : "Solarized Dark",
            "commandline" : "cmd.exe",
            "profile" : "Default",
            "startingDirectory" : null,
            "suppressApplicationTitle" : false,
            "tabTitle" : "cmd.exe"
        }
    ]
}
Rosefield commented 3 years ago

Re

We should wrap this feature up in a Preview-only feature flag (no external community member should have to deal with that)

This is done on 10972 now

Opening an elevated window will automatically re-open all the unelevated window state (as elevated!). This is because there are separate monarchs for elevated and unelevated windows.

I disabled the feature entirely for elevated processes on 11083 in https://github.com/microsoft/terminal/pull/11083/commits/8ffd314b952b4cc98d3d5e84cce74220353bdeb7

Window names need to be persisted as well.

Was added to 11083 in https://github.com/microsoft/terminal/pull/11083/commits/a93d3b164058ccbe4803b74f62f3a0bda9af5758 With some minor modification it should be pretty safe to cherry-pick these if necessary.

How exactly does this work with the _quake window? Should that participate in the session restore? Probably, yea.

With just 10972 this is a problem since that is explicitly checking for the existence of only one window remaining. With 11083 the _quake window will be saved just fine, but might lead to confusing users if they closed all of their visible windows and thinking that it saved the last layout. If the quit action is added that just becomes a point of user education that quit is the proper way to close all of their windows and save them. There might be some issues with saving the position/size of the quake window, but there is already special logic to handle _quake so it might just turn out OK magically.

Same idea with a defterm connection - We won't be able to restore defterm panes at all, because we have no idea what profile they were. Right?

Hypothetically this should be the same as the commandline version above in that it saves fine, but as the "Default" profile.

zadjii-msft commented 3 years ago

wowow I half-assed a list of showerthoughts of things that might go wrong with this (that could be fixed in post), and the man just goes and fixes/addresses all of them. That's great, thanks!

Rosefield commented 2 years ago

Re

Closing the last window with ClosePane will clear out any persisted layout. Similarly with closing the last tab by clicking the x on the tab itself. Doesn't seem to save the window layout in that case.

That was deliberate behavior because I took those actions to mean "I don't want to save these tab(s)". If you close all of your tabs in your browser and then re-open the browser it wouldn't have saved anything either. This is yet again a problem that is ameliorated by quit.

zadjii-msft commented 2 years ago

Okay, thanks for clearing that up. That makes sense, it was a little unexpected by that's fine so long as we've got it all documented.

zadjii-msft commented 2 years ago

Another bug (sorry for the very non minimal repro)

        {
            "hidden":false,
            "name" : "Ubuntu",
            "background" : "#2C001E",
            "tabColor": "#2C001E",
            "commandline" : "wsl.exe",
            "colorScheme" : "Builtin Tango Dark",
        }

If you open a tab with that profile, then close the window and have it reopen, the background that's set in the profile (#2C001E) will not be visible, it'll be the one from he scheme

Rosefield commented 2 years ago

Added to #11083 (until that is merged I'll be making bug fixes there)

ykoehler commented 2 years ago

In the same idea, I would like to be able to "obtain" the wt command to execute to open up my tab exactly the same way (with same panes and panes size, etc). From there I could paste that into a shortcut icon and create a different layout for each icon to open with. So if somehow we could dump a wt command output that I re-run and get a window with the same tab/pane and size that would be appreciated, in this case, I do not care about running commands in those, but I get you would have to support it.

zadjii-msft commented 2 years ago

I'm stashing this here because it was weird, but not because I think we need to do anything about it.

I restarted after an update and found two of these entries in my state.json:


            "tabLayout" : 
            [
                {
                    "action" : "newTab",
                    "commandline" : "\"C:\\Windows\\System32\\cmd.exe\" /q /c rmdir /s /q \"C:\\Users\\migrie\\AppData\\Local\\Microsoft\\OneDrive\\21.234.1111.0001\"",
                    "profile" : "cmd",
                    "startingDirectory" : "C:\\Users\\migrie",
                    "suppressApplicationTitle" : false,
                    "tabTitle" : "C:\\Windows\\System32\\cmd.exe"
                }
            ]

(The other was for 21.226.1101.0001, but otherwise idendical)

Both of them opened like image

Which is quite curious. Presumably these are defterm windows that got captured by the Terminal, but also the Terminal must have been closed at just the right time that these were open.

Rosefield commented 2 years ago

What should still be done in order to enable persisted layouts for stable builds? I'm not sure if there is any telemetry for how often the feature is enabled / crashes related to it. My n=1 data says that it has been working quite well :).

zadjii-msft commented 2 years ago

That's a great question. I'm personally happy with it, I kinda forgot it was preview only. IMO we should have this in 1.12 Stable, but I'll discuss with the team.

zadjii-msft commented 2 years ago

Okay, team consensus. We're gonna move this to stable in 1.14. Just give it a little more time to bake. Overall, we're really happy with it, you did a great job ☺️