Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[1.21.921] Session persist doesn't trigger if the OS shuts down for an update #17014

Closed
zadjii-msft opened this issue Apr 4, 2024 · 2 comments · Fixed by #16914
Closed

[1.21.921] Session persist doesn't trigger if the OS shuts down for an update #17014

zadjii-msft opened this issue Apr 4, 2024 · 2 comments · Fixed by #16914
Labels
Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting

Comments

@zadjii-msft
Copy link
Member

Just took the update to 26100, and all by windows and tabs did get restored! But it was all the windows and tabs that were persisted the previous time. I had since closed a bunch of tabs - all those came back. And I didn't have buffer history for any of them. (I haven't quit the terminal manually, so that kinda tracks. I've only had it closed for updates).

I hope this is repro-able outside of the update scenario 😨
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 4, 2024
@zadjii-msft
Copy link
Member Author

IIRC there's a watson floating around that seems to suggest that #13614 regressed after 1.18's process model changes. Maybe that has something to do with it?

@zadjii-msft
Copy link
Member Author

I'M THE MURDERER

The try_as<SettingsPaneContent>() in TerminalTav::BuildStartupActions() always returns a settings content for me lol

try_as<> into a non-projected type always returns non-null garbage

Seriously?
Try_as doesn't work for private types? LOL

oh you know what, that explains why I didn't get a buffer on update, doesn't it
cause the SUI panes merged which broke session persisting

#16914 fixes this though!

@zadjii-msft zadjii-msft mentioned this issue Apr 4, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Apr 5, 2024
@zadjii-msft
Add support for restoring non-terminal panes, and opening them with `…
dc4026d 
…splitPane`, `newTab` (#16914)

This changes `NewTabArgs`, `SplitPaneArgs`, and `NewWindowArgs` to
accept a `INewContentArgs`, rather than just a `NewTerminalArgs`. This
allows a couple things:
* Users can open arbitrary types of panes with the existing `splitPane`,
`newWindow` actions, just by passing `"type": "scartchpad"` (for
example). This is a lot more flexible than re-defining different
`"openScratchpad"`, `"openTasksPane"`, etc, etc actions for every kind
of pane.
* This allows us to use the existing machinery of session restore to
also restore non-terminal panes.

The `type` property was added to `newTab`, `splitPane`, `newWindow`.
When omitted, we still just treat the json as a blob of NewTerminalArgs.

There's not actually any other kinds of `INewContentArgs` in this PR
(other than the placeholder `GenericContentArgs`). In
[`dev/migrie/fhl/md-pane`](dev/migrie/f/tasks-pane...dev/migrie/fhl/md-pane),
I have a type of pane that would LOVE to add some args here. So that's
forward-thinking.

There's really just two stealth types of pane for now: `settings`, and
`scratchpad`. Those I DON'T have as constants or anything in this PR.
They probably should be? Though, I suspect around the time of the tasks
& MD panes, I'll come up with whatever structure I actually want them to
take.

### future considerations here

* In the future, this should allow extensions to say "I know how to host
`foo` content", for 3p content.
* The `wt` CLI args were not yet updated to also accept `--type` yet.
There's no reason we couldn't easily do that.
* I considered adding `ICanHasCommandline` to allow arbitrary content to
generate a `wt` commandline-serializable string. Punted on that for now.


## other PRs
* #16170
  * #16171 
    * #16172 
      * #16895 
      * #16914 <-- you are here 

Closes #17014
@github-actions github-actions bot mentioned this issue May 2, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting
Projects
None yet
Milestone
No milestone
1 participant
@zadjii-msft