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.

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

backup scratchpads on window close for hotexit=onexit #183473

Merged
merged 9 commits into from
Jun 2, 2023

Conversation

amunger
Copy link
Contributor

@amunger amunger commented May 25, 2023

fixes #183378

this will change scratchpads to treat both hot exit settings the same, and will only veto for

test('should NOT hot exit (reason: CLOSE, windows: multiple, empty workspace)'
test('should NOT hot exit (reason: LOAD, windows: single, empty workspace)'
test('should NOT hot exit (reason: LOAD, windows: multiple, empty workspace)'

@amunger amunger requested a review from bpasero May 25, 2023 22:11
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this change missing some cases, such as changing to a different folder inside a window when having scratchpad working copies? I think in all cases where false (or now [] with your change) is returned, we would always want to include the scratchpad working copies, no?

@amunger
Copy link
Contributor Author

amunger commented May 26, 2023

I think in all cases where false (or now [] with your change) is returned, we would always want to include the scratchpad working copies, no?

I could have gone either way for context switches, so I'll change to recover scratchpads in that case.

But for test('should NOT hot exit (reason: CLOSE, windows: multiple, empty workspace)', we probably can't just backup since we can't tie it to a workspace to be recovered. We have a mechanism to recover those working copies when the application exits (just open an additional window next startup), but it's not as easy to recover when not exiting the app.

@bpasero
Copy link
Member

bpasero commented May 26, 2023

Yes, I would think that scrapbook working copies can only automatically backup in workspace/folder contexts where they can easily be recovered by simply opening said workspace/folder again. For empty windows its harder because we do not keep track of them when you change away from them in a window.

@bpasero
Copy link
Member

bpasero commented May 26, 2023

I think essentially scrapbook working copies should be handled as if "files.hotExit": "onExitAndWindowClose" was configured. I realize how the setting is a bit misleading, because in fact we also support it when the window is not closed but changed to another workspace...

@amunger amunger modified the milestones: May 2023, June 2023 May 30, 2023
@amunger amunger merged commit 65600c1 into main Jun 2, 2023
6 checks passed
@amunger amunger deleted the aamunger/scratchpadHotExit branch June 2, 2023 17:47
@github-actions github-actions bot locked and limited conversation to collaborators Jul 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

scratchpad editors should not confirm on window close when hotexit = onexit
4 participants