diff --git a/news/2 Fixes/8481.md b/news/2 Fixes/8481.md new file mode 100644 index 000000000000..6361eb88d530 --- /dev/null +++ b/news/2 Fixes/8481.md @@ -0,0 +1 @@ +Creating a new blank notebook should not require a search for jupyter. diff --git a/news/2 Fixes/8594.md b/news/2 Fixes/8594.md new file mode 100644 index 000000000000..c181570a62a7 --- /dev/null +++ b/news/2 Fixes/8594.md @@ -0,0 +1 @@ +Typing 'z' in a cell causes the cell to disappear. diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 25de6408886b..62a53097ee4f 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -122,7 +122,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { return this.close(); } - public get contents(): string { + public getContents(): Promise { return this.generateNotebookContent(this.visibleCells); } @@ -432,6 +432,9 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { * @memberof NativeEditor */ private async updateVersionInfoInNotebook(): Promise { + // Make sure we have notebook json + await this.ensureNotebookJson(); + // Use the active interpreter const usableInterpreter = await this.jupyterExecution.getUsableJupyterPython(); if (usableInterpreter && usableInterpreter.version && this.notebookJson.metadata && this.notebookJson.metadata.language_info) { @@ -439,24 +442,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } } - private async loadContents(contents: string | undefined, forceDirty: boolean): Promise { - // tslint:disable-next-line: no-any - const json = contents ? JSON.parse(contents) as any : undefined; - - // Double check json (if we have any) - if (json && !json.cells) { - throw new InvalidNotebookFileError(this.file.fsPath); - } - - // Then compute indent. It's computed from the contents - if (contents) { - this.indentAmount = detectIndent(contents).indent; - } - - // Then save the contents. We'll stick our cells back into this format when we save - if (json) { - this.notebookJson = json; - } else { + private async ensureNotebookJson(): Promise { + if (!this.notebookJson || !this.notebookJson.metadata) { const pythonNumber = await this.extractPythonMainVersion(this.notebookJson); // Use this to build our metadata object // Use these as the defaults unless we have been given some in the options. @@ -484,6 +471,26 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { metadata: metadata }; } + } + + private async loadContents(contents: string | undefined, forceDirty: boolean): Promise { + // tslint:disable-next-line: no-any + const json = contents ? JSON.parse(contents) as any : undefined; + + // Double check json (if we have any) + if (json && !json.cells) { + throw new InvalidNotebookFileError(this.file.fsPath); + } + + // Then compute indent. It's computed from the contents + if (contents) { + this.indentAmount = detectIndent(contents).indent; + } + + // Then save the contents. We'll stick our cells back into this format when we save + if (json) { + this.notebookJson = json; + } // Extract cells from the json const cells = contents ? json.cells as (nbformat.ICodeCell | nbformat.IRawCell | nbformat.IMarkdownCell)[] : []; @@ -705,9 +712,12 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } private async setDirty(): Promise { - // Always update storage. Don't wait for results. - this.storeContents(this.generateNotebookContent(this.visibleCells)) - .catch(ex => traceError('Failed to generate notebook content to store in state', ex)); + // Update storage if not untitled. Don't wait for results. + if (!this.isUntitled) { + this.generateNotebookContent(this.visibleCells) + .then(c => this.storeContents(c).catch(ex => traceError('Failed to generate notebook content to store in state', ex))) + .ignoreErrors(); + } // Then update dirty flag. if (!this._dirty) { @@ -789,7 +799,10 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { return usableInterpreter && usableInterpreter.version ? usableInterpreter.version.major : 3; } - private generateNotebookContent(cells: ICell[]): string { + private async generateNotebookContent(cells: ICell[]): Promise { + // Make sure we have some + await this.ensureNotebookJson(); + // Reuse our original json except for the cells. const json = { ...(this.notebookJson as nbformat.INotebookContent), @@ -825,7 +838,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { if (fileToSaveTo && isDirty) { // Write out our visible cells - await this.fileSystem.writeFile(fileToSaveTo.fsPath, this.generateNotebookContent(this.visibleCells)); + await this.fileSystem.writeFile(fileToSaveTo.fsPath, await this.generateNotebookContent(this.visibleCells)); // Update our file name and dirty state this._file = fileToSaveTo; diff --git a/src/datascience-ui/native-editor/nativeEditor.tsx b/src/datascience-ui/native-editor/nativeEditor.tsx index 666e53377ac8..88d5c5fc6952 100644 --- a/src/datascience-ui/native-editor/nativeEditor.tsx +++ b/src/datascience-ui/native-editor/nativeEditor.tsx @@ -367,14 +367,16 @@ export class NativeEditor extends React.Component { test('Create new editor and add some cells', async () => { const editor = createEditor(); await editor.load(baseFile, Uri.parse('file:///foo.ipynb')); - expect(editor.contents).to.be.equal(baseFile); + expect(await editor.getContents()).to.be.equal(baseFile); editor.onMessage(InteractiveWindowMessages.InsertCell, { index: 0, cell: createEmptyCell('1', 1) }); expect(editor.cells).to.be.lengthOf(4); expect(editor.isDirty).to.be.equal(true, 'Editor should be dirty'); @@ -273,7 +273,7 @@ suite('Data Science - Native Editor', () => { test('Move cells around', async () => { const editor = createEditor(); await editor.load(baseFile, Uri.parse('file:///foo.ipynb')); - expect(editor.contents).to.be.equal(baseFile); + expect(await editor.getContents()).to.be.equal(baseFile); editor.onMessage(InteractiveWindowMessages.SwapCells, { firstCellId: 'NotebookImport#0', secondCellId: 'NotebookImport#1' }); expect(editor.cells).to.be.lengthOf(3); expect(editor.isDirty).to.be.equal(true, 'Editor should be dirty'); @@ -283,7 +283,7 @@ suite('Data Science - Native Editor', () => { test('Edit/delete cells', async () => { const editor = createEditor(); await editor.load(baseFile, Uri.parse('file:///foo.ipynb')); - expect(editor.contents).to.be.equal(baseFile); + expect(await editor.getContents()).to.be.equal(baseFile); expect(editor.isDirty).to.be.equal(false, 'Editor should not be dirty'); editor.onMessage(InteractiveWindowMessages.EditCell, { changes: [{ @@ -313,7 +313,7 @@ suite('Data Science - Native Editor', () => { async function loadEditorAddCellAndWaitForMementoUpdate(file: Uri) { const editor = createEditor(); await editor.load(baseFile, file); - expect(editor.contents).to.be.equal(baseFile); + expect(await editor.getContents()).to.be.equal(baseFile); storageUpdateSpy.resetHistory(); editor.onMessage(InteractiveWindowMessages.InsertCell, { index: 0, cell: createEmptyCell('1', 1) }); expect(editor.cells).to.be.lengthOf(4); @@ -325,7 +325,7 @@ suite('Data Science - Native Editor', () => { // Confirm contents were saved. expect(storage.get(`notebook-storage-${file.toString()}`)).not.to.be.undefined; - expect(editor.contents).not.to.be.equal(baseFile); + expect(await editor.getContents()).not.to.be.equal(baseFile); return editor; } @@ -359,8 +359,8 @@ suite('Data Science - Native Editor', () => { // Verify contents are different. // Meaning it was not loaded from file, but loaded from our storage. - expect(newEditor.contents).not.to.be.equal(baseFile); - const notebook = JSON.parse(newEditor.contents); + expect(await newEditor.getContents()).not.to.be.equal(baseFile); + const notebook = JSON.parse(await newEditor.getContents()); // 4 cells (1 extra for what was added) expect(notebook.cells).to.be.lengthOf(4); }); @@ -387,8 +387,8 @@ suite('Data Science - Native Editor', () => { // Verify contents are different. // Meaning it was not loaded from file, but loaded from our storage. - expect(newEditor.contents).not.to.be.equal(baseFile); - const notebook = JSON.parse(newEditor.contents); + expect(await newEditor.getContents()).not.to.be.equal(baseFile); + const notebook = JSON.parse(await newEditor.getContents()); // 4 cells (1 extra for what was added) expect(notebook.cells).to.be.lengthOf(4); }); @@ -417,18 +417,36 @@ suite('Data Science - Native Editor', () => { // Verify contents are different. // Meaning it was not loaded from file, but loaded from our storage. - expect(newEditor.contents).to.be.equal(baseFile); + expect(await newEditor.getContents()).to.be.equal(baseFile); expect(newEditor.cells).to.be.lengthOf(3); }); + test('Python version info is not queried on creating a blank editor', async () => { + const file = Uri.parse('file:///Untitled1.ipynb'); + + // When a cell is executed, then ensure we store the python version info in the notebook data. + when(executionProvider.getUsableJupyterPython()).thenReject(); + + const editor = createEditor(); + await editor.load('', file); + + try { + await editor.getContents(); + expect(false, 'Did not throw an error'); + } catch { + // This should throw an error + noop(); + } + }); + test('Pyton version info will be updated in notebook when a cell has been executed', async () => { const file = Uri.parse('file:///foo.ipynb'); const editor = createEditor(); await editor.load(baseFile, file); - expect(editor.contents).to.be.equal(baseFile); + expect(await editor.getContents()).to.be.equal(baseFile); // At the begining version info is NOT in the file (at least not the same as what we are using to run cells). - let contents = JSON.parse(editor.contents) as nbformat.INotebookContent; + let contents = JSON.parse(await editor.getContents()) as nbformat.INotebookContent; expect(contents.metadata!.language_info!.version).to.not.equal('10.11.12'); // When a cell is executed, then ensure we store the python version info in the notebook data. @@ -453,7 +471,7 @@ suite('Data Science - Native Editor', () => { }, 5_000, 'Timeout'); // Verify the version info is in the notbook. - contents = JSON.parse(editor.contents) as nbformat.INotebookContent; + contents = JSON.parse(await editor.getContents()) as nbformat.INotebookContent; expect(contents.metadata!.language_info!.version).to.equal('10.11.12'); }); @@ -471,7 +489,7 @@ suite('Data Science - Native Editor', () => { await editor.load('', file); // It should load with that value - expect(editor.contents).to.be.equal(baseFile); + expect(await editor.getContents()).to.be.equal(baseFile); expect(editor.cells).to.be.lengthOf(3); }); @@ -496,7 +514,7 @@ suite('Data Science - Native Editor', () => { const editor = createEditor(); await editor.load(baseFile, file); - expect(editor.contents).to.be.equal(baseFile); + expect(await editor.getContents()).to.be.equal(baseFile); // Make our call to actually export editor.onMessage(InteractiveWindowMessages.Export, editor.cells);