Don't look for a jupyter interpreter when creating blank notebook (#8596)

* Creating a new blank notebook should not require a search for jupyter.

* Fix bug caused by other fix.

Author: rchiodo

news/2 Fixes/8481.md

@@ -0,0 +1 @@
+Creating a new blank notebook should not require a search for jupyter.

news/2 Fixes/8594.md

@@ -0,0 +1 @@
+Typing 'z' in a cell causes the cell to disappear.

src/client/datascience/interactive-ipynb/nativeEditor.ts

@@ -164,7 +164,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
         return this.close();
     }
 
-    public get contents(): string {
+    public getContents(): Promise<string> {
         return this.generateNotebookContent(this.visibleCells);
     }
 
@@ -470,31 +470,18 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
      * @memberof NativeEditor
      */
     private async updateVersionInfoInNotebook(): Promise<void> {
+        // 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) {
             this.notebookJson.metadata.language_info.version = `${usableInterpreter.version.major}.${usableInterpreter.version.minor}.${usableInterpreter.version.patch}`;
         }
     }
 
-    private async loadContents(contents: string | undefined, forceDirty: boolean): Promise<void> {
-        // 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<void> {
+        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.
@@ -522,6 +509,26 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
                 metadata: metadata
             };
         }
+    }
+
+    private async loadContents(contents: string | undefined, forceDirty: boolean): Promise<void> {
+        // 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)[] : [];
@@ -743,9 +750,12 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
     }
 
     private async setDirty(): Promise<void> {
-        // 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) {
@@ -827,7 +837,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<string> {
+        // Make sure we have some
+        await this.ensureNotebookJson();
+
         // Reuse our original json except for the cells.
         const json = {
             ...(this.notebookJson as nbformat.INotebookContent),
@@ -863,7 +876,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;

src/datascience-ui/native-editor/nativeEditor.tsx

@@ -367,14 +367,16 @@ export class NativeEditor extends React.Component<INativeEditorProps, IMainState
             }
             case 'z':
             case 'Z':
-                if (event.shiftKey && !event.ctrlKey && !event.altKey && this.stateController.canRedo()) {
-                    event.stopPropagation();
-                    this.stateController.redo();
-                    this.stateController.sendCommand(NativeCommandType.Redo, 'keyboard');
-                } else if (!event.shiftKey && !event.altKey && !event.ctrlKey && this.stateController.canUndo()) {
-                    event.stopPropagation();
-                    this.stateController.undo();
-                    this.stateController.sendCommand(NativeCommandType.Undo, 'keyboard');
+                if (this.stateController.getState().focusedCellId === undefined) {
+                    if (event.shiftKey && !event.ctrlKey && !event.altKey && this.stateController.canRedo()) {
+                        event.stopPropagation();
+                        this.stateController.redo();
+                        this.stateController.sendCommand(NativeCommandType.Redo, 'keyboard');
+                    } else if (!event.shiftKey && !event.altKey && !event.ctrlKey && this.stateController.canUndo()) {
+                        event.stopPropagation();
+                        this.stateController.undo();
+                        this.stateController.sendCommand(NativeCommandType.Undo, 'keyboard');
+                    }
                 }
                 break;
             default:

src/test/datascience/interactive-ipynb/nativeEditor.unit.test.ts

@@ -263,7 +263,7 @@ suite('Data Science - Native Editor', () => {
     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);