Reopen a notebook does not mark it as modified #2789
Conversation
| return object !== null && typeof object === 'object'; | ||
| } | ||
| const objectKeys1 = Object.keys(object1); | ||
| const objectKeys2 = Object.keys(object2); |
There was a problem hiding this comment.
very minor nitpick: object1Keys and object2Keys may read slightly better
src/polyglot-notebooks-vscode-common/src/vscodeNotebookManagement.ts
Outdated
Show resolved
Hide resolved
| const notebook = vscode.workspace.notebookDocuments.find(d => d.uri === notebookUri); | ||
| if (notebook) { | ||
| const metadata = notebook.metadata; | ||
| const shouldUpdate = compare(metadata, documentMetadata); |
There was a problem hiding this comment.
I was wondering why we need to update when the metadata has not changed i.e. when compare returns true. Could you please describe what the fix is doing here (just for my own understanding :))?
| return succeeded; | ||
| } | ||
|
|
||
| export async function replaceNotebookMetadata(notebookUri: vscode.Uri, documentMetadata: { [key: string]: any }): Promise<boolean> { |
There was a problem hiding this comment.
Do we have tests for this method? This was a pretty annoying bug and a significant code change.
If not, it might be a good conversation about how we can test things like this.
There was a problem hiding this comment.
no it needs to be refactored to be testable
There was a problem hiding this comment.
You know I love to refactor!
colombod
force-pushed
the
notebookmodified
branch
from
956b4e5 to
e4431ad
Compare
colombod
force-pushed
the
notebookmodified
branch
2 times, most recently
from
026ec04 to
319871b
Compare
| "frontend" | ||
| ], | ||
| "languageName": null, | ||
| "name": "vscode" |
There was a problem hiding this comment.
Should this be in the list?
There was a problem hiding this comment.
it is one of the kernel present
There was a problem hiding this comment.
filtering out kernels not implementing submitcode
rework comparing and have tests notebooks notebooks notebooks notebooks notebooks notebook notebooks fix for ipynb notebooks notebooks notebooks notebooks wip sort objects when merging metadata handle documentKernelInfo at sorting too sorted sort and merge metadata rewrite object notebook notebooks notebooks notebook fix compiler error test
colombod
force-pushed
the
notebookmodified
branch
from
31ae590 to
656be8b
Compare
notebooks notebooks filter out kernel not supporting submitcode notebooks
colombod
force-pushed
the
notebookmodified
branch
from
d2841b5 to
322e1a8
Compare
src/polyglot-notebooks-vscode-common/src/notebookControllers.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Brett V. Forsgren <brettfo@microsoft.com>
| } | ||
|
|
||
| export function getRawNotebookDocumentMetadataFromNotebookDocumentMetadata(notebookDocumentMetadata: NotebookDocumentMetadata, createForIpynb: boolean): { [key: string]: any } { | ||
| export function getRawNotebookDocumentMetadataFromNotebookDocumentMetadata(notebookDocumentMetadata: NotebookDocumentMetadata, documentRawMetadata: { [key: string]: any }, createForIpynb: boolean): { [key: string]: any } { |
There was a problem hiding this comment.
It seems that this method is now doing more than one responsibility. Perhaps something like getAndMergetNotebookMetadata
5 participants
No description provided.