From c663c0bae3149c0c07d47a87a8aa8ba786dede5c Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 8 Jul 2020 15:48:55 -0400 Subject: [PATCH 1/5] changed default export file name from interactice window --- src/client/common/application/commands.ts | 6 ++-- .../datascience/commands/exportCommands.ts | 29 ++++++++++++------- .../datascience/export/exportManager.ts | 8 +++-- .../export/exportManagerDependencyChecker.ts | 8 +++-- .../export/exportManagerFileOpener.ts | 8 +++-- .../export/exportManagerFilePicker.ts | 13 +++++++-- src/client/datascience/export/types.ts | 4 +-- .../interactive-ipynb/nativeEditor.ts | 2 +- .../interactive-window/interactiveWindow.ts | 2 +- .../nativeEditor.functional.test.tsx | 2 +- 10 files changed, 55 insertions(+), 27 deletions(-) diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index d18b5cf737a5..6dd8052485ac 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -173,9 +173,9 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu [DSCommands.ScrollToCell]: [string, string]; [DSCommands.ViewJupyterOutput]: []; [DSCommands.ExportAsPythonScript]: [INotebookModel]; - [DSCommands.ExportToHTML]: [INotebookModel]; - [DSCommands.ExportToPDF]: [INotebookModel]; - [DSCommands.Export]: [INotebookModel]; + [DSCommands.ExportToHTML]: [INotebookModel, string | undefined]; + [DSCommands.ExportToPDF]: [INotebookModel, string | undefined]; + [DSCommands.Export]: [INotebookModel, string | undefined]; [DSCommands.SwitchJupyterKernel]: [INotebook | undefined, 'raw' | 'jupyter']; [DSCommands.SelectJupyterCommandLine]: [undefined | Uri]; [DSCommands.SaveNotebookNonCustomEditor]: [Uri]; diff --git a/src/client/datascience/commands/exportCommands.ts b/src/client/datascience/commands/exportCommands.ts index c82c4d2b58cb..f28cd6e8a940 100644 --- a/src/client/datascience/commands/exportCommands.ts +++ b/src/client/datascience/commands/exportCommands.ts @@ -31,9 +31,13 @@ export class ExportCommands implements IDisposable { ) {} public register() { this.registerCommand(Commands.ExportAsPythonScript, (model) => this.export(model, ExportFormat.python)); - this.registerCommand(Commands.ExportToHTML, (model) => this.export(model, ExportFormat.html)); - this.registerCommand(Commands.ExportToPDF, (model) => this.export(model, ExportFormat.pdf)); - this.registerCommand(Commands.Export, (model) => this.export(model)); + this.registerCommand(Commands.ExportToHTML, (model, fileName?) => + this.export(model, ExportFormat.html, fileName) + ); + this.registerCommand(Commands.ExportToPDF, (model, fileName?) => + this.export(model, ExportFormat.pdf, fileName) + ); + this.registerCommand(Commands.Export, (model, fileName?) => this.export(model, undefined, fileName)); } public dispose() { @@ -49,7 +53,7 @@ export class ExportCommands implements IDisposable { this.disposables.push(disposable); } - private async export(model: INotebookModel, exportMethod?: ExportFormat) { + private async export(model: INotebookModel, exportMethod?: ExportFormat, fileName?: string) { if (!model) { // if no model was passed then this was called from the command pallete, // so we need to get the active editor @@ -65,11 +69,11 @@ export class ExportCommands implements IDisposable { } if (exportMethod) { - await this.exportManager.export(exportMethod, model); + await this.exportManager.export(exportMethod, model, fileName); } else { // if we don't have an export method we need to ask for one and display the // quickpick menu - const pickedItem = await this.showExportQuickPickMenu(model).then((item) => item); + const pickedItem = await this.showExportQuickPickMenu(model, fileName).then((item) => item); if (pickedItem !== undefined) { pickedItem.handler(); } else { @@ -78,7 +82,7 @@ export class ExportCommands implements IDisposable { } } - private getExportQuickPickItems(model: INotebookModel): IExportQuickPickItem[] { + private getExportQuickPickItems(model: INotebookModel, fileName?: string): IExportQuickPickItem[] { return [ { label: DataScience.exportPythonQuickPickLabel(), @@ -97,7 +101,7 @@ export class ExportCommands implements IDisposable { sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, { format: ExportFormat.html }); - this.commandManager.executeCommand(Commands.ExportToHTML, model); + this.commandManager.executeCommand(Commands.ExportToHTML, model, fileName); } }, { @@ -107,14 +111,17 @@ export class ExportCommands implements IDisposable { sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, { format: ExportFormat.pdf }); - this.commandManager.executeCommand(Commands.ExportToPDF, model); + this.commandManager.executeCommand(Commands.ExportToPDF, model, fileName); } } ]; } - private async showExportQuickPickMenu(model: INotebookModel): Promise { - const items = this.getExportQuickPickItems(model); + private async showExportQuickPickMenu( + model: INotebookModel, + fileName?: string + ): Promise { + const items = this.getExportQuickPickItems(model, fileName); const options: QuickPickOptions = { ignoreFocusOut: false, diff --git a/src/client/datascience/export/exportManager.ts b/src/client/datascience/export/exportManager.ts index 41eab797fe66..9aec833e9b15 100644 --- a/src/client/datascience/export/exportManager.ts +++ b/src/client/datascience/export/exportManager.ts @@ -19,11 +19,15 @@ export class ExportManager implements IExportManager { @inject(ExportUtil) private readonly exportUtil: ExportUtil ) {} - public async export(format: ExportFormat, model: INotebookModel): Promise { + public async export( + format: ExportFormat, + model: INotebookModel, + defaultFileName?: string + ): Promise { let target; if (format !== ExportFormat.python) { - target = await this.filePicker.getExportFileLocation(format, model.file); + target = await this.filePicker.getExportFileLocation(format, model.file, defaultFileName); if (!target) { return; } diff --git a/src/client/datascience/export/exportManagerDependencyChecker.ts b/src/client/datascience/export/exportManagerDependencyChecker.ts index 8d3d0b99a6f7..daf8e44a453a 100644 --- a/src/client/datascience/export/exportManagerDependencyChecker.ts +++ b/src/client/datascience/export/exportManagerDependencyChecker.ts @@ -16,7 +16,11 @@ export class ExportManagerDependencyChecker implements IExportManager { @inject(ProgressReporter) private readonly progressReporter: ProgressReporter ) {} - public async export(format: ExportFormat, model: INotebookModel): Promise { + public async export( + format: ExportFormat, + model: INotebookModel, + defaultFileName?: string + ): Promise { // Before we try the import, see if we don't support it, if we don't give a chance to install dependencies const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`); try { @@ -29,6 +33,6 @@ export class ExportManagerDependencyChecker implements IExportManager { } finally { reporter.dispose(); } - return this.manager.export(format, model); + return this.manager.export(format, model, defaultFileName); } } diff --git a/src/client/datascience/export/exportManagerFileOpener.ts b/src/client/datascience/export/exportManagerFileOpener.ts index afc9c9310e43..5373d1924260 100644 --- a/src/client/datascience/export/exportManagerFileOpener.ts +++ b/src/client/datascience/export/exportManagerFileOpener.ts @@ -22,10 +22,14 @@ export class ExportManagerFileOpener implements IExportManager { @inject(IBrowserService) private readonly browserService: IBrowserService ) {} - public async export(format: ExportFormat, model: INotebookModel): Promise { + public async export( + format: ExportFormat, + model: INotebookModel, + defaultFileName?: string + ): Promise { let uri: Uri | undefined; try { - uri = await this.manager.export(format, model); + uri = await this.manager.export(format, model, defaultFileName); } catch (e) { let msg = e; traceError('Export failed', e); diff --git a/src/client/datascience/export/exportManagerFilePicker.ts b/src/client/datascience/export/exportManagerFilePicker.ts index 6dbe8ca58458..5b73148d230d 100644 --- a/src/client/datascience/export/exportManagerFilePicker.ts +++ b/src/client/datascience/export/exportManagerFilePicker.ts @@ -20,7 +20,11 @@ export class ExportManagerFilePicker implements IExportManagerFilePicker { @inject(IMemento) @named(WORKSPACE_MEMENTO) private workspaceStorage: Memento ) {} - public async getExportFileLocation(format: ExportFormat, source: Uri): Promise { + public async getExportFileLocation( + format: ExportFormat, + source: Uri, + defaultFileName?: string + ): Promise { // map each export method to a set of file extensions let fileExtensions; switch (format) { @@ -41,7 +45,12 @@ export class ExportManagerFilePicker implements IExportManagerFilePicker { } const notebookFileName = path.basename(source.fsPath, path.extname(source.fsPath)); - const dialogUri = Uri.file(path.join(this.getLastFileSaveLocation().fsPath, notebookFileName)); + let dialogUri: Uri; + if (defaultFileName) { + dialogUri = Uri.file(defaultFileName); + } else { + dialogUri = Uri.file(path.join(this.getLastFileSaveLocation().fsPath, notebookFileName)); + } const options: SaveDialogOptions = { defaultUri: dialogUri, saveLabel: 'Export', diff --git a/src/client/datascience/export/types.ts b/src/client/datascience/export/types.ts index a8de0fa1cf95..cb632e57ebc1 100644 --- a/src/client/datascience/export/types.ts +++ b/src/client/datascience/export/types.ts @@ -9,7 +9,7 @@ export enum ExportFormat { export const IExportManager = Symbol('IExportManager'); export interface IExportManager { - export(format: ExportFormat, model: INotebookModel): Promise; + export(format: ExportFormat, model: INotebookModel, defaultFileName?: string): Promise; } export const IExport = Symbol('IExport'); @@ -19,5 +19,5 @@ export interface IExport { export const IExportManagerFilePicker = Symbol('IExportManagerFilePicker'); export interface IExportManagerFilePicker { - getExportFileLocation(format: ExportFormat, source: Uri): Promise; + getExportFileLocation(format: ExportFormat, source: Uri, defaultFileName?: string): Promise; } diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index ef299a60b1ed..9b7c7eaa7815 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -730,7 +730,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { if (!activeEditor || !activeEditor.model) { return; } - this.commandManager.executeCommand(Commands.Export, activeEditor.model); + this.commandManager.executeCommand(Commands.Export, activeEditor.model, undefined); } private logNativeCommand(args: INativeCommand) { diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index b5e8f6536d34..baf888602b07 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -437,7 +437,7 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi this.stopProgress(); } if (model) { - this.commandManager.executeCommand(Commands.Export, model); + this.commandManager.executeCommand(Commands.Export, model, this.lastFile); } } diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 87b3e8a1116e..659602c257b4 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -630,7 +630,7 @@ df.head()`; const model = editor!.model!; ioc.serviceManager.rebindInstance(ICommandManager, commandManager.object); commandManager - .setup((cmd) => cmd.executeCommand(Commands.Export, model)) + .setup((cmd) => cmd.executeCommand(Commands.Export, model, undefined)) .returns(() => { commandFired.resolve(); return Promise.resolve(); From 20ebb1ee7b2b202f5cc3a42f271c35b15ab7506d Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 8 Jul 2020 16:11:56 -0400 Subject: [PATCH 2/5] fixed naming issue --- .../datascience/export/exportManagerFilePicker.ts | 12 +++++------- .../interactive-window/interactiveWindow.ts | 6 +++++- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/client/datascience/export/exportManagerFilePicker.ts b/src/client/datascience/export/exportManagerFilePicker.ts index 5b73148d230d..afb3e7ffb567 100644 --- a/src/client/datascience/export/exportManagerFilePicker.ts +++ b/src/client/datascience/export/exportManagerFilePicker.ts @@ -44,13 +44,11 @@ export class ExportManagerFilePicker implements IExportManagerFilePicker { return; } - const notebookFileName = path.basename(source.fsPath, path.extname(source.fsPath)); - let dialogUri: Uri; - if (defaultFileName) { - dialogUri = Uri.file(defaultFileName); - } else { - dialogUri = Uri.file(path.join(this.getLastFileSaveLocation().fsPath, notebookFileName)); - } + const notebookFileName = defaultFileName + ? defaultFileName + : path.basename(source.fsPath, path.extname(source.fsPath)); + + const dialogUri = Uri.file(path.join(this.getLastFileSaveLocation().fsPath, notebookFileName)); const options: SaveDialogOptions = { defaultUri: dialogUri, saveLabel: 'Export', diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index baf888602b07..ca5a209ecde1 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -437,7 +437,11 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi this.stopProgress(); } if (model) { - this.commandManager.executeCommand(Commands.Export, model, this.lastFile); + let defaultFileName; + if (this.lastFile) { + defaultFileName = path.basename(this.lastFile, path.extname(this.lastFile)); + } + this.commandManager.executeCommand(Commands.Export, model, defaultFileName); } } From 3f2ff99c87c8edea8899675952c4c8de3bd1cd1f Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 8 Jul 2020 16:34:37 -0400 Subject: [PATCH 3/5] refactor --- .../datascience/commands/exportCommands.ts | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/client/datascience/commands/exportCommands.ts b/src/client/datascience/commands/exportCommands.ts index f28cd6e8a940..d609860657c4 100644 --- a/src/client/datascience/commands/exportCommands.ts +++ b/src/client/datascience/commands/exportCommands.ts @@ -31,13 +31,15 @@ export class ExportCommands implements IDisposable { ) {} public register() { this.registerCommand(Commands.ExportAsPythonScript, (model) => this.export(model, ExportFormat.python)); - this.registerCommand(Commands.ExportToHTML, (model, fileName?) => - this.export(model, ExportFormat.html, fileName) + this.registerCommand(Commands.ExportToHTML, (model, defaultFileName?) => + this.export(model, ExportFormat.html, defaultFileName) ); - this.registerCommand(Commands.ExportToPDF, (model, fileName?) => - this.export(model, ExportFormat.pdf, fileName) + this.registerCommand(Commands.ExportToPDF, (model, defaultFileName?) => + this.export(model, ExportFormat.pdf, defaultFileName) + ); + this.registerCommand(Commands.Export, (model, defaultFileName?) => + this.export(model, undefined, defaultFileName) ); - this.registerCommand(Commands.Export, (model, fileName?) => this.export(model, undefined, fileName)); } public dispose() { @@ -53,7 +55,7 @@ export class ExportCommands implements IDisposable { this.disposables.push(disposable); } - private async export(model: INotebookModel, exportMethod?: ExportFormat, fileName?: string) { + private async export(model: INotebookModel, exportMethod?: ExportFormat, defaultFileName?: string) { if (!model) { // if no model was passed then this was called from the command pallete, // so we need to get the active editor @@ -69,11 +71,11 @@ export class ExportCommands implements IDisposable { } if (exportMethod) { - await this.exportManager.export(exportMethod, model, fileName); + await this.exportManager.export(exportMethod, model, defaultFileName); } else { // if we don't have an export method we need to ask for one and display the // quickpick menu - const pickedItem = await this.showExportQuickPickMenu(model, fileName).then((item) => item); + const pickedItem = await this.showExportQuickPickMenu(model, defaultFileName).then((item) => item); if (pickedItem !== undefined) { pickedItem.handler(); } else { @@ -82,7 +84,7 @@ export class ExportCommands implements IDisposable { } } - private getExportQuickPickItems(model: INotebookModel, fileName?: string): IExportQuickPickItem[] { + private getExportQuickPickItems(model: INotebookModel, defaultFileName?: string): IExportQuickPickItem[] { return [ { label: DataScience.exportPythonQuickPickLabel(), @@ -101,7 +103,7 @@ export class ExportCommands implements IDisposable { sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, { format: ExportFormat.html }); - this.commandManager.executeCommand(Commands.ExportToHTML, model, fileName); + this.commandManager.executeCommand(Commands.ExportToHTML, model, defaultFileName); } }, { @@ -111,7 +113,7 @@ export class ExportCommands implements IDisposable { sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, { format: ExportFormat.pdf }); - this.commandManager.executeCommand(Commands.ExportToPDF, model, fileName); + this.commandManager.executeCommand(Commands.ExportToPDF, model, defaultFileName); } } ]; @@ -119,9 +121,9 @@ export class ExportCommands implements IDisposable { private async showExportQuickPickMenu( model: INotebookModel, - fileName?: string + defaultFileName?: string ): Promise { - const items = this.getExportQuickPickItems(model, fileName); + const items = this.getExportQuickPickItems(model, defaultFileName); const options: QuickPickOptions = { ignoreFocusOut: false, From 30b323b8651649178d3c2376a7fee8fdb6562d35 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 8 Jul 2020 16:59:15 -0400 Subject: [PATCH 4/5] fixed tests --- .../export/exportManagerFileOpener.unit.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/datascience/export/exportManagerFileOpener.unit.test.ts b/src/test/datascience/export/exportManagerFileOpener.unit.test.ts index 87dadc8a5592..174a9921141a 100644 --- a/src/test/datascience/export/exportManagerFileOpener.unit.test.ts +++ b/src/test/datascience/export/exportManagerFileOpener.unit.test.ts @@ -48,20 +48,20 @@ suite('Data Science - Export File Opener', () => { test('No file is opened if nothing is exported', async () => { when(exporter.export(anything(), anything())).thenResolve(); - await fileOpener.export(ExportFormat.python, model); + await fileOpener.export(ExportFormat.python, model, undefined); verify(documentManager.showTextDocument(anything())).never(); }); test('Python File is opened if exported', async () => { const uri = Uri.file('test.python'); - when(exporter.export(anything(), anything())).thenResolve(uri); - await fileOpener.export(ExportFormat.python, model); + when(exporter.export(anything(), anything(), anything())).thenResolve(uri); + await fileOpener.export(ExportFormat.python, model, undefined); verify(documentManager.showTextDocument(anything())).once(); }); test('HTML File opened if yes button pressed', async () => { const uri = Uri.file('test.html'); - when(exporter.export(anything(), anything())).thenResolve(uri); + when(exporter.export(anything(), anything(), anything())).thenResolve(uri); when(applicationShell.showInformationMessage(anything(), anything(), anything())).thenReturn( Promise.resolve(getLocString('DataScience.openExportFileYes', 'Yes')) ); @@ -82,14 +82,14 @@ suite('Data Science - Export File Opener', () => { verify(browserService.launch(anything())).never(); }); test('Exporting to PDF displays message if operation fails', async () => { - when(exporter.export(anything(), anything())).thenThrow(new Error('Export failed...')); + when(exporter.export(anything(), anything(), anything())).thenThrow(new Error('Export failed...')); when(applicationShell.showErrorMessage(anything())).thenResolve(); await fileOpener.export(ExportFormat.pdf, model); verify(applicationShell.showErrorMessage(anything())).once(); }); test('PDF File opened if yes button pressed', async () => { const uri = Uri.file('test.pdf'); - when(exporter.export(anything(), anything())).thenResolve(uri); + when(exporter.export(anything(), anything(), anything())).thenResolve(uri); when(applicationShell.showInformationMessage(anything(), anything(), anything())).thenReturn( Promise.resolve(getLocString('DataScience.openExportFileYes', 'Yes')) ); From 0348ddde6f0157af9d64ff5a1e0dfff4f174bbe7 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 8 Jul 2020 17:44:20 -0400 Subject: [PATCH 5/5] hopefully fixed test --- src/test/datascience/export/exportManager.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/datascience/export/exportManager.test.ts b/src/test/datascience/export/exportManager.test.ts index 862f3e06192c..359fa59d6ba5 100644 --- a/src/test/datascience/export/exportManager.test.ts +++ b/src/test/datascience/export/exportManager.test.ts @@ -33,7 +33,7 @@ suite('Data Science - Export Manager', () => { exportPdf = mock(); // tslint:disable-next-line: no-any - when(filePicker.getExportFileLocation(anything(), anything())).thenReturn( + when(filePicker.getExportFileLocation(anything(), anything(), anything())).thenReturn( Promise.resolve(Uri.file('test.pdf')) ); // tslint:disable-next-line: no-empty