From 4dbdb5bd0dff995f7ecbb70ff6019223366dc498 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 31 Oct 2019 10:40:38 -0700 Subject: [PATCH 1/4] Prompt to open exported nb in editor --- .../interactiveWindowCommandListener.ts | 42 +++++++++------ ...eractiveWindowCommandListener.unit.test.ts | 52 +++++++++---------- 2 files changed, 51 insertions(+), 43 deletions(-) diff --git a/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts b/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts index 80dc782bdd0a..458f69092004 100644 --- a/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts +++ b/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts @@ -25,6 +25,7 @@ import { IInteractiveBase, IInteractiveWindowProvider, IJupyterExecution, + INotebookEditorProvider, INotebookExporter, INotebookImporter, INotebookServer, @@ -45,7 +46,8 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList @inject(IConfigurationService) private configuration: IConfigurationService, @inject(IStatusProvider) private statusProvider: IStatusProvider, @inject(INotebookImporter) private jupyterImporter: INotebookImporter, - @inject(IDataScienceErrorHandler) private dataScienceErrorHandler: IDataScienceErrorHandler + @inject(IDataScienceErrorHandler) private dataScienceErrorHandler: IDataScienceErrorHandler, + @inject(INotebookEditorProvider) protected ipynbProvider: INotebookEditorProvider ) { } @@ -159,7 +161,6 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList saveLabel: localize.DataScience.exportDialogTitle(), filters: filtersObject }); - await this.waitForStatus(async () => { if (uri) { let directoryChange; @@ -172,16 +173,19 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList await this.fileSystem.writeFile(uri.fsPath, JSON.stringify(notebook)); } }, localize.DataScience.exportingFormat(), file); - // When all done, show a notice that it completed. - const openQuestion = (await this.jupyterExecution.isSpawnSupported()) ? localize.DataScience.exportOpenQuestion() : undefined; if (uri && uri.fsPath) { - this.showInformationMessage(localize.DataScience.exportDialogComplete().format(uri.fsPath), openQuestion).then((str: string | undefined) => { - if (str === openQuestion) { - // If the user wants to, open the notebook they just generated. - this.jupyterExecution.spawnNotebook(uri.fsPath).ignoreErrors(); - } - }); + const openQuestion1 = localize.DataScience.exportOpenQuestion1(); + const openQuestion2 = (await this.jupyterExecution.isSpawnSupported()) ? localize.DataScience.exportOpenQuestion() : undefined; + const questions = [openQuestion1, ...(openQuestion2 ? [openQuestion2] : [])]; + const selection = await this.applicationShell.showInformationMessage(localize.DataScience.exportDialogComplete().format(uri.fsPath), ...questions); + if (selection === openQuestion1) { + await this.ipynbProvider.open(uri, await this.fileSystem.readFile(uri.fsPath)); + } + if (selection === openQuestion2) { + // If the user wants to, open the notebook they just generated. + this.jupyterExecution.spawnNotebook(uri.fsPath).ignoreErrors(); + } } } } @@ -220,13 +224,17 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList }); // When all done, show a notice that it completed. - const openQuestion = (await this.jupyterExecution.isSpawnSupported()) ? localize.DataScience.exportOpenQuestion() : undefined; - this.showInformationMessage(localize.DataScience.exportDialogComplete().format(output), openQuestion).then((str: string | undefined) => { - if (str === openQuestion && output) { - // If the user wants to, open the notebook they just generated. - this.jupyterExecution.spawnNotebook(output).ignoreErrors(); - } - }); + const openQuestion1 = localize.DataScience.exportOpenQuestion1(); + const openQuestion2 = (await this.jupyterExecution.isSpawnSupported()) ? localize.DataScience.exportOpenQuestion() : undefined; + const questions = [openQuestion1, ...(openQuestion2 ? [openQuestion2] : [])]; + const selection = await this.applicationShell.showInformationMessage(localize.DataScience.exportDialogComplete().format(output), ...questions); + if (selection === openQuestion1) { + await this.ipynbProvider.open(Uri.file(output), await this.fileSystem.readFile(output)); + } + if (selection === openQuestion2) { + // If the user wants to, open the notebook they just generated. + this.jupyterExecution.spawnNotebook(output).ignoreErrors(); + } return Uri.file(output); } diff --git a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts index 5309aa3a74c5..90a75b555b3c 100644 --- a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts +++ b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts @@ -3,22 +3,25 @@ 'use strict'; import { nbformat } from '@jupyterlab/coreutils/lib/nbformat'; import { assert } from 'chai'; -import { anything, instance, mock, when } from 'ts-mockito'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; import { Matcher } from 'ts-mockito/lib/matcher/type/Matcher'; import * as TypeMoq from 'typemoq'; import * as uuid from 'uuid/v4'; import { EventEmitter, Uri } from 'vscode'; import { ApplicationShell } from '../../client/common/application/applicationShell'; +import { IApplicationShell } from '../../client/common/application/types'; import { PythonSettings } from '../../client/common/configSettings'; import { ConfigurationService } from '../../client/common/configuration/service'; import { Logger } from '../../client/common/logger'; import { FileSystem } from '../../client/common/platform/fileSystem'; import { IFileSystem } from '../../client/common/platform/types'; import { IConfigurationService, IDisposable, ILogger } from '../../client/common/types'; +import * as localize from '../../client/common/utils/localize'; import { generateCells } from '../../client/datascience/cellFactory'; import { Commands } from '../../client/datascience/constants'; import { DataScienceErrorHandler } from '../../client/datascience/errorHandler/errorHandler'; +import { NativeEditorProvider } from '../../client/datascience/interactive-ipynb/nativeEditorProvider'; import { InteractiveWindowCommandListener } from '../../client/datascience/interactive-window/interactiveWindowCommandListener'; @@ -28,15 +31,15 @@ import { JupyterExporter } from '../../client/datascience/jupyter/jupyterExporte import { JupyterImporter } from '../../client/datascience/jupyter/jupyterImporter'; import { IInteractiveWindow, + IJupyterExecution, INotebook, + INotebookEditorProvider, INotebookServer } from '../../client/datascience/types'; import { InterpreterService } from '../../client/interpreter/interpreterService'; import { KnownSearchPathsForInterpreters } from '../../client/interpreter/locators/services/KnownPathsService'; import { ServiceContainer } from '../../client/ioc/container'; -import { noop } from '../core'; import { MockAutoSelectionService } from '../mocks/autoSelector'; -import * as vscodeMocks from '../vscode-mock'; import { MockCommandManager } from './mockCommandManager'; import { MockDocumentManager } from './mockDocumentManager'; import { MockStatusProvider } from './mockStatusProvider'; @@ -67,25 +70,17 @@ suite('Interactive window command listener', async () => { const dataScienceErrorHandler = mock(DataScienceErrorHandler); const notebookImporter = mock(JupyterImporter); const notebookExporter = mock(JupyterExporter); - const applicationShell = mock(ApplicationShell); - const jupyterExecution = mock(JupyterExecutionFactory); + let applicationShell: IApplicationShell; + let jupyterExecution: IJupyterExecution; const interactiveWindow = createTypeMoq('Interactive Window'); const documentManager = new MockDocumentManager(); const statusProvider = new MockStatusProvider(); const commandManager = new MockCommandManager(); + let notebookEditorProvider: INotebookEditorProvider; const server = createTypeMoq('jupyter server'); let lastFileContents: any; - suiteSetup(() => { - vscodeMocks.initialize(); - }); - suiteTeardown(() => { - noop(); - }); - - setup(() => { - noop(); - }); + setup(createCommandListener); teardown(() => { documentManager.activeTextEditor = undefined; @@ -111,6 +106,10 @@ suite('Interactive window command listener', async () => { } function createCommandListener(): InteractiveWindowCommandListener { + notebookEditorProvider = mock(NativeEditorProvider); + jupyterExecution = mock(JupyterExecutionFactory); + applicationShell = mock(ApplicationShell); + // Setup defaults when(interpreterService.onDidChangeInterpreter).thenReturn(dummyEvent.event); when(interpreterService.getInterpreterDetails(argThat(o => !o.includes || !o.includes('python')))).thenReject('Unknown interpreter'); @@ -190,9 +189,7 @@ suite('Interactive window command listener', async () => { } ); - if (jupyterExecution.isNotebookSupported) { - when(jupyterExecution.isNotebookSupported()).thenResolve(true); - } + when(jupyterExecution.isNotebookSupported()).thenResolve(true); documentManager.addDocument('#%%\r\nprint("code")', 'bar.ipynb'); @@ -211,34 +208,36 @@ suite('Interactive window command listener', async () => { instance(configService), statusProvider, instance(notebookImporter), - instance(dataScienceErrorHandler)); + instance(dataScienceErrorHandler), + instance(notebookEditorProvider)); result.register(commandManager); return result; } test('Import', async () => { - createCommandListener(); when(applicationShell.showOpenDialog(argThat(o => o.openLabel && o.openLabel.includes('Import')))).thenReturn(Promise.resolve([Uri.file('foo')])); await commandManager.executeCommand(Commands.ImportNotebook, undefined, undefined); assert.ok(documentManager.activeTextEditor, 'Imported file was not opened'); }); test('Import File', async () => { - createCommandListener(); await commandManager.executeCommand(Commands.ImportNotebook, Uri.file('bar.ipynb'), undefined); assert.ok(documentManager.activeTextEditor, 'Imported file was not opened'); }); test('Export File', async () => { - createCommandListener(); const doc = await documentManager.openTextDocument('bar.ipynb'); await documentManager.showTextDocument(doc); when(applicationShell.showSaveDialog(argThat(o => o.saveLabel && o.saveLabel.includes('Export')))).thenReturn(Promise.resolve(Uri.file('foo'))); + when(applicationShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve('moo')); + when(applicationShell.showInformationMessage(anything(), anything(), anything())).thenReturn(Promise.resolve('moo')); + when(jupyterExecution.isSpawnSupported()).thenResolve(true); await commandManager.executeCommand(Commands.ExportFileAsNotebook, Uri.file('bar.ipynb'), undefined); + assert.ok(lastFileContents, 'Export file was not written to'); + verify(applicationShell.showInformationMessage(anything(), localize.DataScience.exportOpenQuestion1(), localize.DataScience.exportOpenQuestion())).once(); }); test('Export File and output', async () => { - createCommandListener(); const doc = await documentManager.openTextDocument('bar.ipynb'); await documentManager.showTextDocument(doc); when(jupyterExecution.connectToNotebookServer(anything(), anything())).thenResolve(server.object); @@ -250,23 +249,24 @@ suite('Interactive window command listener', async () => { when(applicationShell.showSaveDialog(argThat(o => o.saveLabel && o.saveLabel.includes('Export')))).thenReturn(Promise.resolve(Uri.file('foo'))); when(applicationShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve('moo')); + when(applicationShell.showInformationMessage(anything(), anything(), anything())).thenReturn(Promise.resolve('moo')); + when(jupyterExecution.isSpawnSupported()).thenResolve(true); await commandManager.executeCommand(Commands.ExportFileAndOutputAsNotebook, Uri.file('bar.ipynb')); + assert.ok(lastFileContents, 'Export file was not written to'); + verify(applicationShell.showInformationMessage(anything(), localize.DataScience.exportOpenQuestion1(), localize.DataScience.exportOpenQuestion())).once(); }); test('Export skipped on no file', async () => { - createCommandListener(); when(applicationShell.showSaveDialog(argThat(o => o.saveLabel && o.saveLabel.includes('Export')))).thenReturn(Promise.resolve(Uri.file('foo'))); await commandManager.executeCommand(Commands.ExportFileAndOutputAsNotebook, Uri.file('bar.ipynb')); assert.notExists(lastFileContents, 'Export file was written to'); }); test('Export happens on no file', async () => { - createCommandListener(); const doc = await documentManager.openTextDocument('bar.ipynb'); await documentManager.showTextDocument(doc); when(applicationShell.showSaveDialog(argThat(o => o.saveLabel && o.saveLabel.includes('Export')))).thenReturn(Promise.resolve(Uri.file('foo'))); await commandManager.executeCommand(Commands.ExportFileAsNotebook, undefined, undefined); assert.ok(lastFileContents, 'Export file was not written to'); }); - }); From 5601a41f4bd5dded1dab08afbf8447f174bbd9c2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 31 Oct 2019 10:41:40 -0700 Subject: [PATCH 2/4] News entry file --- news/1 Enhancements/8078.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/1 Enhancements/8078.md diff --git a/news/1 Enhancements/8078.md b/news/1 Enhancements/8078.md new file mode 100644 index 000000000000..46d527d74a79 --- /dev/null +++ b/news/1 Enhancements/8078.md @@ -0,0 +1 @@ +Prompt to open exported `Notebook` in the `Notebook Editor`. From 0ec1a1b7f1625d327c5af8535e0487815a90c166 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 1 Nov 2019 09:52:36 -0700 Subject: [PATCH 3/4] Fix tests --- .../interactiveWindowCommandListener.unit.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts index 90a75b555b3c..743dc943aa80 100644 --- a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts +++ b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts @@ -80,8 +80,6 @@ suite('Interactive window command listener', async () => { const server = createTypeMoq('jupyter server'); let lastFileContents: any; - setup(createCommandListener); - teardown(() => { documentManager.activeTextEditor = undefined; lastFileContents = undefined; @@ -216,15 +214,18 @@ suite('Interactive window command listener', async () => { } test('Import', async () => { + createCommandListener(); when(applicationShell.showOpenDialog(argThat(o => o.openLabel && o.openLabel.includes('Import')))).thenReturn(Promise.resolve([Uri.file('foo')])); await commandManager.executeCommand(Commands.ImportNotebook, undefined, undefined); assert.ok(documentManager.activeTextEditor, 'Imported file was not opened'); }); test('Import File', async () => { + createCommandListener(); await commandManager.executeCommand(Commands.ImportNotebook, Uri.file('bar.ipynb'), undefined); assert.ok(documentManager.activeTextEditor, 'Imported file was not opened'); }); test('Export File', async () => { + createCommandListener(); const doc = await documentManager.openTextDocument('bar.ipynb'); await documentManager.showTextDocument(doc); when(applicationShell.showSaveDialog(argThat(o => o.saveLabel && o.saveLabel.includes('Export')))).thenReturn(Promise.resolve(Uri.file('foo'))); @@ -238,6 +239,7 @@ suite('Interactive window command listener', async () => { verify(applicationShell.showInformationMessage(anything(), localize.DataScience.exportOpenQuestion1(), localize.DataScience.exportOpenQuestion())).once(); }); test('Export File and output', async () => { + createCommandListener(); const doc = await documentManager.openTextDocument('bar.ipynb'); await documentManager.showTextDocument(doc); when(jupyterExecution.connectToNotebookServer(anything(), anything())).thenResolve(server.object); @@ -258,11 +260,13 @@ suite('Interactive window command listener', async () => { verify(applicationShell.showInformationMessage(anything(), localize.DataScience.exportOpenQuestion1(), localize.DataScience.exportOpenQuestion())).once(); }); test('Export skipped on no file', async () => { + createCommandListener(); when(applicationShell.showSaveDialog(argThat(o => o.saveLabel && o.saveLabel.includes('Export')))).thenReturn(Promise.resolve(Uri.file('foo'))); await commandManager.executeCommand(Commands.ExportFileAndOutputAsNotebook, Uri.file('bar.ipynb')); assert.notExists(lastFileContents, 'Export file was written to'); }); test('Export happens on no file', async () => { + createCommandListener(); const doc = await documentManager.openTextDocument('bar.ipynb'); await documentManager.showTextDocument(doc); when(applicationShell.showSaveDialog(argThat(o => o.saveLabel && o.saveLabel.includes('Export')))).thenReturn(Promise.resolve(Uri.file('foo'))); From c702d9d2008a714f2acf41201cad6b9eece67780 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 1 Nov 2019 10:40:59 -0700 Subject: [PATCH 4/4] Fixes to how tests are run --- build/.mocha.unittests.ts.opts | 4 ++-- src/test/mocks/vsc/telemetryReporter.ts | 3 +-- src/test/vscode-mock.ts | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/build/.mocha.unittests.ts.opts b/build/.mocha.unittests.ts.opts index f6672aed1db6..9d97e8ef972a 100644 --- a/build/.mocha.unittests.ts.opts +++ b/build/.mocha.unittests.ts.opts @@ -1,8 +1,8 @@ --require ts-node/register ---require out/test/unittests.js +--require src/test/unittests.ts --reporter mocha-multi-reporters --reporter-options configFile=build/.mocha-multi-reporters.config --ui tdd --recursive --colors -./src/test/**/*.unit.test.ts \ No newline at end of file +./src/test/**/*.unit.test.ts diff --git a/src/test/mocks/vsc/telemetryReporter.ts b/src/test/mocks/vsc/telemetryReporter.ts index 9b5ef94178cf..d0250eb1cf5e 100644 --- a/src/test/mocks/vsc/telemetryReporter.ts +++ b/src/test/mocks/vsc/telemetryReporter.ts @@ -4,8 +4,7 @@ 'use strict'; // tslint:disable:all -import * as telemetry from 'vscode-extension-telemetry'; -export class vscMockTelemetryReporter implements telemetry.default { +export class vscMockTelemetryReporter { constructor() { // } diff --git a/src/test/vscode-mock.ts b/src/test/vscode-mock.ts index fb8b870c919f..93798c7697a5 100644 --- a/src/test/vscode-mock.ts +++ b/src/test/vscode-mock.ts @@ -38,7 +38,7 @@ export function initialize() { return mockedVSCode; } if (request === 'vscode-extension-telemetry') { - return { default: vscMockTelemetryReporter }; + return { default: vscMockTelemetryReporter as any }; } // less files need to be in import statements to be converted to css // But we don't want to try to load them in the mock vscode