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

No save dialog if another widget shares identical model #10614

Merged
merged 4 commits into from
Jan 31, 2022

Conversation

colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Jan 11, 2022

What it does

Fixes #9474 by modifying Saveable.apply to accept a function to generate a list of widgets that may share the same underlying saveable resource as the widget being closed. If any widget does, then the widget is closed without saving and without reverting the saveable's state.

How to test

  1. Open two editors for the same file.
  2. Dirty the file.
  3. Close one editor.
  4. Observe that no dialog is shown asking whether you would like to save and that the other editor remains dirty.
  5. Close the other editor.
  6. Observe that a dialog is shown asking whether you would like to save.

The tricky part is the multi-close commands, close all, close others, and close to the right.

  1. Use one of the above named command such that you would not close all instances of a document open in multiple editors.
  2. You should not be prompted to save.
  3. Use the same command in such a way that you would close all instances
  4. You should be prompted to save, and if you cancel, nothing should be closed.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added the editor issues related to the editor label Jan 11, 2022
@colin-grant-work colin-grant-work marked this pull request as draft January 11, 2022 00:43
@colin-grant-work colin-grant-work force-pushed the bugfix/save-request branch 2 times, most recently from 8f69c1e to f129560 Compare January 11, 2022 20:47
@colin-grant-work colin-grant-work marked this pull request as ready for review January 11, 2022 21:24
@colin-grant-work colin-grant-work added the shell issues related to the core shell label Jan 12, 2022
@msujew msujew self-requested a review January 17, 2022 15:33
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

The code looks code to me, and the functionality of this change as well:

  • Opening multiple editors for the same model (splitting) continues to work correctly
  • Closing a dirty editor whose model is shared with another widget does not prompt a confirmation message
  • Using the Close All context menu entry closes all duplicated editor models of the same group, exactly replicating the behavior of vscode
  • Closing the last instance of a dirty editor model prompts a confirmation message, as well as closing the app.

@colin-grant-work
Copy link
Contributor Author

Thanks, @msujew. I'll merge this after the release.

@colin-grant-work
Copy link
Contributor Author

@vince-fugnitto, I've disabled the run reference code lens test with a TODO. Does that seem reasonable?

@paul-marechal
Copy link
Member

@colin-grant-work please consider merging/squashing the following instead: #10688

This way you can squash and merge the current PR without the noise from skipping that failing test.

@paul-marechal
Copy link
Member

@colin-grant-work you can now remove 52da73d as #10688 got merged :)

@colin-grant-work colin-grant-work merged commit 0ad6d10 into master Jan 31, 2022
@colin-grant-work colin-grant-work deleted the bugfix/save-request branch January 31, 2022 22:48
@github-actions github-actions bot added this to the 1.23.0 milestone Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editor issues related to the editor shell issues related to the core shell
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prompted to save when multiple editors open for same file
3 participants