-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Handling Save as
of notebooks when using new VSC Notebook API
#11876
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
Conversation
* master: Sync VSCode output Cell Output & add INotebookEditor/INotebookEditorProvider (#11849) Do not execute shebang as an interpreter until user has clicked on the codelens enclosing the shebang (#11816) Debug and variable test failures on nightly (#11848) Add cell execution to the new VSC Notebooks (#11846) Prelimnary support to load ipyn files in native Notebook editor (#11841) Fix memleaks during tests (#11835) turn on raw kernel for insiders (#11820) Some fixes for nightly (#11802) correctly hook up session status in raw kernel (#11801) Remove waitForUpdate from tests to make them more deterministic (#11799) Fix run by line to work for restart/change kernel (#11795) Clarify that using a custom jedi also means a custom parso (#11708) Fix typo lanaguage -> language (#11720) Remove tpn (#11765) on raw kernel launch wait for ready, and have raw kernel switch respect timeout and retry (#11768) Allow azure files and only nest scrolling in notebooks (#11771)
Save as
of notebooks when using new VSC Notebook API
// tslint:disable-next-line: no-suspicious-comment | ||
// TODO: This will not work, if we close an untitled document. | ||
// See if we have any untitled storage already | ||
const untitledStorage = Array.from(this.models.values()).filter(isUntitled); |
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.
Why not use a counter? Aren't you mimicing the existing bug with 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.
u mimicing the existing bug with this?
Yes, didn't want to fix it here. Just want to leave the bug and fix it correctly with tests. Doing that means more work at this stage.
This is being tracked, hence leaving 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.
🕐
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.
Approved with a couple of small comments.
Kudos, SonarCloud Quality Gate passed!
|
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.
For #10496
Save As
.Final implementation details outlined here: