-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix: file-saving dialog with dirty editors #12864
Conversation
This commit fixes the issue where cancelling the file-saving dialog incorrectly closes the editor. Signed-off-by: Vlad Arama <vlad.arama@ericsson.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly enough, I now have the exact opposite issue: When closing an untitled file and saving it to a new location, the editor isn't closed. Instead the model now points to the newly created file. VSCode correctly closes the editor after saving the untitled file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vladarama Are you planning to make this other fix part of this PR? |
@JonasHelming |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sure. If you want to address the other issue separately, then I'm fine with merging this PR. LGTM 👍
This commit fixes the issue where cancelling the file-saving dialog incorrectly closes the editor. Signed-off-by: Vlad Arama <vlad.arama@ericsson.com>
b4e91d1
to
8827d4b
Compare
@vince-fugnitto Do you want to merge this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dismissing my approval, the change leads to failing browser tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @msujew ,
When we cancel the file-saving dialog, the dirty editor should not be disposed. I updated the browser tests to reflect this behavior. It should be good to go now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, looks good to me 👍
@vince-fugnitto Do you want to merge this now? |
@JonasHelming sure, we can merge for the release :) |
What it does
Fixes: #12859
This PR fixes a bug where cancelling the file-saving dialog when closing a dirty editor would actually close the editor.
With the new changes, cancelling the dialog will leave the dirty editor opened, which is the intended behavior.
How to test
File
->New Text File
orAlt+N
).X
orAlt+W
.Cancel
.Review checklist
Reminder for reviewers