From 8f685e9fb1d5179fc9357e693e3c2a0474e2643b Mon Sep 17 00:00:00 2001 From: rchiodo Date: Tue, 1 Dec 2020 16:45:46 -0800 Subject: [PATCH 1/5] Fix export to use the source file for the directory it wants to open --- package.nls.json | 1 + src/client/common/application/commands.ts | 8 +- src/client/common/utils/localize.ts | 1 + .../datascience/commands/exportCommands.ts | 68 ++++++++++----- src/client/datascience/export/exportDialog.ts | 70 ++++++++++++++++ .../datascience/export/exportManager.ts | 33 +++----- .../export/exportManagerFilePicker.ts | 83 ------------------- src/client/datascience/export/exportUtil.ts | 27 +----- src/client/datascience/export/types.ts | 12 +-- .../interactive-ipynb/nativeEditor.ts | 3 +- .../interactive-window/interactiveWindow.ts | 62 +++++++------- .../interactiveWindowCommandListener.ts | 35 ++------ .../interactiveWindowProvider.ts | 6 +- .../datascience/jupyter/jupyterUtils.ts | 2 +- src/client/datascience/serviceRegistry.ts | 6 +- .../datascience/dataScienceIocContainer.ts | 6 +- .../export/exportManager.vscode.test.ts | 22 +++-- ...eractiveWindowCommandListener.unit.test.ts | 27 ++---- .../nativeEditor.functional.test.tsx | 3 +- 19 files changed, 215 insertions(+), 260 deletions(-) create mode 100644 src/client/datascience/export/exportDialog.ts delete mode 100644 src/client/datascience/export/exportManagerFilePicker.ts diff --git a/package.nls.json b/package.nls.json index fabcfc2d168..640846bce4c 100644 --- a/package.nls.json +++ b/package.nls.json @@ -384,6 +384,7 @@ "DataScience.dirtyNotebookDialogFilter": "Jupyter Notebooks", "DataScience.exportAsPythonFileTooltip": "Convert and save to a python script", "DataScience.exportAsPythonFileTitle": "Save as Python File", + "DataScience.exportButtonTitle": "Export", "DataScience.runCell": "Run cell", "DataScience.deleteCell": "Delete cell", "DataScience.moveCellUp": "Move cell up", diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index d98173747e9..f2f932485cb 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -125,10 +125,10 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu [DSCommands.GotoPrevCellInFile]: []; [DSCommands.ScrollToCell]: [Uri, string]; [DSCommands.ViewJupyterOutput]: []; - [DSCommands.ExportAsPythonScript]: [INotebookModel | undefined, PythonEnvironment | undefined]; - [DSCommands.ExportToHTML]: [INotebookModel | undefined, string | undefined, PythonEnvironment | undefined]; - [DSCommands.ExportToPDF]: [INotebookModel | undefined, string | undefined, PythonEnvironment | undefined]; - [DSCommands.Export]: [INotebookModel | undefined, string | undefined, PythonEnvironment | undefined]; + [DSCommands.ExportAsPythonScript]: [string | undefined, Uri | undefined, PythonEnvironment | undefined]; + [DSCommands.ExportToHTML]: [string | undefined, Uri | undefined, string | undefined, PythonEnvironment | undefined]; + [DSCommands.ExportToPDF]: [string | undefined, Uri | undefined, string | undefined, PythonEnvironment | undefined]; + [DSCommands.Export]: [string | undefined, Uri | undefined, string | undefined, PythonEnvironment | undefined]; [DSCommands.NativeNotebookExport]: [Uri]; [DSCommands.SetJupyterKernel]: [KernelConnectionMetadata, Uri, undefined | Uri]; [DSCommands.SwitchJupyterKernel]: [ISwitchKernelOptions | undefined]; diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 32d4d90bca6..c181fe90aa6 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -719,6 +719,7 @@ export namespace DataScience { ); export const notebookExportAs = localize('DataScience.notebookExportAs', 'Export As'); export const exportAsPythonFileTitle = localize('DataScience.exportAsPythonFileTitle', 'Save As Python File'); + export const exportButtonTitle = localize('DataScience.exportButtonTitle', 'Export'); export const exportAsQuickPickPlaceholder = localize('DataScience.exportAsQuickPickPlaceholder', 'Export As...'); export const openExportedFileMessage = localize( 'DataScience.openExportedFileMessage', diff --git a/src/client/datascience/commands/exportCommands.ts b/src/client/datascience/commands/exportCommands.ts index 7bfe9b2cd05..298154bfbbb 100644 --- a/src/client/datascience/commands/exportCommands.ts +++ b/src/client/datascience/commands/exportCommands.ts @@ -3,6 +3,7 @@ 'use strict'; +import { nbformat } from '@jupyterlab/coreutils'; import { inject, injectable } from 'inversify'; import { QuickPickItem, QuickPickOptions, Uri } from 'vscode'; import { getLocString } from '../../../datascience-ui/react-common/locReactSide'; @@ -18,7 +19,7 @@ import { sendTelemetryEvent } from '../../telemetry'; import { Commands, Telemetry } from '../constants'; import { ExportManager } from '../export/exportManager'; import { ExportFormat, IExportManager } from '../export/types'; -import { INotebookEditorProvider, INotebookModel } from '../types'; +import { INotebookEditorProvider } from '../types'; interface IExportQuickPickItem extends QuickPickItem { handler(): void; @@ -35,17 +36,17 @@ export class ExportCommands implements IDisposable { @inject(IFileSystem) private readonly fs: IFileSystem ) {} public register() { - this.registerCommand(Commands.ExportAsPythonScript, (model, interpreter?) => - this.export(model, ExportFormat.python, undefined, interpreter) + this.registerCommand(Commands.ExportAsPythonScript, (contents, file, interpreter?) => + this.export(contents, file, ExportFormat.python, undefined, interpreter) ); - this.registerCommand(Commands.ExportToHTML, (model, defaultFileName?, interpreter?) => - this.export(model, ExportFormat.html, defaultFileName, interpreter) + this.registerCommand(Commands.ExportToHTML, (contents, file, defaultFileName?, interpreter?) => + this.export(contents, file, ExportFormat.html, defaultFileName, interpreter) ); - this.registerCommand(Commands.ExportToPDF, (model, defaultFileName?, interpreter?) => - this.export(model, ExportFormat.pdf, defaultFileName, interpreter) + this.registerCommand(Commands.ExportToPDF, (contents, file, defaultFileName?, interpreter?) => + this.export(contents, file, ExportFormat.pdf, defaultFileName, interpreter) ); - this.registerCommand(Commands.Export, (model, defaultFileName?, interpreter?) => - this.export(model, undefined, defaultFileName, interpreter) + this.registerCommand(Commands.Export, (contents, file, defaultFileName?, interpreter?) => + this.export(contents, file, undefined, defaultFileName, interpreter) ); this.registerCommand(Commands.NativeNotebookExport, (uri) => this.nativeNotebookExport(uri)); } @@ -69,26 +70,28 @@ export class ExportCommands implements IDisposable { if (editor && editor.model) { const interpreter = editor.notebook?.getMatchingInterpreter(); - return this.export(editor.model, undefined, undefined, interpreter); + return this.export(editor.model.getContent(), editor.model.file, undefined, undefined, interpreter); } else { return this.export(undefined, undefined, undefined, undefined); } } private async export( - model?: INotebookModel, + contents?: string, + source?: Uri, exportMethod?: ExportFormat, defaultFileName?: string, interpreter?: PythonEnvironment ) { - if (!model) { - // if no model was passed then this was called from the command palette, + if (!contents || !source) { + // if no contents was passed then this was called from the command palette, // so we need to get the active editor const activeEditor = this.notebookProvider.activeEditor; if (!activeEditor || !activeEditor.model) { return; } - model = activeEditor.model; + contents = contents ? contents : activeEditor.model.getContent(); + source = source ? source : activeEditor.model.file; // At this point also see if the active editor has a candidate interpreter to use if (!interpreter) { @@ -101,11 +104,11 @@ export class ExportCommands implements IDisposable { } if (exportMethod) { - await this.exportManager.export(exportMethod, model, defaultFileName, interpreter); + await this.exportManager.export(exportMethod, contents, source, defaultFileName, interpreter); } 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, defaultFileName, interpreter).then( + const pickedItem = await this.showExportQuickPickMenu(contents, source, defaultFileName, interpreter).then( (item) => item ); if (pickedItem !== undefined) { @@ -117,13 +120,19 @@ export class ExportCommands implements IDisposable { } private getExportQuickPickItems( - model: INotebookModel, + contents: string, + source: Uri, defaultFileName?: string, interpreter?: PythonEnvironment ): IExportQuickPickItem[] { const items: IExportQuickPickItem[] = []; + const notebook = JSON.parse(contents) as nbformat.INotebookContent; - if (model.metadata && model.metadata.language_info && model.metadata.language_info.name === PYTHON_LANGUAGE) { + if ( + notebook.metadata && + notebook.metadata.language_info && + notebook.metadata.language_info.name === PYTHON_LANGUAGE + ) { items.push({ label: DataScience.exportPythonQuickPickLabel(), picked: true, @@ -131,7 +140,7 @@ export class ExportCommands implements IDisposable { sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, { format: ExportFormat.python }); - this.commandManager.executeCommand(Commands.ExportAsPythonScript, model, interpreter); + this.commandManager.executeCommand(Commands.ExportAsPythonScript, contents, source, interpreter); } }); } @@ -145,7 +154,13 @@ export class ExportCommands implements IDisposable { sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, { format: ExportFormat.html }); - this.commandManager.executeCommand(Commands.ExportToHTML, model, defaultFileName, interpreter); + this.commandManager.executeCommand( + Commands.ExportToHTML, + contents, + source, + defaultFileName, + interpreter + ); } }, { @@ -155,7 +170,13 @@ export class ExportCommands implements IDisposable { sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, { format: ExportFormat.pdf }); - this.commandManager.executeCommand(Commands.ExportToPDF, model, defaultFileName, interpreter); + this.commandManager.executeCommand( + Commands.ExportToPDF, + contents, + source, + defaultFileName, + interpreter + ); } } ] @@ -165,11 +186,12 @@ export class ExportCommands implements IDisposable { } private async showExportQuickPickMenu( - model: INotebookModel, + contents: string, + source: Uri, defaultFileName?: string, interpreter?: PythonEnvironment ): Promise { - const items = this.getExportQuickPickItems(model, defaultFileName, interpreter); + const items = this.getExportQuickPickItems(contents, source, defaultFileName, interpreter); const options: QuickPickOptions = { ignoreFocusOut: false, diff --git a/src/client/datascience/export/exportDialog.ts b/src/client/datascience/export/exportDialog.ts new file mode 100644 index 00000000000..d73081c03c1 --- /dev/null +++ b/src/client/datascience/export/exportDialog.ts @@ -0,0 +1,70 @@ +import { inject, injectable } from 'inversify'; +import * as path from 'path'; +import { SaveDialogOptions, Uri } from 'vscode'; +import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; +import * as localize from '../../common/utils/localize'; +import { computeWorkingDirectory } from '../jupyter/jupyterUtils'; +import { ExportFormat, IExportDialog } from './types'; + +// File extensions for each export method +export const PDFExtensions = { PDF: ['pdf'] }; +export const HTMLExtensions = { HTML: ['html', 'htm'] }; +export const PythonExtensions = { Python: ['py'] }; + +@injectable() +export class ExportDialog implements IExportDialog { + constructor( + @inject(IApplicationShell) private readonly applicationShell: IApplicationShell, + @inject(IWorkspaceService) private workspaceService: IWorkspaceService + ) {} + + public async showDialog( + format: ExportFormat, + source: Uri | undefined, + defaultFileName?: string + ): Promise { + // map each export method to a set of file extensions + let fileExtensions: { [name: string]: string[] } = {}; + let extension: string | undefined; + switch (format) { + case ExportFormat.python: + fileExtensions = PythonExtensions; + extension = '.py'; + break; + + case ExportFormat.pdf: + extension = '.pdf'; + fileExtensions = PDFExtensions; + break; + + case ExportFormat.html: + extension = '.html'; + fileExtensions = HTMLExtensions; + break; + + case ExportFormat.ipynb: + extension = '.ipynb'; + const filtersKey = localize.DataScience.exportDialogFilter(); + fileExtensions[filtersKey] = ['ipynb']; + break; + + default: + return; + } + + const targetFileName = + defaultFileName || !source + ? defaultFileName || '' + : `${path.basename(source.fsPath, path.extname(source.fsPath))}${extension}`; + + const directory = await computeWorkingDirectory(source, this.workspaceService); + const dialogUri = Uri.file(path.join(directory, targetFileName)); + const options: SaveDialogOptions = { + defaultUri: dialogUri, + saveLabel: localize.DataScience.exportButtonTitle(), + filters: fileExtensions + }; + + return this.applicationShell.showSaveDialog(options); + } +} diff --git a/src/client/datascience/export/exportManager.ts b/src/client/datascience/export/exportManager.ts index fc8668b81c3..bf4db2a447e 100644 --- a/src/client/datascience/export/exportManager.ts +++ b/src/client/datascience/export/exportManager.ts @@ -10,11 +10,10 @@ import { PythonEnvironment } from '../../pythonEnvironments/info'; import { sendTelemetryEvent } from '../../telemetry'; import { Telemetry } from '../constants'; import { ProgressReporter } from '../progress/progressReporter'; -import { INotebookModel } from '../types'; import { ExportFileOpener } from './exportFileOpener'; import { ExportInterpreterFinder } from './exportInterpreterFinder'; import { ExportUtil } from './exportUtil'; -import { ExportFormat, IExport, IExportManager, IExportManagerFilePicker } from './types'; +import { ExportFormat, IExport, IExportDialog, IExportManager } from './types'; @injectable() export class ExportManager implements IExportManager { @@ -23,7 +22,7 @@ export class ExportManager implements IExportManager { @inject(IExport) @named(ExportFormat.html) private readonly exportToHTML: IExport, @inject(IExport) @named(ExportFormat.python) private readonly exportToPython: IExport, @inject(IFileSystem) private readonly fs: IFileSystem, - @inject(IExportManagerFilePicker) private readonly filePicker: IExportManagerFilePicker, + @inject(IExportDialog) private readonly filePicker: IExportDialog, @inject(ProgressReporter) private readonly progressReporter: ProgressReporter, @inject(ExportUtil) private readonly exportUtil: ExportUtil, @inject(IApplicationShell) private readonly applicationShell: IApplicationShell, @@ -33,7 +32,8 @@ export class ExportManager implements IExportManager { public async export( format: ExportFormat, - model: INotebookModel, + contents: string, + source: Uri, defaultFileName?: string, candidateInterpreter?: PythonEnvironment ): Promise { @@ -44,11 +44,11 @@ export class ExportManager implements IExportManager { format, candidateInterpreter ); - target = await this.getTargetFile(format, model, defaultFileName); + target = await this.getTargetFile(format, source, defaultFileName); if (!target) { return; } - await this.performExport(format, model, target, exportInterpreter); + await this.performExport(format, contents, target, exportInterpreter); } catch (e) { traceError('Export failed', e); sendTelemetryEvent(Telemetry.ExportNotebookAsFailed, undefined, { format: format }); @@ -61,19 +61,14 @@ export class ExportManager implements IExportManager { } } - private async performExport( - format: ExportFormat, - model: INotebookModel, - target: Uri, - interpreter: PythonEnvironment - ) { + private async performExport(format: ExportFormat, contents: string, target: Uri, interpreter: PythonEnvironment) { /* Need to make a temp directory here, instead of just a temp file. This is because we need to store the contents of the notebook in a file that is named the same as what we want the title of the exported file to be. To ensure this file path will be unique we store it in a temp directory. The name of the file matters because when exporting to certain formats the filename is used within the exported document as the title. */ const tempDir = await this.exportUtil.generateTempDir(); - const source = await this.makeSourceFile(target, model, tempDir); + const source = await this.makeSourceFile(target, contents, tempDir); const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`, true); try { @@ -90,15 +85,11 @@ export class ExportManager implements IExportManager { await this.exportFileOpener.openFile(format, target); } - private async getTargetFile( - format: ExportFormat, - model: INotebookModel, - defaultFileName?: string - ): Promise { + private async getTargetFile(format: ExportFormat, source: Uri, defaultFileName?: string): Promise { let target; if (format !== ExportFormat.python) { - target = await this.filePicker.getExportFileLocation(format, model.file, defaultFileName); + target = await this.filePicker.showDialog(format, source, defaultFileName); } else { target = Uri.file((await this.fs.createTemporaryLocalFile('.py')).filePath); } @@ -106,10 +97,10 @@ export class ExportManager implements IExportManager { return target; } - private async makeSourceFile(target: Uri, model: INotebookModel, tempDir: TemporaryDirectory): Promise { + private async makeSourceFile(target: Uri, contents: string, tempDir: TemporaryDirectory): Promise { // Creates a temporary file with the same base name as the target file const fileName = path.basename(target.fsPath, path.extname(target.fsPath)); - const sourceFilePath = await this.exportUtil.makeFileInDirectory(model, `${fileName}.ipynb`, tempDir.path); + const sourceFilePath = await this.exportUtil.makeFileInDirectory(contents, `${fileName}.ipynb`, tempDir.path); return Uri.file(sourceFilePath); } diff --git a/src/client/datascience/export/exportManagerFilePicker.ts b/src/client/datascience/export/exportManagerFilePicker.ts deleted file mode 100644 index c92436c295d..00000000000 --- a/src/client/datascience/export/exportManagerFilePicker.ts +++ /dev/null @@ -1,83 +0,0 @@ -import { inject, injectable, named } from 'inversify'; -import * as path from 'path'; -import { Memento, SaveDialogOptions, Uri } from 'vscode'; -import { IApplicationShell } from '../../common/application/types'; -import { IMemento, WORKSPACE_MEMENTO } from '../../common/types'; -import { ExportNotebookSettings } from '../interactive-common/interactiveWindowTypes'; -import { ExportFormat, IExportManagerFilePicker } from './types'; - -// File extensions for each export method -export const PDFExtensions = { PDF: ['pdf'] }; -export const HTMLExtensions = { HTML: ['html', 'htm'] }; -export const PythonExtensions = { Python: ['py'] }; - -@injectable() -export class ExportManagerFilePicker implements IExportManagerFilePicker { - private readonly defaultExportSaveLocation = ''; // set default save location - - constructor( - @inject(IApplicationShell) private readonly applicationShell: IApplicationShell, - @inject(IMemento) @named(WORKSPACE_MEMENTO) private workspaceStorage: Memento - ) {} - - public async getExportFileLocation( - format: ExportFormat, - source: Uri, - defaultFileName?: string - ): Promise { - // map each export method to a set of file extensions - let fileExtensions; - let extension: string | undefined; - switch (format) { - case ExportFormat.python: - fileExtensions = PythonExtensions; - extension = '.py'; - break; - - case ExportFormat.pdf: - extension = '.pdf'; - fileExtensions = PDFExtensions; - break; - - case ExportFormat.html: - extension = '.html'; - fileExtensions = HTMLExtensions; - break; - - default: - return; - } - - const targetFileName = defaultFileName - ? defaultFileName - : `${path.basename(source.fsPath, path.extname(source.fsPath))}${extension}`; - - const dialogUri = Uri.file(path.join(this.getLastFileSaveLocation().fsPath, targetFileName)); - const options: SaveDialogOptions = { - defaultUri: dialogUri, - saveLabel: 'Export', - filters: fileExtensions - }; - - const uri = await this.applicationShell.showSaveDialog(options); - if (uri) { - await this.updateFileSaveLocation(uri); - } - - return uri; - } - - private getLastFileSaveLocation(): Uri { - const filePath = this.workspaceStorage.get( - ExportNotebookSettings.lastSaveLocation, - this.defaultExportSaveLocation - ); - - return Uri.file(filePath); - } - - private async updateFileSaveLocation(value: Uri) { - const location = path.dirname(value.fsPath); - await this.workspaceStorage.update(ExportNotebookSettings.lastSaveLocation, location); - } -} diff --git a/src/client/datascience/export/exportUtil.ts b/src/client/datascience/export/exportUtil.ts index 973efa1c617..c9d5afb29d6 100644 --- a/src/client/datascience/export/exportUtil.ts +++ b/src/client/datascience/export/exportUtil.ts @@ -6,14 +6,13 @@ import * as uuid from 'uuid/v4'; import { Uri } from 'vscode'; import { IFileSystem, TemporaryDirectory } from '../../common/platform/types'; import { sleep } from '../../common/utils/async'; -import { ICell, INotebookExporter, INotebookModel, INotebookStorage } from '../types'; +import { INotebookStorage } from '../types'; @injectable() export class ExportUtil { constructor( @inject(IFileSystem) private fs: IFileSystem, - @inject(INotebookStorage) private notebookStorage: INotebookStorage, - @inject(INotebookExporter) private jupyterExporter: INotebookExporter + @inject(INotebookStorage) private notebookStorage: INotebookStorage ) {} public async generateTempDir(): Promise { @@ -40,32 +39,14 @@ export class ExportUtil { }; } - public async makeFileInDirectory(model: INotebookModel, fileName: string, dirPath: string): Promise { + public async makeFileInDirectory(contents: string, fileName: string, dirPath: string): Promise { const newFilePath = path.join(dirPath, fileName); - await this.fs.writeLocalFile(newFilePath, model.getContent()); + await this.fs.writeLocalFile(newFilePath, contents); return newFilePath; } - public async getModelFromCells(cells: ICell[]): Promise { - const tempDir = await this.generateTempDir(); - const tempFile = await this.fs.createTemporaryLocalFile('.ipynb'); - let model: INotebookModel; - - try { - await this.jupyterExporter.exportToFile(cells, tempFile.filePath, false); - const newPath = path.join(tempDir.path, '.ipynb'); - await this.fs.copyLocal(tempFile.filePath, newPath); - model = await this.notebookStorage.getOrCreateModel({ file: Uri.file(newPath) }); - } finally { - tempFile.dispose(); - tempDir.dispose(); - } - - return model; - } - public async removeSvgs(source: Uri) { const model = await this.notebookStorage.getOrCreateModel({ file: source }); const content = JSON.parse(model.getContent()) as nbformat.INotebookContent; diff --git a/src/client/datascience/export/types.ts b/src/client/datascience/export/types.ts index 76e57df9bcb..53cc6034684 100644 --- a/src/client/datascience/export/types.ts +++ b/src/client/datascience/export/types.ts @@ -1,16 +1,16 @@ import { CancellationToken, Uri } from 'vscode'; import { PythonEnvironment } from '../../pythonEnvironments/info'; -import { INotebookModel } from '../types'; export enum ExportFormat { pdf = 'pdf', html = 'html', - python = 'python' + python = 'python', + ipynb = 'ipynb' } export const IExportManager = Symbol('IExportManager'); export interface IExportManager { - export(format: ExportFormat, model: INotebookModel, defaultFileName?: string): Promise; + export(format: ExportFormat, contents: string, source: Uri, defaultFileName?: string): Promise; } export const IExport = Symbol('IExport'); @@ -18,7 +18,7 @@ export interface IExport { export(source: Uri, target: Uri, interpreter: PythonEnvironment, token: CancellationToken): Promise; } -export const IExportManagerFilePicker = Symbol('IExportManagerFilePicker'); -export interface IExportManagerFilePicker { - getExportFileLocation(format: ExportFormat, source: Uri, defaultFileName?: string): Promise; +export const IExportDialog = Symbol('IExportDialog'); +export interface IExportDialog { + showDialog(format: ExportFormat, source: Uri | undefined, defaultFileName?: string): Promise; } diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 6a7815e986f..597a974f4c7 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -735,7 +735,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } this.commandManager.executeCommand( Commands.Export, - activeEditor.model, + activeEditor.model.getContent(), + activeEditor.model.file, undefined, activeEditor.notebook?.getMatchingInterpreter() ); diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index 21b2496c703..1bdeb517e51 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -31,7 +31,7 @@ import { EXTENSION_ROOT_DIR } from '../../constants'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; import { Commands, EditorContexts, Identifiers, Telemetry } from '../constants'; import { IDataViewerFactory } from '../data-viewing/types'; -import { ExportUtil } from '../export/exportUtil'; +import { ExportFormat, IExportDialog } from '../export/types'; import { InteractiveBase } from '../interactive-common/interactiveBase'; import { INotebookIdentity, @@ -42,6 +42,7 @@ import { } from '../interactive-common/interactiveWindowTypes'; import { KernelSelector } from '../jupyter/kernels/kernelSelector'; import { KernelConnectionMetadata } from '../jupyter/kernels/types'; +import { updateNotebookMetadata } from '../notebookStorage/baseModel'; import { ICell, ICodeCssGenerator, @@ -56,7 +57,6 @@ import { IJupyterVariableDataProviderFactory, IJupyterVariables, INotebookExporter, - INotebookModel, INotebookProvider, IStatusProvider, IThemeFinder, @@ -127,13 +127,13 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi workspaceStorage: Memento, notebookProvider: INotebookProvider, useCustomEditorApi: boolean, - private exportUtil: ExportUtil, owner: Resource, mode: InteractiveWindowMode, title: string | undefined, selector: KernelSelector, private readonly extensionChecker: IPythonExtensionChecker, - serverStorage: IJupyterServerUriStorage + serverStorage: IJupyterServerUriStorage, + private readonly exportDialog: IExportDialog ) { super( listeners, @@ -507,19 +507,12 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi } // Should be an array of cells - if (cells && this.applicationShell) { + if (cells && this.exportDialog) { // Indicate busy this.startProgress(); try { - const filtersKey = localize.DataScience.exportDialogFilter(); - const filtersObject: Record = {}; - filtersObject[filtersKey] = ['ipynb']; - - // Bring up the open file dialog box - const uri = await this.applicationShell.showSaveDialog({ - saveLabel: localize.DataScience.exportDialogTitle(), - filters: filtersObject - }); + // Bring up the export file dialog box + const uri = await this.exportDialog.showDialog(ExportFormat.ipynb, this.owningResource); if (uri) { await this.jupyterExporter.exportToFile(cells, uri.fsPath); } @@ -535,27 +528,32 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi return this.extensionChecker.showPythonExtensionInstallRequiredPrompt(); } - let model: INotebookModel; - this.startProgress(); - try { - model = await this.exportUtil.getModelFromCells(cells); - } finally { - this.stopProgress(); + // Pull out the metadata from our active notebook + const metadata: nbformat.INotebookMetadata = { orig_nbformat: 3 }; + if (this.notebook) { + updateNotebookMetadata(metadata, this.notebook?.getKernelConnection()); } - if (model) { - let defaultFileName; - if (this.submitters && this.submitters.length) { - const lastSubmitter = this.submitters[this.submitters.length - 1]; - defaultFileName = path.basename(lastSubmitter.fsPath, path.extname(lastSubmitter.fsPath)); - } - this.commandManager.executeCommand( - Commands.Export, - model, - defaultFileName, - this.notebook?.getMatchingInterpreter() - ); + // Turn the cells into a json object + const json = await this.jupyterExporter.translateToNotebook(cells, undefined, metadata.kernelspec); + + // Turn this into a string + const contents = JSON.stringify(json, undefined, 4); + + let defaultFileName; + if (this.submitters && this.submitters.length) { + const lastSubmitter = this.submitters[this.submitters.length - 1]; + defaultFileName = path.basename(lastSubmitter.fsPath, path.extname(lastSubmitter.fsPath)); } + + // Then run the export command with these contents + this.commandManager.executeCommand( + Commands.Export, + contents, + this.owningResource, + defaultFileName, + this.notebook?.getMatchingInterpreter() + ); } private handleModelChange(update: NotebookModelChange) { diff --git a/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts b/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts index 151a623b8ac..4cf5092a4e0 100644 --- a/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts +++ b/src/client/datascience/interactive-window/interactiveWindowCommandListener.ts @@ -18,9 +18,8 @@ import { captureTelemetry } from '../../telemetry'; import { CommandSource } from '../../testing/common/constants'; import { generateCellRangesFromDocument, generateCellsFromDocument } from '../cellFactory'; import { Commands, Telemetry } from '../constants'; -import { ExportFormat, IExportManager } from '../export/types'; +import { ExportFormat, IExportDialog, IExportManager } from '../export/types'; import { JupyterInstallError } from '../jupyter/jupyterInstallError'; -import { INotebookStorageProvider } from '../notebookStorage/notebookStorageProvider'; import { IDataScienceCommandListener, IDataScienceErrorHandler, @@ -51,7 +50,7 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList @inject(IDataScienceErrorHandler) private dataScienceErrorHandler: IDataScienceErrorHandler, @inject(INotebookEditorProvider) protected ipynbProvider: INotebookEditorProvider, @inject(IExportManager) private exportManager: IExportManager, - @inject(INotebookStorageProvider) private notebookStorageProvider: INotebookStorageProvider + @inject(IExportDialog) private exportDialog: IExportDialog ) {} public register(commandManager: ICommandManager): void { @@ -188,15 +187,8 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList this.configuration.getSettings(activeEditor.document.uri) ); if (cells) { - const filtersKey = localize.DataScience.exportDialogFilter(); - const filtersObject: { [name: string]: string[] } = {}; - filtersObject[filtersKey] = ['ipynb']; - - // Bring up the save file dialog box - const uri = await this.applicationShell.showSaveDialog({ - saveLabel: localize.DataScience.exportDialogTitle(), - filters: filtersObject - }); + // Bring up the export dialog box + const uri = await this.exportDialog.showDialog(ExportFormat.ipynb, file); await this.waitForStatus( async () => { if (uri) { @@ -253,7 +245,7 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList const ranges = generateCellRangesFromDocument(activeEditor.document); if (ranges.length > 0) { // Ask user for path - const output = await this.showExportDialog(); + const output = await this.showExportDialog(file); // If that worked, we need to start a jupyter server to get our output values. // In the future we could potentially only update changed cells. @@ -354,16 +346,9 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList } } - private async showExportDialog(): Promise { - const filtersKey = localize.DataScience.exportDialogFilter(); - const filtersObject: { [name: string]: string[] } = {}; - filtersObject[filtersKey] = ['ipynb']; - + private async showExportDialog(file: Uri): Promise { // Bring up the save file dialog box - return this.applicationShell.showSaveDialog({ - saveLabel: localize.DataScience.exportDialogTitle(), - filters: filtersObject - }); + return this.exportDialog.showDialog(ExportFormat.ipynb, file); } private undoCells() { @@ -454,8 +439,7 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList await this.waitForStatus( async () => { const contents = await this.fileSystem.readFile(uris[0]); - const model = await this.notebookStorageProvider.createNew(contents); - await this.exportManager.export(ExportFormat.python, model); + await this.exportManager.export(ExportFormat.python, contents, uris[0]); }, localize.DataScience.importingFormat(), uris[0].fsPath @@ -469,8 +453,7 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList await this.waitForStatus( async () => { const contents = await this.fileSystem.readFile(file); - const model = await this.notebookStorageProvider.createNew(contents); - await this.exportManager.export(ExportFormat.python, model); + await this.exportManager.export(ExportFormat.python, contents, file); }, localize.DataScience.importingFormat(), file.fsPath diff --git a/src/client/datascience/interactive-window/interactiveWindowProvider.ts b/src/client/datascience/interactive-window/interactiveWindowProvider.ts index 3934a4748a9..7e062544f31 100644 --- a/src/client/datascience/interactive-window/interactiveWindowProvider.ts +++ b/src/client/datascience/interactive-window/interactiveWindowProvider.ts @@ -36,7 +36,7 @@ import * as localize from '../../common/utils/localize'; import { IServiceContainer } from '../../ioc/types'; import { Identifiers, LiveShare, LiveShareCommands } from '../constants'; import { IDataViewerFactory } from '../data-viewing/types'; -import { ExportUtil } from '../export/exportUtil'; +import { IExportDialog } from '../export/types'; import { KernelSelector } from '../jupyter/kernels/kernelSelector'; import { PostOffice } from '../liveshare/postOffice'; import { @@ -192,13 +192,13 @@ export class InteractiveWindowProvider implements IInteractiveWindowProvider, IA this.serviceContainer.get(IMemento, WORKSPACE_MEMENTO), this.serviceContainer.get(INotebookProvider), this.serviceContainer.get(UseCustomEditorApi), - this.serviceContainer.get(ExportUtil), resource, mode, title, this.serviceContainer.get(KernelSelector), this.serviceContainer.get(IPythonExtensionChecker), - this.serviceContainer.get(IJupyterServerUriStorage) + this.serviceContainer.get(IJupyterServerUriStorage), + this.serviceContainer.get(IExportDialog) ); this._windows.push(result); diff --git a/src/client/datascience/jupyter/jupyterUtils.ts b/src/client/datascience/jupyter/jupyterUtils.ts index af51cd7478a..b3325de05a2 100644 --- a/src/client/datascience/jupyter/jupyterUtils.ts +++ b/src/client/datascience/jupyter/jupyterUtils.ts @@ -96,5 +96,5 @@ export async function computeWorkingDirectory(resource: Resource, workspace: IWo } // Otherwise a file without an extension or directory doesn't exist. Just use the workspace root - return workspace.getWorkspaceFolder(resource)?.uri.fsPath || process.cwd(); + return workspace.getWorkspaceFolder(resource)?.uri.fsPath || workspace.rootPath || process.cwd(); } diff --git a/src/client/datascience/serviceRegistry.ts b/src/client/datascience/serviceRegistry.ts index 3f4e0ad2d67..433cc1e4793 100644 --- a/src/client/datascience/serviceRegistry.ts +++ b/src/client/datascience/serviceRegistry.ts @@ -36,15 +36,15 @@ import { Decorator } from './editor-integration/decorator'; import { HoverProvider } from './editor-integration/hoverProvider'; import { DataScienceErrorHandler } from './errorHandler/errorHandler'; import { ExportBase } from './export/exportBase'; +import { ExportDialog } from './export/exportDialog'; import { ExportFileOpener } from './export/exportFileOpener'; import { ExportInterpreterFinder } from './export/exportInterpreterFinder'; import { ExportManager } from './export/exportManager'; -import { ExportManagerFilePicker } from './export/exportManagerFilePicker'; import { ExportToHTML } from './export/exportToHTML'; import { ExportToPDF } from './export/exportToPDF'; import { ExportToPython } from './export/exportToPython'; import { ExportUtil } from './export/exportUtil'; -import { ExportFormat, IExport, IExportManager, IExportManagerFilePicker } from './export/types'; +import { ExportFormat, IExport, IExportDialog, IExportManager } from './export/types'; import { DebugListener } from './interactive-common/debugListener'; import { IntellisenseProvider } from './interactive-common/intellisense/intellisenseProvider'; import { LinkProvider } from './interactive-common/linkProvider'; @@ -317,7 +317,7 @@ export function registerTypes(serviceManager: IServiceManager, inNotebookApiExpe serviceManager.addSingleton(IExport, ExportBase, 'Export Base'); serviceManager.addSingleton(ExportUtil, ExportUtil); serviceManager.addSingleton(ExportCommands, ExportCommands); - serviceManager.addSingleton(IExportManagerFilePicker, ExportManagerFilePicker); + serviceManager.addSingleton(IExportDialog, ExportDialog); serviceManager.addSingleton(IJupyterUriProviderRegistration, JupyterUriProviderRegistration); serviceManager.addSingleton(ISystemPseudoRandomNumberGenerator, SystemPseudoRandomNumberGenerator); serviceManager.addSingleton(IDigestStorage, DigestStorage); diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 6fbb418dd9e..c2703d5d435 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -132,12 +132,12 @@ import { ExportBase } from '../../client/datascience/export/exportBase'; import { ExportFileOpener } from '../../client/datascience/export/exportFileOpener'; import { ExportInterpreterFinder } from '../../client/datascience/export/exportInterpreterFinder'; import { ExportManager } from '../../client/datascience/export/exportManager'; -import { ExportManagerFilePicker } from '../../client/datascience/export/exportManagerFilePicker'; +import { ExportDialog } from '../../client/datascience/export/exportDialog'; import { ExportToHTML } from '../../client/datascience/export/exportToHTML'; import { ExportToPDF } from '../../client/datascience/export/exportToPDF'; import { ExportToPython } from '../../client/datascience/export/exportToPython'; import { ExportUtil } from '../../client/datascience/export/exportUtil'; -import { ExportFormat, IExport, IExportManager, IExportManagerFilePicker } from '../../client/datascience/export/types'; +import { ExportFormat, IExport, IExportManager, IExportDialog } from '../../client/datascience/export/types'; import { IntellisenseProvider } from '../../client/datascience/interactive-common/intellisense/intellisenseProvider'; import { NotebookProvider } from '../../client/datascience/interactive-common/notebookProvider'; import { NotebookServerProvider } from '../../client/datascience/interactive-common/notebookServerProvider'; @@ -476,7 +476,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.addSingleton(IExport, ExportBase, 'Export Base'); this.serviceManager.addSingleton(ExportUtil, ExportUtil); this.serviceManager.addSingleton(ExportCommands, ExportCommands); - this.serviceManager.addSingleton(IExportManagerFilePicker, ExportManagerFilePicker); + this.serviceManager.addSingleton(IExportDialog, ExportDialog); this.serviceManager.addSingleton( INbConvertInterpreterDependencyChecker, NbConvertInterpreterDependencyChecker diff --git a/src/test/datascience/export/exportManager.vscode.test.ts b/src/test/datascience/export/exportManager.vscode.test.ts index cdd545d08d5..0a0a2ac6b14 100644 --- a/src/test/datascience/export/exportManager.vscode.test.ts +++ b/src/test/datascience/export/exportManager.vscode.test.ts @@ -12,9 +12,8 @@ import { ExportFileOpener } from '../../../client/datascience/export/exportFileO import { ExportInterpreterFinder } from '../../../client/datascience/export/exportInterpreterFinder'; import { ExportManager } from '../../../client/datascience/export/exportManager'; import { ExportUtil } from '../../../client/datascience/export/exportUtil'; -import { ExportFormat, IExport, IExportManagerFilePicker } from '../../../client/datascience/export/types'; +import { ExportFormat, IExport, IExportDialog } from '../../../client/datascience/export/types'; import { ProgressReporter } from '../../../client/datascience/progress/progressReporter'; -import { INotebookModel } from '../../../client/datascience/types'; suite('DataScience - Export Manager', () => { let exporter: ExportManager; @@ -23,15 +22,14 @@ suite('DataScience - Export Manager', () => { let exportPdf: IExport; let fileSystem: IFileSystem; let exportUtil: ExportUtil; - let filePicker: IExportManagerFilePicker; + let filePicker: IExportDialog; let appShell: IApplicationShell; let exportFileOpener: ExportFileOpener; let exportInterpreterFinder: ExportInterpreterFinder; - const model = mock(); setup(async () => { exportUtil = mock(); const reporter = mock(ProgressReporter); - filePicker = mock(); + filePicker = mock(); fileSystem = mock(); exportPython = mock(); exportHtml = mock(); @@ -40,7 +38,7 @@ suite('DataScience - Export Manager', () => { exportFileOpener = mock(); exportInterpreterFinder = mock(); // tslint:disable-next-line: no-any - when(filePicker.getExportFileLocation(anything(), anything(), anything())).thenReturn( + when(filePicker.showDialog(anything(), anything(), anything())).thenReturn( Promise.resolve(Uri.file('test.pdf')) ); // tslint:disable-next-line: no-empty @@ -51,7 +49,7 @@ suite('DataScience - Export Manager', () => { // tslint:disable-next-line: no-empty when(fileSystem.createTemporaryLocalFile(anything())).thenResolve({ filePath: 'test', dispose: () => {} }); when(exportPdf.export(anything(), anything(), anything(), anything())).thenResolve(); - when(filePicker.getExportFileLocation(anything(), anything())).thenResolve(Uri.file('foo')); + when(filePicker.showDialog(anything(), anything())).thenResolve(Uri.file('foo')); when(exportInterpreterFinder.getExportInterpreter(anything())).thenResolve(); when(exportFileOpener.openFile(anything(), anything())).thenResolve(); // tslint:disable-next-line: no-any @@ -71,27 +69,27 @@ suite('DataScience - Export Manager', () => { }); test('Remove svg is called when exporting to PDF', async () => { - await exporter.export(ExportFormat.pdf, model); + await exporter.export(ExportFormat.pdf, 'model', Uri.file('foo')); verify(exportUtil.removeSvgs(anything())).once(); }); test('Erorr message is shown if export fails', async () => { when(exportHtml.export(anything(), anything(), anything(), anything())).thenThrow(new Error('failed...')); - await exporter.export(ExportFormat.html, model); + await exporter.export(ExportFormat.html, 'model', Uri.file('foo')); verify(appShell.showErrorMessage(anything())).once(); verify(exportFileOpener.openFile(anything(), anything())).never(); }); test('Export to PDF is called when export method is PDF', async () => { - await exporter.export(ExportFormat.pdf, model); + await exporter.export(ExportFormat.pdf, 'model', Uri.file('foo')); verify(exportPdf.export(anything(), anything(), anything(), anything())).once(); verify(exportFileOpener.openFile(ExportFormat.pdf, anything())).once(); }); test('Export to HTML is called when export method is HTML', async () => { - await exporter.export(ExportFormat.html, model); + await exporter.export(ExportFormat.html, 'model', Uri.file('foo')); verify(exportHtml.export(anything(), anything(), anything(), anything())).once(); verify(exportFileOpener.openFile(ExportFormat.html, anything())).once(); }); test('Export to Python is called when export method is Python', async () => { - await exporter.export(ExportFormat.python, model); + await exporter.export(ExportFormat.python, 'model', Uri.file('foo')); verify(exportPython.export(anything(), anything(), anything(), anything())).once(); verify(exportFileOpener.openFile(ExportFormat.python, anything())).once(); }); diff --git a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts index f052f372684..68851b03a4c 100644 --- a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts +++ b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts @@ -18,14 +18,14 @@ 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 { ExportFormat, IExportManager } from '../../client/datascience/export/types'; +import { ExportDialog } from '../../client/datascience/export/exportDialog'; +import { ExportFormat, IExportDialog, IExportManager } from '../../client/datascience/export/types'; import { NotebookProvider } from '../../client/datascience/interactive-common/notebookProvider'; import { InteractiveWindowCommandListener } from '../../client/datascience/interactive-window/interactiveWindowCommandListener'; import { InteractiveWindowProvider } from '../../client/datascience/interactive-window/interactiveWindowProvider'; import { JupyterExecutionFactory } from '../../client/datascience/jupyter/jupyterExecutionFactory'; import { JupyterExporter } from '../../client/datascience/jupyter/jupyterExporter'; import { NativeEditorProvider } from '../../client/datascience/notebookStorage/nativeEditorProvider'; -import { NotebookStorageProvider } from '../../client/datascience/notebookStorage/notebookStorageProvider'; import { IInteractiveWindow, IJupyterExecution, @@ -70,10 +70,10 @@ suite('Interactive window command listener', async () => { const statusProvider = new MockStatusProvider(); const commandManager = new MockCommandManager(); const exportManager = mock(); - const notebookStorageProvider = mock(NotebookStorageProvider); let notebookEditorProvider: INotebookEditorProvider; const server = createTypeMoq('jupyter server'); let lastFileContents: any; + let exportDialog: IExportDialog; teardown(() => { documentManager.activeTextEditor = undefined; @@ -102,6 +102,7 @@ suite('Interactive window command listener', async () => { notebookEditorProvider = mock(NativeEditorProvider); jupyterExecution = mock(JupyterExecutionFactory); applicationShell = mock(ApplicationShell); + exportDialog = mock(ExportDialog); // Setup defaults when(interpreterService.onDidChangeInterpreter).thenReturn(dummyEvent.event); @@ -114,6 +115,8 @@ suite('Interactive window command listener', async () => { when(serviceContainer.get(IFileSystem)).thenReturn(instance(fileSystem)); when(configService.getSettings(anything())).thenReturn(pythonSettings); + when(exportDialog.showDialog(anything(), anything())).thenReturn(Promise.resolve(Uri.file('foo'))); + // Setup default settings pythonSettings.assign({ allowImportFromNotebook: true, @@ -213,7 +216,7 @@ suite('Interactive window command listener', async () => { instance(dataScienceErrorHandler), instance(notebookEditorProvider), instance(exportManager), - instance(notebookStorageProvider) + instance(exportDialog) ); result.register(commandManager); @@ -226,20 +229,17 @@ suite('Interactive window command listener', async () => { Promise.resolve([Uri.file('foo')]) ); await commandManager.executeCommand(Commands.ImportNotebook, undefined, undefined); - verify(exportManager.export(ExportFormat.python, anything())).once(); + verify(exportManager.export(ExportFormat.python, anything(), anything())).once(); }); test('Import File', async () => { createCommandListener(); await commandManager.executeCommand(Commands.ImportNotebook, Uri.file('bar.ipynb'), undefined); - verify(exportManager.export(ExportFormat.python, anything())).twice(); + verify(exportManager.export(ExportFormat.python, anything(), anything())).twice(); }); 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') @@ -280,9 +280,6 @@ suite('Interactive window command listener', async () => { return Promise.resolve(generateCells(undefined, 'a=1', 'bar.py', 0, false, uuid())); }); - 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') @@ -302,9 +299,6 @@ suite('Interactive window command listener', async () => { }); 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'); }); @@ -312,9 +306,6 @@ suite('Interactive window command listener', 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'); }); diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 289207501e5..471f1ee92ce 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -818,7 +818,8 @@ df.head()`; .setup((cmd) => cmd.executeCommand( Commands.Export, - model, + model.getContent(), + model.file, undefined, editor?.notebook?.getMatchingInterpreter() ) From 12abc5a324e8e3fabe5e0f569a1031e9864bc01a Mon Sep 17 00:00:00 2001 From: rchiodo Date: Tue, 1 Dec 2020 16:51:56 -0800 Subject: [PATCH 2/5] Add news entry --- news/2 Fixes/3991.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/3991.md diff --git a/news/2 Fixes/3991.md b/news/2 Fixes/3991.md new file mode 100644 index 00000000000..1dbfa901781 --- /dev/null +++ b/news/2 Fixes/3991.md @@ -0,0 +1 @@ +Fix the directory for exporting from the interactive window and notebooks to match the directory where the original file was created. \ No newline at end of file From 591f70b0557008ca13afce028b76bb34ae178f05 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Tue, 1 Dec 2020 16:57:21 -0800 Subject: [PATCH 3/5] Handle remote cases (I hope as nothing to test this with yet) --- src/client/datascience/export/exportDialog.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/client/datascience/export/exportDialog.ts b/src/client/datascience/export/exportDialog.ts index d73081c03c1..d95a731606d 100644 --- a/src/client/datascience/export/exportDialog.ts +++ b/src/client/datascience/export/exportDialog.ts @@ -57,14 +57,23 @@ export class ExportDialog implements IExportDialog { ? defaultFileName || '' : `${path.basename(source.fsPath, path.extname(source.fsPath))}${extension}`; - const directory = await computeWorkingDirectory(source, this.workspaceService); - const dialogUri = Uri.file(path.join(directory, targetFileName)); const options: SaveDialogOptions = { - defaultUri: dialogUri, + defaultUri: await this.getDefaultUri(source, targetFileName), saveLabel: localize.DataScience.exportButtonTitle(), filters: fileExtensions }; return this.applicationShell.showSaveDialog(options); } + + private async getDefaultUri(source: Uri | undefined, targetFileName: string): Promise { + if (!source || source.scheme === 'file' || source.scheme === 'untitled') { + // Just combine the working directory with the file + return Uri.file(path.join(await computeWorkingDirectory(source, this.workspaceService), targetFileName)); + } + + // Otherwise split off the end of the path and combine it with the target file name + const newPath = path.join(path.dirname(source.path), targetFileName); + return Uri.parse(`${source.scheme}://${newPath}`); + } } From 7c8ae7ba656981e58e7642c5189a01a744bbebe3 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Wed, 2 Dec 2020 10:09:35 -0800 Subject: [PATCH 4/5] Fix functional tests --- .../datascience/interactiveWindow.functional.test.tsx | 7 +++++-- src/test/datascience/nativeEditor.functional.test.tsx | 8 ++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index 7a35dd25685..541ff879679 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -68,6 +68,7 @@ import { } from './testHelpers'; import { ITestInteractiveWindowProvider } from './testInteractiveWindowProvider'; import { InteractiveWindowMessageListener } from '../../client/datascience/interactive-common/interactiveWindowMessageListener'; +import { IExportDialog } from '../../client/datascience/export/types'; // tslint:disable-next-line: no-require-imports no-var-requires const _escape = require('lodash/escape') as typeof import('lodash/escape'); // NOSONAR @@ -653,6 +654,7 @@ for i in range(0, 100): try { let exportCalled = false; const appShell = TypeMoq.Mock.ofType(); + const exportDialog = TypeMoq.Mock.ofType(); appShell .setup((a) => a.showErrorMessage(TypeMoq.It.isAnyString())) .returns((e) => { @@ -661,14 +663,15 @@ for i in range(0, 100): appShell .setup((a) => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve('')); - appShell - .setup((a) => a.showSaveDialog(TypeMoq.It.isAny())) + exportDialog + .setup((a) => a.showDialog(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => { exportCalled = true; return Promise.resolve(Uri.file(tf.filePath)); }); appShell.setup((a) => a.setStatusBarMessage(TypeMoq.It.isAny())).returns(() => dummyDisposable); ioc.serviceManager.rebindInstance(IApplicationShell, appShell.object); + ioc.serviceManager.rebindInstance(IExportDialog, exportDialog.object); const exportCode = ` for i in range(100): time.sleep(0.1) diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 471f1ee92ce..dd56515a5b4 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -796,7 +796,7 @@ df.head()`; appShell.setup((a) => a.setStatusBarMessage(TypeMoq.It.isAny())).returns(() => dummyDisposable); ioc.serviceManager.rebindInstance(IApplicationShell, appShell.object); - // Make sure to create the interactive window after the rebind or it gets the wrong application shell. + // Make sure to create the editor after the rebind or it gets the wrong application shell. const ne = await createNewEditor(ioc); const dirtyPromise = waitForMessage(ioc, InteractiveWindowMessages.NotebookDirty); await addCell(ne.mount, 'a=1\na'); @@ -811,8 +811,8 @@ df.head()`; // Click export and wait for a document to change const commandFired = createDeferred(); const commandManager = TypeMoq.Mock.ofType(); - const editor = TypeMoq.Mock.ofType().object.activeEditor; - const model = editor!.model!; + const editor = ne.editor; + const model = editor.model; ioc.serviceManager.rebindInstance(ICommandManager, commandManager.object); commandManager .setup((cmd) => @@ -821,7 +821,7 @@ df.head()`; model.getContent(), model.file, undefined, - editor?.notebook?.getMatchingInterpreter() + editor.notebook?.getMatchingInterpreter() ) ) .returns(() => { From f687cffbe696762c0348d5eeaea51722fc19060d Mon Sep 17 00:00:00 2001 From: rchiodo Date: Wed, 2 Dec 2020 11:19:10 -0800 Subject: [PATCH 5/5] Fix palette export to only come up for notebooks (interactive would be much harder to support) --- src/client/datascience/notebook/notebookEditorProvider.ts | 1 + src/client/datascience/notebookStorage/nativeEditorProvider.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/src/client/datascience/notebook/notebookEditorProvider.ts b/src/client/datascience/notebook/notebookEditorProvider.ts index f38e214e171..34d6d3985bf 100644 --- a/src/client/datascience/notebook/notebookEditorProvider.ts +++ b/src/client/datascience/notebook/notebookEditorProvider.ts @@ -122,6 +122,7 @@ export class NotebookEditorProvider implements INotebookEditorProvider { if (this.openedEditors.has(editor)) { this.openedEditors.delete(editor); this._onDidCloseNotebookEditor.fire(editor); + this._onDidChangeActiveNotebookEditor.fire(this.activeEditor); // Find all notebooks associated with this editor (ipynb file). const otherEditors = this.editors.filter( diff --git a/src/client/datascience/notebookStorage/nativeEditorProvider.ts b/src/client/datascience/notebookStorage/nativeEditorProvider.ts index 284937278fc..2763d48c7d0 100644 --- a/src/client/datascience/notebookStorage/nativeEditorProvider.ts +++ b/src/client/datascience/notebookStorage/nativeEditorProvider.ts @@ -307,6 +307,7 @@ export class NativeEditorProvider implements INotebookEditorProvider, CustomEdit private closedEditor(editor: INotebookEditor): void { this.openedEditors.delete(editor); this._onDidCloseNotebookEditor.fire(editor); + this._onDidChangeActiveNotebookEditor.fire(this.activeEditor); } private trackModel(model: INotebookModel) { if (!this.models.has(model)) {