Don't veto shutdown in web mode #6091
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
E2E Tests 🚀 |
sharon-wang
previously approved these changes
Jan 23, 2025
sharon-wang
reviewed
Jan 23, 2025
Comment on lines
+324
to
+326
| if (!isWeb) { | ||
| e.veto(this.clearWorkspaceSessions(), 'positron.runtimeStartup.clearWorkspaceSessions'); | ||
| } |
There was a problem hiding this comment.
One more thought! Maybe a comment here noting that async operations are not supported in Web, and that's why we only veto on Desktop
There was a problem hiding this comment.
Oh that's a good idea! c4d22dd
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change fixes an issue in which Positron prompts to save changes whenever you try to navigate away from the page in web mode.
It turns out that in web mode (specifically), async shutdown operations aren't supported, and trying to use them results in this behavior. The fix is simply to not clear the session list in web mode, which should be fine as the reason we need to do this has to do with multiple Electron windows.
Addresses #6062.
QA Notes
As noted in the issue it's very easy to hit this, so should be easy to verify as well!