From ea0643cb333da8e7f28ab79448c477a51a76ba9b Mon Sep 17 00:00:00 2001 From: Timothy Ruscica <35348871+techwithtim@users.noreply.github.com> Date: Wed, 8 Jul 2020 18:04:59 -0400 Subject: [PATCH 01/11] Changed default save name when exporting from interactive window (#12815) * changed default export file name from interactice window * fixed naming issue * refactor * fixed tests * hopefully fixed test --- src/client/common/application/commands.ts | 6 ++-- .../datascience/commands/exportCommands.ts | 31 ++++++++++++------- .../datascience/export/exportManager.ts | 8 +++-- .../export/exportManagerDependencyChecker.ts | 8 +++-- .../export/exportManagerFileOpener.ts | 8 +++-- .../export/exportManagerFilePicker.ts | 11 +++++-- src/client/datascience/export/types.ts | 4 +-- .../interactive-ipynb/nativeEditor.ts | 2 +- .../interactive-window/interactiveWindow.ts | 6 +++- .../datascience/export/exportManager.test.ts | 2 +- .../exportManagerFileOpener.unit.test.ts | 12 +++---- .../nativeEditor.functional.test.tsx | 2 +- 12 files changed, 66 insertions(+), 34 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..d609860657c4 100644 --- a/src/client/datascience/commands/exportCommands.ts +++ b/src/client/datascience/commands/exportCommands.ts @@ -31,9 +31,15 @@ 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, defaultFileName?) => + this.export(model, ExportFormat.html, defaultFileName) + ); + this.registerCommand(Commands.ExportToPDF, (model, defaultFileName?) => + this.export(model, ExportFormat.pdf, defaultFileName) + ); + this.registerCommand(Commands.Export, (model, defaultFileName?) => + this.export(model, undefined, defaultFileName) + ); } public dispose() { @@ -49,7 +55,7 @@ export class ExportCommands implements IDisposable { this.disposables.push(disposable); } - private async export(model: INotebookModel, exportMethod?: ExportFormat) { + 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 @@ -65,11 +71,11 @@ export class ExportCommands implements IDisposable { } if (exportMethod) { - await this.exportManager.export(exportMethod, model); + 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).then((item) => item); + const pickedItem = await this.showExportQuickPickMenu(model, defaultFileName).then((item) => item); if (pickedItem !== undefined) { pickedItem.handler(); } else { @@ -78,7 +84,7 @@ export class ExportCommands implements IDisposable { } } - private getExportQuickPickItems(model: INotebookModel): IExportQuickPickItem[] { + private getExportQuickPickItems(model: INotebookModel, defaultFileName?: string): IExportQuickPickItem[] { return [ { label: DataScience.exportPythonQuickPickLabel(), @@ -97,7 +103,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, defaultFileName); } }, { @@ -107,14 +113,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, defaultFileName); } } ]; } - private async showExportQuickPickMenu(model: INotebookModel): Promise { - const items = this.getExportQuickPickItems(model); + private async showExportQuickPickMenu( + model: INotebookModel, + defaultFileName?: string + ): Promise { + const items = this.getExportQuickPickItems(model, defaultFileName); 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..afb3e7ffb567 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) { @@ -40,7 +44,10 @@ export class ExportManagerFilePicker implements IExportManagerFilePicker { return; } - const notebookFileName = path.basename(source.fsPath, path.extname(source.fsPath)); + 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, 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 7e2953cf8b3e..c40a543ad097 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -738,7 +738,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..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); + let defaultFileName; + if (this.lastFile) { + defaultFileName = path.basename(this.lastFile, path.extname(this.lastFile)); + } + this.commandManager.executeCommand(Commands.Export, model, defaultFileName); } } 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 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')) ); 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 69322df7c1fc819484333e29680c8fe531fbf2d7 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 9 Jul 2020 13:28:43 -0700 Subject: [PATCH 02/11] Include notebook renderers in extension (#12793) --- gulpfile.js | 1 + package.json | 40 +++++++- .../datascience/notebook/integration.ts | 96 +++++-------------- .../notebook/notebookEditorProvider.ts | 8 -- 4 files changed, 61 insertions(+), 84 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index 85bed2d5b04b..96a2a6459be8 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -177,6 +177,7 @@ gulp.task('webpack', async () => { await buildWebPackForDevOrProduction('./build/webpack/webpack.datascience-ui-notebooks.config.js', 'production'); await buildWebPackForDevOrProduction('./build/webpack/webpack.datascience-ui-viewers.config.js', 'production'); await buildWebPackForDevOrProduction('./build/webpack/webpack.extension.config.js', 'extension'); + await buildWebPackForDevOrProduction('./build/webpack/webpack.datascience-ui-renderers.config.js', 'production'); }); gulp.task('updateBuildNumber', async () => { diff --git a/package.json b/package.json index b18fd8ed816d..8967f7c60213 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ }, "languageServerVersion": "0.5.30", "publisher": "ms-python", - "enableProposedApi": false, + "enableProposedApi": true, "author": { "name": "Microsoft Corporation" }, @@ -3085,7 +3085,43 @@ "when": "testsDiscovered" } ] - } + }, + "notebookOutputRenderer": [ + { + "viewType": "jupyter-notebook-renderer", + "displayName": "Jupyter Notebook Renderer", + "mimeTypes": [ + "application/geo+json", + "application/vdom.v1+json", + "application/vnd.dataresource+json", + "application/vnd.plotly.v1+json", + "application/vnd.vega.v2+json", + "application/vnd.vega.v3+json", + "application/vnd.vega.v4+json", + "application/vnd.vega.v5+json", + "application/vnd.vegalite.v1+json", + "application/vnd.vegalite.v2+json", + "application/vnd.vegalite.v3+json", + "application/vnd.vegalite.v4+json", + "application/x-nteract-model-debug+json", + "image/gif", + "text/latex", + "text/vnd.plotly.v1+html" + ] + } + ], + "notebookProvider": [ + { + "viewType": "jupyter-notebook", + "displayName": "Jupyter Notebook (preview)", + "selector": [ + { + "filenamePattern": "*.ipynb" + } + ], + "priority": "option" + } + ] }, "scripts": { "package": "gulp clean && gulp prePublishBundle && vsce package -o ms-python-insiders.vsix", diff --git a/src/client/datascience/notebook/integration.ts b/src/client/datascience/notebook/integration.ts index 2ba2c55ff379..28c19f777c57 100644 --- a/src/client/datascience/notebook/integration.ts +++ b/src/client/datascience/notebook/integration.ts @@ -2,18 +2,17 @@ // Licensed under the MIT License. 'use strict'; import { inject, injectable } from 'inversify'; -import * as path from 'path'; +import { ConfigurationTarget } from 'vscode'; import { IExtensionSingleActivationService } from '../../activation/types'; import { IApplicationEnvironment, IApplicationShell, - ICommandManager, - IVSCodeNotebook + IVSCodeNotebook, + IWorkspaceService } from '../../common/application/types'; import { NotebookEditorSupport } from '../../common/experiments/groups'; import { traceError } from '../../common/logger'; -import { IFileSystem } from '../../common/platform/types'; -import { IDisposableRegistry, IExperimentsManager, IExtensionContext } from '../../common/types'; +import { IDisposableRegistry, IExperimentsManager } from '../../common/types'; import { DataScience } from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; import { JupyterNotebookView } from './constants'; @@ -33,36 +32,18 @@ export class NotebookIntegration implements IExtensionSingleActivationService { @inject(IExperimentsManager) private readonly experiment: IExperimentsManager, @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(INotebookContentProvider) private readonly notebookContentProvider: INotebookContentProvider, - @inject(IExtensionContext) private readonly context: IExtensionContext, - @inject(IFileSystem) private readonly fs: IFileSystem, - @inject(ICommandManager) private readonly commandManager: ICommandManager, @inject(NotebookKernel) private readonly notebookKernel: NotebookKernel, @inject(NotebookOutputRenderer) private readonly renderer: NotebookOutputRenderer, @inject(IApplicationEnvironment) private readonly env: IApplicationEnvironment, - @inject(IApplicationShell) private readonly shell: IApplicationShell + @inject(IApplicationShell) private readonly shell: IApplicationShell, + @inject(IWorkspaceService) private readonly workspace: IWorkspaceService ) {} - public get isEnabled() { - const packageJsonFile = path.join(this.context.extensionPath, 'package.json'); - const content = JSON.parse(this.fs.readFileSync(packageJsonFile)); - - // This code is temporary. - return ( - content.enableProposedApi && - Array.isArray(content.contributes.notebookOutputRenderer) && - (content.contributes.notebookOutputRenderer as []).length > 0 && - Array.isArray(content.contributes.notebookProvider) && - (content.contributes.notebookProvider as []).length > 0 - ); - } - public async enableSideBySideUsage() { - await this.enableNotebooks(false); - } public async activate(): Promise { // This condition is temporary. // If user belongs to the experiment, then make the necessary changes to package.json. // Once the API is final, we won't need to modify the package.json. if (this.experiment.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) { - await this.enableNotebooks(true); + await this.enableNotebooks(); } if (this.env.channel !== 'insiders') { return; @@ -108,62 +89,29 @@ export class NotebookIntegration implements IExtensionSingleActivationService { } } } - private async enableNotebooks(useVSCodeNotebookAsDefaultEditor: boolean) { + private async enableNotebooks() { if (this.env.channel === 'stable') { this.shell.showErrorMessage(DataScience.previewNotebookOnlySupportedInVSCInsiders()).then(noop, noop); return; } - const packageJsonFile = path.join(this.context.extensionPath, 'package.json'); - const content = JSON.parse(this.fs.readFileSync(packageJsonFile)); // This code is temporary. + const settings = this.workspace.getConfiguration('workbench', undefined); + const editorAssociations = settings.get('editorAssociations') as { + viewType: string; + filenamePattern: string; + }[]; + if ( - !content.enableProposedApi || - !Array.isArray(content.contributes.notebookOutputRenderer) || - !Array.isArray(content.contributes.notebookProvider) + !Array.isArray(editorAssociations) || + editorAssociations.length === 0 || + !editorAssociations.find((item) => item.viewType === JupyterNotebookView) ) { - content.enableProposedApi = true; - content.contributes.notebookOutputRenderer = [ - { - viewType: 'jupyter-notebook-renderer', - displayName: 'Jupyter Notebook Renderer', - mimeTypes: [ - 'application/geo+json', - 'application/vdom.v1+json', - 'application/vnd.dataresource+json', - 'application/vnd.plotly.v1+json', - 'application/vnd.vega.v2+json', - 'application/vnd.vega.v3+json', - 'application/vnd.vega.v4+json', - 'application/vnd.vega.v5+json', - 'application/vnd.vegalite.v1+json', - 'application/vnd.vegalite.v2+json', - 'application/vnd.vegalite.v3+json', - 'application/vnd.vegalite.v4+json', - 'application/x-nteract-model-debug+json', - 'image/gif', - 'text/latex', - 'text/vnd.plotly.v1+html' - ] - } - ]; - content.contributes.notebookProvider = [ - { - viewType: JupyterNotebookView, - displayName: 'Jupyter Notebook (preview)', - selector: [ - { - filenamePattern: '*.ipynb' - } - ], - priority: useVSCodeNotebookAsDefaultEditor ? 'default' : 'option' - } - ]; - - await this.fs.writeFile(packageJsonFile, JSON.stringify(content, undefined, 4)); - await this.commandManager - .executeCommand('python.reloadVSCode', DataScience.reloadVSCodeNotebookEditor()) - .then(noop, noop); + editorAssociations.push({ + viewType: 'jupyter-notebook', + filenamePattern: '*.ipynb' + }); + await settings.update('editorAssociations', editorAssociations, ConfigurationTarget.Global); } } } diff --git a/src/client/datascience/notebook/notebookEditorProvider.ts b/src/client/datascience/notebook/notebookEditorProvider.ts index a5a88521b6d2..cdf3c0614742 100644 --- a/src/client/datascience/notebook/notebookEditorProvider.ts +++ b/src/client/datascience/notebook/notebookEditorProvider.ts @@ -28,7 +28,6 @@ import { JupyterNotebookView } from './constants'; import { mapVSCNotebookCellsToNotebookCellModels } from './helpers/cellMappers'; import { updateCellModelWithChangesToVSCCell } from './helpers/cellUpdateHelpers'; import { isJupyterNotebook } from './helpers/helpers'; -import { NotebookIntegration } from './integration'; import { NotebookEditor } from './notebookEditor'; import { INotebookContentProvider, INotebookExecutionService } from './types'; @@ -85,13 +84,6 @@ export class NotebookEditorProvider implements INotebookEditorProvider { if (uri) { setSharedProperty('ds_notebookeditor', 'native'); captureTelemetry(Telemetry.OpenNotebook, { scope: 'command' }, false); - const integration = serviceContainer.get(NotebookIntegration); - // If user is not meant to be using VSC Notebooks, and it is not enabled, - // then enable it for side by side usage. - if (!integration.isEnabled && !useVSCodeNotebookEditorApi) { - // At this point we need to reload VS Code, hence return and do not try to load nb, else it will fail. - return integration.enableSideBySideUsage(); - } this.open(uri).ignoreErrors(); } }) From c8736b339f82863923f755a3e41404a3c0137a27 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 9 Jul 2020 14:45:56 -0700 Subject: [PATCH 03/11] Update categories in package.json for better discoverability of extension features (#12845) --- news/3 Code Health/12844.md | 1 + package.json | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 news/3 Code Health/12844.md diff --git a/news/3 Code Health/12844.md b/news/3 Code Health/12844.md new file mode 100644 index 000000000000..004246586d91 --- /dev/null +++ b/news/3 Code Health/12844.md @@ -0,0 +1 @@ +Update categories in `package.json`. diff --git a/package.json b/package.json index 8967f7c60213..c2f70540e91b 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.46.0" + "vscode": "^1.47.0" }, "keywords": [ "python", @@ -43,7 +43,10 @@ "Snippets", "Formatters", "Other", - "Extension Packs" + "Extension Packs", + "Data Science", + "Machine Learning", + "Notebooks" ], "activationEvents": [ "onLanguage:python", From 7d50ab9f735c129844992268ed27ad278f8faf82 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 9 Jul 2020 15:37:46 -0700 Subject: [PATCH 04/11] News entry for VS Code Notebooks editor in VSC Insiders (#12843) --- news/1 Enhancements/10496.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 news/1 Enhancements/10496.md diff --git a/news/1 Enhancements/10496.md b/news/1 Enhancements/10496.md new file mode 100644 index 000000000000..e486012325cb --- /dev/null +++ b/news/1 Enhancements/10496.md @@ -0,0 +1,4 @@ +Opening notebooks in the preview Notebook editor for [Visual Studio Code Insiders](https://code.visualstudio.com/insiders/). +* Install Python extension in the latest [Visual Studio Code Insiders](https://code.visualstudio.com/insiders/). +* Wait for `Python Extension` to get activated (e.g. open a `Python` file). +* Right click on an `*.ipynb (Jupyter Notebook)` file and select `Open in preview Notebook Editor`. From f7a6b5deb20a76f0ea235eae126e1c81eddc6724 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 10 Jul 2020 09:33:37 -0700 Subject: [PATCH 05/11] Revert settings changes when opting out of Native NB Experiment (#12852) For #10496 --- .../datascience/notebook/integration.ts | 51 ++++++++++++++++--- 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/src/client/datascience/notebook/integration.ts b/src/client/datascience/notebook/integration.ts index 28c19f777c57..e9a073f1c2bf 100644 --- a/src/client/datascience/notebook/integration.ts +++ b/src/client/datascience/notebook/integration.ts @@ -12,7 +12,7 @@ import { } from '../../common/application/types'; import { NotebookEditorSupport } from '../../common/experiments/groups'; import { traceError } from '../../common/logger'; -import { IDisposableRegistry, IExperimentsManager } from '../../common/types'; +import { IDisposableRegistry, IExperimentsManager, IExtensionContext } from '../../common/types'; import { DataScience } from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; import { JupyterNotebookView } from './constants'; @@ -20,6 +20,8 @@ import { NotebookKernel } from './notebookKernel'; import { NotebookOutputRenderer } from './renderer'; import { INotebookContentProvider } from './types'; +const EditorAssociationUpdatedKey = 'EditorAssociationUpdatedToUseNotebooks'; + /** * This class basically registers the necessary providers and the like with VSC. * I.e. this is where we integrate our stuff with VS Code via their extension endpoints. @@ -36,7 +38,8 @@ export class NotebookIntegration implements IExtensionSingleActivationService { @inject(NotebookOutputRenderer) private readonly renderer: NotebookOutputRenderer, @inject(IApplicationEnvironment) private readonly env: IApplicationEnvironment, @inject(IApplicationShell) private readonly shell: IApplicationShell, - @inject(IWorkspaceService) private readonly workspace: IWorkspaceService + @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, + @inject(IExtensionContext) private readonly extensionContext: IExtensionContext ) {} public async activate(): Promise { // This condition is temporary. @@ -44,6 +47,10 @@ export class NotebookIntegration implements IExtensionSingleActivationService { // Once the API is final, we won't need to modify the package.json. if (this.experiment.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) { await this.enableNotebooks(); + } else { + // Possible user was in experiment, then they opted out. In this case we need to revert the changes made to the settings file. + // Again, this is temporary code. + await this.disableNotebooks(); } if (this.env.channel !== 'insiders') { return; @@ -95,6 +102,9 @@ export class NotebookIntegration implements IExtensionSingleActivationService { return; } + await this.enableDisableEditorAssociation(true); + } + private async enableDisableEditorAssociation(enable: boolean) { // This code is temporary. const settings = this.workspace.getConfiguration('workbench', undefined); const editorAssociations = settings.get('editorAssociations') as { @@ -102,16 +112,45 @@ export class NotebookIntegration implements IExtensionSingleActivationService { filenamePattern: string; }[]; + // Update the settings. if ( - !Array.isArray(editorAssociations) || - editorAssociations.length === 0 || - !editorAssociations.find((item) => item.viewType === JupyterNotebookView) + enable && + (!Array.isArray(editorAssociations) || + editorAssociations.length === 0 || + !editorAssociations.find((item) => item.viewType === JupyterNotebookView)) ) { editorAssociations.push({ viewType: 'jupyter-notebook', filenamePattern: '*.ipynb' }); - await settings.update('editorAssociations', editorAssociations, ConfigurationTarget.Global); + await Promise.all([ + this.extensionContext.globalState.update(EditorAssociationUpdatedKey, true), + settings.update('editorAssociations', editorAssociations, ConfigurationTarget.Global) + ]); + } + + // Revert the settings. + if ( + !enable && + this.extensionContext.globalState.get(EditorAssociationUpdatedKey, false) && + Array.isArray(editorAssociations) && + editorAssociations.find((item) => item.viewType === JupyterNotebookView) + ) { + const updatedSettings = editorAssociations.filter((item) => item.viewType !== JupyterNotebookView); + await Promise.all([ + this.extensionContext.globalState.update(EditorAssociationUpdatedKey, false), + settings.update('editorAssociations', updatedSettings, ConfigurationTarget.Global) + ]); + } + } + private async disableNotebooks() { + if (this.env.channel === 'stable') { + return; + } + // If we never modified the settings, then nothing to do. + if (!this.extensionContext.globalState.get(EditorAssociationUpdatedKey, false)) { + return; } + await this.enableDisableEditorAssociation(false); } } From f7ef7e6d630b197d3efe26019020c79c2d249cfd Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 10 Jul 2020 10:11:25 -0700 Subject: [PATCH 06/11] Ability to export VS Code Native notebooks (#12879) --- package.json | 22 +++++++++++++++++ resources/dark/export_to_python.svg | 4 ++++ resources/light/export_to_python.svg | 4 ++++ src/client/common/application/commands.ts | 2 +- .../datascience/commands/exportCommands.ts | 24 +++++++++++++++---- 5 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 resources/dark/export_to_python.svg create mode 100644 resources/light/export_to_python.svg diff --git a/package.json b/package.json index c2f70540e91b..2c04f6e8c16b 100644 --- a/package.json +++ b/package.json @@ -63,6 +63,7 @@ "onCommand:python.viewTestOutput", "onCommand:python.viewOutput", "onCommand:python.datascience.viewJupyterOutput", + "onCOmmand:python.datascience.export", "onCommand:python.datascience.exportAsPythonScript", "onCommand:python.datascience.exportToHTML", "onCommand:python.datascience.exportToPDF", @@ -361,6 +362,15 @@ "title": "%python.command.python.datascience.viewJupyterOutput.title%", "category": "Python" }, + { + "command": "python.datascience.export", + "title": "%DataScience.notebookExportAs%", + "category": "Python", + "icon": { + "light": "resources/light/export_to_python.svg", + "dark": "resources/dark/export_to_python.svg" + } + }, { "command": "python.datascience.exportAsPythonScript", "title": "%python.command.python.datascience.exportAsPythonScript.title%", @@ -849,6 +859,12 @@ "title": "%python.command.python.datascience.restartkernel.title%", "group": "navigation", "when": "notebookEditorFocused" + }, + { + "command": "python.datascience.export", + "title": "%DataScience.notebookExportAs%", + "group": "navigation", + "when": "notebookEditorFocused" } ], "explorer/context": [ @@ -1236,6 +1252,12 @@ "title": "%DataScience.gatherQuality%", "category": "Python", "when": "false" + }, + { + "command": "python.datascience.export", + "title": "%DataScience.notebookExportAs%", + "category": "Python", + "when": "false" } ], "view/title": [ diff --git a/resources/dark/export_to_python.svg b/resources/dark/export_to_python.svg new file mode 100644 index 000000000000..a68ca2942cb7 --- /dev/null +++ b/resources/dark/export_to_python.svg @@ -0,0 +1,4 @@ + + + + diff --git a/resources/light/export_to_python.svg b/resources/light/export_to_python.svg new file mode 100644 index 000000000000..873383aaeb21 --- /dev/null +++ b/resources/light/export_to_python.svg @@ -0,0 +1,4 @@ + + + + diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 6dd8052485ac..17c73a6e3a43 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -175,7 +175,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu [DSCommands.ExportAsPythonScript]: [INotebookModel]; [DSCommands.ExportToHTML]: [INotebookModel, string | undefined]; [DSCommands.ExportToPDF]: [INotebookModel, string | undefined]; - [DSCommands.Export]: [INotebookModel, string | undefined]; + [DSCommands.Export]: [Uri | 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 d609860657c4..e936e60d3ec3 100644 --- a/src/client/datascience/commands/exportCommands.ts +++ b/src/client/datascience/commands/exportCommands.ts @@ -4,12 +4,14 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { QuickPickItem, QuickPickOptions } from 'vscode'; +import { QuickPickItem, QuickPickOptions, Uri } from 'vscode'; import { getLocString } from '../../../datascience-ui/react-common/locReactSide'; import { ICommandNameArgumentTypeMapping } from '../../common/application/commands'; import { IApplicationShell, ICommandManager } from '../../common/application/types'; +import { IFileSystem } from '../../common/platform/types'; import { IDisposable } from '../../common/types'; import { DataScience } from '../../common/utils/localize'; +import { isUri } from '../../common/utils/misc'; import { sendTelemetryEvent } from '../../telemetry'; import { Commands, Telemetry } from '../constants'; import { ExportManager } from '../export/exportManager'; @@ -27,7 +29,8 @@ export class ExportCommands implements IDisposable { @inject(ICommandManager) private readonly commandManager: ICommandManager, @inject(IExportManager) private exportManager: ExportManager, @inject(IApplicationShell) private readonly applicationShell: IApplicationShell, - @inject(INotebookEditorProvider) private readonly notebookProvider: INotebookEditorProvider + @inject(INotebookEditorProvider) private readonly notebookProvider: INotebookEditorProvider, + @inject(IFileSystem) private readonly fs: IFileSystem ) {} public register() { this.registerCommand(Commands.ExportAsPythonScript, (model) => this.export(model, ExportFormat.python)); @@ -55,9 +58,22 @@ export class ExportCommands implements IDisposable { this.disposables.push(disposable); } - private async export(model: INotebookModel, exportMethod?: ExportFormat, defaultFileName?: string) { + private async export(modelOrUri: Uri | INotebookModel, exportMethod?: ExportFormat, defaultFileName?: string) { + defaultFileName = typeof defaultFileName === 'string' ? defaultFileName : undefined; + let model: INotebookModel | undefined; + if (modelOrUri && isUri(modelOrUri)) { + const uri = modelOrUri; + const editor = this.notebookProvider.editors.find((item) => + this.fs.arePathsSame(item.file.fsPath, uri.fsPath) + ); + if (editor && editor.model) { + model = editor.model; + } + } else { + model = modelOrUri; + } if (!model) { - // if no model was passed then this was called from the command pallete, + // if no model 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) { From dbf399677138b1d63ef82d21f406a928320cf736 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 10 Jul 2020 10:36:30 -0700 Subject: [PATCH 07/11] Fixes to exporting of notebook to html (#12882) Ensure we have file extensions for the source ipynb Ensure we have file extensions for the target html/pdf files Misc cleanup --- src/client/datascience/export/exportBase.ts | 7 ++++++- src/client/datascience/export/exportManager.ts | 2 +- .../datascience/export/exportManagerFilePicker.ts | 10 +++++++--- src/client/datascience/export/exportUtil.ts | 10 ++-------- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/client/datascience/export/exportBase.ts b/src/client/datascience/export/exportBase.ts index 7bf957085fed..a5ed31449b51 100644 --- a/src/client/datascience/export/exportBase.ts +++ b/src/client/datascience/export/exportBase.ts @@ -44,7 +44,7 @@ export class ExportBase implements IExport { throw new Error(result.stderr); } else if (oldFileExists) { // If we exported to a file that already exists we need to check that - // this file was actually overriden during export + // this file was actually overridden during export const newFileTime = (await this.fileSystem.stat(target.fsPath)).mtime; if (newFileTime === oldFileTime) { throw new Error(result.stderr); @@ -53,8 +53,13 @@ export class ExportBase implements IExport { } protected async getExecutionService(source: Uri): Promise { + const interpreter = await this.jupyterService.getSelectedInterpreter(); + if (!interpreter) { + return; + } return this.pythonExecutionFactory.createActivatedEnvironment({ resource: source, + interpreter, allowEnvironmentFetchExceptions: false, bypassCondaExecution: true }); diff --git a/src/client/datascience/export/exportManager.ts b/src/client/datascience/export/exportManager.ts index 9aec833e9b15..dcfb47f80183 100644 --- a/src/client/datascience/export/exportManager.ts +++ b/src/client/datascience/export/exportManager.ts @@ -42,7 +42,7 @@ export class ExportManager implements IExportManager { // exporting to certain formats the filename is used within the exported document as the title. const fileName = path.basename(target.fsPath, path.extname(target.fsPath)); const tempDir = await this.exportUtil.generateTempDir(); - const sourceFilePath = await this.exportUtil.makeFileInDirectory(model, fileName, tempDir.path); + const sourceFilePath = await this.exportUtil.makeFileInDirectory(model, `${fileName}.ipynb`, tempDir.path); const source = Uri.file(sourceFilePath); if (format === ExportFormat.pdf) { diff --git a/src/client/datascience/export/exportManagerFilePicker.ts b/src/client/datascience/export/exportManagerFilePicker.ts index afb3e7ffb567..c92436c295dd 100644 --- a/src/client/datascience/export/exportManagerFilePicker.ts +++ b/src/client/datascience/export/exportManagerFilePicker.ts @@ -27,16 +27,20 @@ export class ExportManagerFilePicker implements IExportManagerFilePicker { ): 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; @@ -44,11 +48,11 @@ export class ExportManagerFilePicker implements IExportManagerFilePicker { return; } - const notebookFileName = defaultFileName + const targetFileName = defaultFileName ? defaultFileName - : path.basename(source.fsPath, path.extname(source.fsPath)); + : `${path.basename(source.fsPath, path.extname(source.fsPath))}${extension}`; - const dialogUri = Uri.file(path.join(this.getLastFileSaveLocation().fsPath, notebookFileName)); + const dialogUri = Uri.file(path.join(this.getLastFileSaveLocation().fsPath, targetFileName)); const options: SaveDialogOptions = { defaultUri: dialogUri, saveLabel: 'Export', diff --git a/src/client/datascience/export/exportUtil.ts b/src/client/datascience/export/exportUtil.ts index d7fe774d9923..f637bd8bd25c 100644 --- a/src/client/datascience/export/exportUtil.ts +++ b/src/client/datascience/export/exportUtil.ts @@ -6,13 +6,12 @@ import * as uuid from 'uuid/v4'; import { CancellationTokenSource, Uri } from 'vscode'; import { IFileSystem, TemporaryDirectory } from '../../common/platform/types'; import { sleep } from '../../common/utils/async'; -import { ICell, IDataScienceErrorHandler, INotebookExporter, INotebookModel, INotebookStorage } from '../types'; +import { ICell, INotebookExporter, INotebookModel, INotebookStorage } from '../types'; @injectable() export class ExportUtil { constructor( @inject(IFileSystem) private fileSystem: IFileSystem, - @inject(IDataScienceErrorHandler) private readonly errorHandler: IDataScienceErrorHandler, @inject(INotebookStorage) private notebookStorage: INotebookStorage, @inject(INotebookExporter) private jupyterExporter: INotebookExporter ) {} @@ -44,12 +43,7 @@ export class ExportUtil { public async makeFileInDirectory(model: INotebookModel, fileName: string, dirPath: string): Promise { const newFilePath = path.join(dirPath, fileName); - try { - const content = model ? model.getContent() : ''; - await this.fileSystem.writeFile(newFilePath, content, 'utf-8'); - } catch (e) { - await this.errorHandler.handleError(e); - } + await this.fileSystem.writeFile(newFilePath, model.getContent(), 'utf-8'); return newFilePath; } From b1eb6f159283b1735d427c31ea822704b0b26548 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 10 Jul 2020 11:05:38 -0700 Subject: [PATCH 08/11] Add trust feature for VS Code Notebooks (#12818) --- build/ci/vscode-python-pr-validation.yaml | 1 + package.json | 43 +++ package.nls.json | 1 + resources/dark/trusted.svg | 4 + resources/dark/un-trusted.svg | 4 + resources/light/run-file.svg | 5 +- resources/light/trusted.svg | 4 + resources/light/un-trusted.svg | 4 + src/client/common/application/commands.ts | 2 + src/client/datascience/constants.ts | 2 + .../context/activeEditorContext.ts | 2 +- src/client/datascience/export/exportUtil.ts | 4 +- .../interactive-ipynb/nativeEditor.ts | 48 +-- .../interactive-ipynb/nativeEditorProvider.ts | 2 +- .../interactive-ipynb/nativeEditorStorage.ts | 6 +- .../notebookStorageProvider.ts | 10 +- .../interactive-ipynb/trustCommandHandler.ts | 76 +++++ .../datascience/notebook/contentProvider.ts | 10 +- .../datascience/notebook/executionService.ts | 2 +- .../notebook/helpers/cellUpdateHelpers.ts | 51 ++- .../datascience/notebook/helpers/helpers.ts | 54 ++-- .../notebook/notebookEditorProvider.ts | 7 +- .../notebook/notebookTrustHandler.ts | 38 +++ .../datascience/notebook/serviceRegistry.ts | 5 + src/client/datascience/serviceRegistry.ts | 2 + src/client/datascience/types.ts | 4 +- src/test/datascience/.vscode/settings.json | 3 +- .../datascience/export/exportUtil.test.ts | 2 +- .../trustCommandHandler.unit.test.ts | 127 ++++++++ .../trustService.unit.test.ts | 12 +- .../nativeEditorStorage.unit.test.ts | 20 +- .../notebook/contentProvider.unit.test.ts | 292 +++++++++--------- src/test/datascience/notebook/helper.ts | 80 ++++- .../datascience/notebook/helpers.unit.test.ts | 6 +- .../notebookTrustHandler.unit.test.ts | 142 +++++++++ 35 files changed, 829 insertions(+), 246 deletions(-) create mode 100644 resources/dark/trusted.svg create mode 100644 resources/dark/un-trusted.svg create mode 100644 resources/light/trusted.svg create mode 100644 resources/light/un-trusted.svg create mode 100644 src/client/datascience/interactive-ipynb/trustCommandHandler.ts create mode 100644 src/client/datascience/notebook/notebookTrustHandler.ts create mode 100644 src/test/datascience/interactive-common/trustCommandHandler.unit.test.ts rename src/test/datascience/{ => interactive-common}/trustService.unit.test.ts (84%) create mode 100644 src/test/datascience/notebook/notebookTrustHandler.unit.test.ts diff --git a/build/ci/vscode-python-pr-validation.yaml b/build/ci/vscode-python-pr-validation.yaml index 1d60dd550c82..f46d58cbef18 100644 --- a/build/ci/vscode-python-pr-validation.yaml +++ b/build/ci/vscode-python-pr-validation.yaml @@ -56,6 +56,7 @@ stages: 'DataScience': TestsToRun: 'testDataScience' NeedsPythonTestReqs: true + NeedsPythonFunctionalReqs: true 'Smoke': TestsToRun: 'testSmoke' NeedsPythonTestReqs: true diff --git a/package.json b/package.json index 2c04f6e8c16b..1a7fdca8262e 100644 --- a/package.json +++ b/package.json @@ -694,6 +694,25 @@ }, "enablement": "python.datascience.notebookeditor.canrestartNotebookkernel" }, + { + "command": "python.datascience.notebookeditor.trust", + "title": "%DataScience.trustNotebookCommandTitle%", + "category": "Python", + "icon": { + "light": "resources/light/un-trusted.svg", + "dark": "resources/dark/un-trusted.svg" + }, + "enablement": "notebookEditorFocused && !python.datascience.isnotebooktrusted && python.datascience.trustfeatureenabled" + }, + { + "command": "python.datascience.notebookeditor.trusted", + "title": "%DataScience.notebookIsTrusted%", + "category": "Python", + "icon": { + "light": "resources/light/trusted.svg", + "dark": "resources/dark/trusted.svg" + } + }, { "command": "python.datascience.notebookeditor.runallcells", "title": "%python.command.python.datascience.notebookeditor.runallcells.title%", @@ -860,6 +879,18 @@ "group": "navigation", "when": "notebookEditorFocused" }, + { + "command": "python.datascience.notebookeditor.trust", + "title": "%DataScience.trustNotebookCommandTitle%", + "group": "navigation@1", + "when": "notebookEditorFocused && !python.datascience.isnotebooktrusted && python.datascience.trustfeatureenabled" + }, + { + "command": "python.datascience.notebookeditor.trusted", + "title": "%DataScience.notebookIsTrusted%", + "group": "navigation@1", + "when": "notebookEditorFocused && python.datascience.isnotebooktrusted && python.datascience.trustfeatureenabled" + }, { "command": "python.datascience.export", "title": "%DataScience.notebookExportAs%", @@ -1138,6 +1169,18 @@ "category": "Python", "when": "python.datascience.isnativeactive && python.datascience.featureenabled && python.datascience.isnotebooktrusted" }, + { + "command": "python.datascience.notebookeditor.trust", + "title": "%DataScience.trustNotebookCommandTitle%", + "category": "Python", + "when": "python.datascience.featureenabled && notebookEditorFocused && !python.datascience.isnotebooktrusted && python.datascience.trustfeatureenabled" + }, + { + "command": "python.datascience.notebookeditor.trusted", + "title": "%DataScience.notebookIsTrusted%", + "category": "Python", + "when": "config.noExists" + }, { "command": "python.datascience.notebookeditor.runallcells", "title": "%python.command.python.datascience.notebookeditor.runallcells.title%", diff --git a/package.nls.json b/package.nls.json index 244a4ffefc9b..b8941cffecd4 100644 --- a/package.nls.json +++ b/package.nls.json @@ -370,6 +370,7 @@ "DataScience.fetchingDataViewer": "Fetching data ...", "DataScience.noRowsInDataViewer": "No rows match current filter", "DataScience.jupyterServer": "Jupyter Server", + "DataScience.trustNotebookCommandTitle": "Trust notebook", "DataScience.notebookIsTrusted": "Trusted", "DataScience.notebookIsNotTrusted": "Not Trusted", "DataScience.noKernel": "No Kernel", diff --git a/resources/dark/trusted.svg b/resources/dark/trusted.svg new file mode 100644 index 000000000000..a841903129b0 --- /dev/null +++ b/resources/dark/trusted.svg @@ -0,0 +1,4 @@ + + + + diff --git a/resources/dark/un-trusted.svg b/resources/dark/un-trusted.svg new file mode 100644 index 000000000000..f97f5f561d9e --- /dev/null +++ b/resources/dark/un-trusted.svg @@ -0,0 +1,4 @@ + + + diff --git a/resources/light/run-file.svg b/resources/light/run-file.svg index f41a5c8fa2d8..0adc9e411b03 100644 --- a/resources/light/run-file.svg +++ b/resources/light/run-file.svg @@ -1,3 +1,4 @@ - - + + diff --git a/resources/light/trusted.svg b/resources/light/trusted.svg new file mode 100644 index 000000000000..82e41b9ff47b --- /dev/null +++ b/resources/light/trusted.svg @@ -0,0 +1,4 @@ + + + diff --git a/resources/light/un-trusted.svg b/resources/light/un-trusted.svg new file mode 100644 index 000000000000..c9be7cd88dda --- /dev/null +++ b/resources/light/un-trusted.svg @@ -0,0 +1,4 @@ + + + diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 17c73a6e3a43..dc310105fdb1 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -183,4 +183,6 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu [DSCommands.OpenNotebookNonCustomEditor]: [Uri]; [DSCommands.GatherQuality]: [string]; [DSCommands.EnableLoadingWidgetsFrom3rdPartySource]: [undefined | never]; + [DSCommands.TrustNotebook]: [undefined | never | Uri]; + [DSCommands.TrustedNotebook]: [undefined | never]; } diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index 7967ba05ce8b..7d7b732a1e4c 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -90,6 +90,8 @@ export namespace Commands { export const SaveAsNotebookNonCustomEditor = 'python.datascience.notebookeditor.saveAs'; export const OpenNotebookNonCustomEditor = 'python.datascience.notebookeditor.open'; export const GatherQuality = 'python.datascience.gatherquality'; + export const TrustNotebook = 'python.datascience.notebookeditor.trust'; + export const TrustedNotebook = 'python.datascience.notebookeditor.trusted'; export const EnableLoadingWidgetsFrom3rdPartySource = 'python.datascience.enableLoadingWidgetScriptsFromThirdPartySource'; } diff --git a/src/client/datascience/context/activeEditorContext.ts b/src/client/datascience/context/activeEditorContext.ts index 39d7ad76cdcf..103021f29481 100644 --- a/src/client/datascience/context/activeEditorContext.ts +++ b/src/client/datascience/context/activeEditorContext.ts @@ -122,7 +122,7 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer .getOrCreateNotebook({ identity: activeEditor.file, getOnly: true }) .then((nb) => { if (activeEditor === this.notebookEditorProvider.activeEditor) { - const canStart = nb && nb.status !== ServerStatus.NotStarted; + const canStart = nb && nb.status !== ServerStatus.NotStarted && activeEditor.model?.isTrusted; this.canRestartNotebookKernelContext.set(!!canStart).ignoreErrors(); } }) diff --git a/src/client/datascience/export/exportUtil.ts b/src/client/datascience/export/exportUtil.ts index f637bd8bd25c..48bcf44f1535 100644 --- a/src/client/datascience/export/exportUtil.ts +++ b/src/client/datascience/export/exportUtil.ts @@ -57,7 +57,7 @@ export class ExportUtil { await this.jupyterExporter.exportToFile(cells, tempFile.filePath, false); const newPath = path.join(tempDir.path, '.ipynb'); await this.fileSystem.copyFile(tempFile.filePath, newPath); - model = await this.notebookStorage.load(Uri.file(newPath)); + model = await this.notebookStorage.get(Uri.file(newPath)); } finally { tempFile.dispose(); tempDir.dispose(); @@ -67,7 +67,7 @@ export class ExportUtil { } public async removeSvgs(source: Uri) { - const model = await this.notebookStorage.load(source); + const model = await this.notebookStorage.get(source); const newCells: ICell[] = []; for (const cell of model.cells) { diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index c40a543ad097..0ae442b19afb 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -6,7 +6,6 @@ import * as path from 'path'; import { CancellationToken, CancellationTokenSource, - commands, Event, EventEmitter, Memento, @@ -98,7 +97,6 @@ import { KernelSwitcher } from '../jupyter/kernels/kernelSwitcher'; const nativeEditorDir = path.join(EXTENSION_ROOT_DIR, 'out', 'datascience-ui', 'notebook'); @injectable() export class NativeEditor extends InteractiveBase implements INotebookEditor { - public readonly type: 'old' | 'custom' = 'custom'; public get onDidChangeViewState(): Event { return this._onDidChangeViewState.event; } @@ -140,6 +138,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { public get isDirty(): boolean { return this.model ? this.model.isDirty : false; } + public readonly type: 'old' | 'custom' = 'custom'; public model: Readonly | undefined; protected savedEvent: EventEmitter = new EventEmitter(); protected closedEvent: EventEmitter = new EventEmitter(); @@ -152,6 +151,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { private startupTimer: StopWatch = new StopWatch(); private loadedAllCells: boolean = false; private executeCancelTokens = new Set(); + private previouslyNotTrusted: boolean = false; constructor( @multiInject(IInteractiveWindowListener) listeners: IInteractiveWindowListener[], @@ -227,6 +227,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { ); asyncRegistry.push(this); + asyncRegistry.push(this.trustService.onDidSetNotebookTrust(this.monitorChangesToTrust, this)); this.synchronizer.subscribeToUserActions(this, this.postMessage.bind(this)); } @@ -251,6 +252,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { // Sign up for dirty events model.changed(this.modelChanged.bind(this)); + this.previouslyNotTrusted = model.isTrusted; } // tslint:disable-next-line: no-any @@ -596,7 +598,13 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } } } - + private async monitorChangesToTrust() { + if (this.previouslyNotTrusted && this.model?.isTrusted) { + this.previouslyNotTrusted = false; + // Tell UI to update main state + this.postMessage(InteractiveWindowMessages.TrustNotebookComplete).ignoreErrors(); + } + } private renameVariableExplorerHeights(name: string, updatedName: string) { // Updates the workspace storage to reflect the updated name of the notebook // should be called if the name of the notebook changes @@ -612,38 +620,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } private async launchNotebookTrustPrompt() { - const prompts = [ - localize.DataScience.trustNotebook(), - localize.DataScience.doNotTrustNotebook(), - localize.DataScience.trustAllNotebooks() - ]; - const selection = await this.applicationShell.showErrorMessage( - localize.DataScience.launchNotebookTrustPrompt(), - ...prompts - ); - if (!selection) { - return; - } - if (this.model && selection === localize.DataScience.trustNotebook() && !this.model.isTrusted) { - try { - const contents = this.model.getContent(); - await this.trustService.trustNotebook(this.model.file, contents); - // Update model trust - this.model.update({ - source: 'user', - kind: 'updateTrust', - oldDirty: this.model.isDirty, - newDirty: this.model.isDirty, - isNotebookTrusted: true - }); - // Tell UI to update main state - await this.postMessage(InteractiveWindowMessages.TrustNotebookComplete); - } catch (err) { - traceError(err); - } - } else if (selection === localize.DataScience.trustAllNotebooks()) { - // Take the user to the settings UI where they can manually turn on the alwaysTrustNotebooks setting - commands.executeCommand('workbench.action.openSettings', 'python.dataScience.alwaysTrustNotebooks'); + if (this.model && !this.model.isTrusted) { + await this.commandManager.executeCommand(Commands.TrustNotebook, this.model.file); } } diff --git a/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts b/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts index 53e1a39506b7..2b0445be4cf5 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditorProvider.ts @@ -184,7 +184,7 @@ export class NativeEditorProvider implements INotebookEditorProvider, CustomEdit this.untitledCounter = getNextUntitledCounter(file, this.untitledCounter); // Load our model from our storage object. - const model = await this.storage.load(file, contents, options); + const model = await this.storage.get(file, contents, options); // Make sure to listen to events on the model this.trackModel(model); diff --git a/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts b/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts index 0586697ef17b..10f2e29eb8ed 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts @@ -74,20 +74,20 @@ export class NativeEditorStorage implements INotebookStorage { return `${path.basename(model.file.fsPath)}-${uuid()}`; } - public load( + public get( file: Uri, possibleContents?: string, backupId?: string, forVSCodeNotebook?: boolean ): Promise; - public load( + public get( file: Uri, possibleContents?: string, // tslint:disable-next-line: unified-signatures skipDirtyContents?: boolean, forVSCodeNotebook?: boolean ): Promise; - public load( + public get( file: Uri, possibleContents?: string, // tslint:disable-next-line: no-any diff --git a/src/client/datascience/interactive-ipynb/notebookStorageProvider.ts b/src/client/datascience/interactive-ipynb/notebookStorageProvider.ts index 0461c7e5f6ea..5ffb2814f4f4 100644 --- a/src/client/datascience/interactive-ipynb/notebookStorageProvider.ts +++ b/src/client/datascience/interactive-ipynb/notebookStorageProvider.ts @@ -56,8 +56,8 @@ export class NotebookStorageProvider implements INotebookStorageProvider { public deleteBackup(model: INotebookModel, backupId?: string) { return this.storage.deleteBackup(model, backupId); } - public load(file: Uri, contents?: string, backupId?: string, forVSCodeNotebook?: boolean): Promise; - public load( + public get(file: Uri, contents?: string, backupId?: string, forVSCodeNotebook?: boolean): Promise; + public get( file: Uri, contents?: string, // tslint:disable-next-line: unified-signatures @@ -66,7 +66,7 @@ export class NotebookStorageProvider implements INotebookStorageProvider { ): Promise; // tslint:disable-next-line: no-any - public load(file: Uri, contents?: string, options?: any, forVSCodeNotebook?: boolean): Promise { + public get(file: Uri, contents?: string, options?: any, forVSCodeNotebook?: boolean): Promise { const key = file.toString(); if (!this.storageAndModels.has(key)) { // Every time we load a new untitled file, up the counter past the max value for this counter @@ -74,7 +74,7 @@ export class NotebookStorageProvider implements INotebookStorageProvider { file, NotebookStorageProvider.untitledCounter ); - const promise = this.storage.load(file, contents, options, forVSCodeNotebook); + const promise = this.storage.get(file, contents, options, forVSCodeNotebook); this.storageAndModels.set(key, promise.then(this.trackModel.bind(this))); } return this.storageAndModels.get(key)!; @@ -90,7 +90,7 @@ export class NotebookStorageProvider implements INotebookStorageProvider { const uri = this.getNextNewNotebookUri(forVSCodeNotebooks); // Always skip loading from the hot exit file. When creating a new file we want a new file. - return this.load(uri, contents, true); + return this.get(uri, contents, true); } private getNextNewNotebookUri(forVSCodeNotebooks?: boolean): Uri { diff --git a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts new file mode 100644 index 000000000000..09ee31f49e62 --- /dev/null +++ b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts @@ -0,0 +1,76 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import { Uri } from 'vscode'; +import { IExtensionSingleActivationService } from '../../activation/types'; +import { IApplicationShell, ICommandManager } from '../../common/application/types'; +import { ContextKey } from '../../common/contextKey'; +import { EnableTrustedNotebooks } from '../../common/experiments/groups'; +import '../../common/extensions'; +import { IDisposableRegistry, IExperimentService } from '../../common/types'; +import { swallowExceptions } from '../../common/utils/decorators'; +import { DataScience } from '../../common/utils/localize'; +import { noop } from '../../common/utils/misc'; +import { Commands } from '../constants'; +import { INotebookStorageProvider } from '../interactive-ipynb/notebookStorageProvider'; +import { INotebookEditorProvider, ITrustService } from '../types'; + +@injectable() +export class TrustCommandHandler implements IExtensionSingleActivationService { + constructor( + @inject(ITrustService) private readonly trustService: ITrustService, + @inject(INotebookEditorProvider) private readonly editorProvider: INotebookEditorProvider, + @inject(INotebookStorageProvider) private readonly storageProvider: INotebookStorageProvider, + @inject(ICommandManager) private readonly commandManager: ICommandManager, + @inject(IApplicationShell) private readonly applicationShell: IApplicationShell, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + @inject(IExperimentService) private readonly experiments: IExperimentService + ) {} + public async activate(): Promise { + this.activateInBackground().ignoreErrors(); + } + public async activateInBackground(): Promise { + if (!(await this.experiments.inExperiment(EnableTrustedNotebooks.experiment))) { + return; + } + const context = new ContextKey('python.datascience.trustfeatureenabled', this.commandManager); + context.set(true).ignoreErrors(); + this.disposables.push(this.commandManager.registerCommand(Commands.TrustNotebook, this.onTrustNotebook, this)); + this.disposables.push(this.commandManager.registerCommand(Commands.TrustedNotebook, noop)); + } + @swallowExceptions('Trusting notebook') + private async onTrustNotebook(uri?: Uri) { + uri = uri ?? this.editorProvider.activeEditor?.file; + if (!uri) { + return; + } + + const model = await this.storageProvider.get(uri); + if (model.isTrusted) { + return; + } + + const selection = await this.applicationShell.showErrorMessage( + DataScience.launchNotebookTrustPrompt(), + DataScience.trustNotebook(), + DataScience.doNotTrustNotebook(), + DataScience.trustAllNotebooks() + ); + if (selection !== DataScience.trustNotebook() || model.isTrusted) { + return; + } + // Update model trust + model.update({ + source: 'user', + kind: 'updateTrust', + oldDirty: model.isDirty, + newDirty: model.isDirty, + isNotebookTrusted: true + }); + const contents = model.getContent(); + await this.trustService.trustNotebook(model.file, contents); + } +} diff --git a/src/client/datascience/notebook/contentProvider.ts b/src/client/datascience/notebook/contentProvider.ts index 3e48f4c88e84..622298bf7cd1 100644 --- a/src/client/datascience/notebook/contentProvider.ts +++ b/src/client/datascience/notebook/contentProvider.ts @@ -72,8 +72,8 @@ export class NotebookContentProvider implements INotebookContentProvider { } // If there's no backup id, then skip loading dirty contents. const model = await (openContext.backupId - ? this.notebookStorage.load(uri, undefined, openContext.backupId, true) - : this.notebookStorage.load(uri, undefined, true, true)); + ? this.notebookStorage.get(uri, undefined, openContext.backupId, true) + : this.notebookStorage.get(uri, undefined, true, true)); setSharedProperty('ds_notebookeditor', 'native'); sendTelemetryEvent(Telemetry.CellCount, undefined, { count: model.cells.length }); @@ -81,7 +81,7 @@ export class NotebookContentProvider implements INotebookContentProvider { } @captureTelemetry(Telemetry.Save, undefined, true) public async saveNotebook(document: NotebookDocument, cancellation: CancellationToken) { - const model = await this.notebookStorage.load(document.uri, undefined, undefined, true); + const model = await this.notebookStorage.get(document.uri, undefined, undefined, true); if (cancellation.isCancellationRequested) { return; } @@ -97,7 +97,7 @@ export class NotebookContentProvider implements INotebookContentProvider { document: NotebookDocument, cancellation: CancellationToken ): Promise { - const model = await this.notebookStorage.load(document.uri, undefined, undefined, true); + const model = await this.notebookStorage.get(document.uri, undefined, undefined, true); if (!cancellation.isCancellationRequested) { await this.notebookStorage.saveAs(model, targetResource); } @@ -107,7 +107,7 @@ export class NotebookContentProvider implements INotebookContentProvider { _context: NotebookDocumentBackupContext, cancellation: CancellationToken ): Promise { - const model = await this.notebookStorage.load(document.uri, undefined, undefined, true); + const model = await this.notebookStorage.get(document.uri, undefined, undefined, true); const id = this.notebookStorage.generateBackupId(model); await this.notebookStorage.backup(model, cancellation, id); return { diff --git a/src/client/datascience/notebook/executionService.ts b/src/client/datascience/notebook/executionService.ts index 95b2bc3fdba6..613c29a4e767 100644 --- a/src/client/datascience/notebook/executionService.ts +++ b/src/client/datascience/notebook/executionService.ts @@ -144,7 +144,7 @@ export class NotebookExecutionService implements INotebookExecutionService { private async getNotebookAndModel( document: NotebookDocument ): Promise<{ model: VSCodeNotebookModel; nb: INotebook }> { - const model = await this.notebookStorage.load(document.uri, undefined, undefined, true); + const model = await this.notebookStorage.get(document.uri, undefined, undefined, true); const nb = await this.notebookProvider.getOrCreateNotebook({ identity: document.uri, resource: document.uri, diff --git a/src/client/datascience/notebook/helpers/cellUpdateHelpers.ts b/src/client/datascience/notebook/helpers/cellUpdateHelpers.ts index c0d9274e2813..eb91df59f389 100644 --- a/src/client/datascience/notebook/helpers/cellUpdateHelpers.ts +++ b/src/client/datascience/notebook/helpers/cellUpdateHelpers.ts @@ -10,7 +10,7 @@ */ import * as assert from 'assert'; -import { NotebookCell } from '../../../../../types/vscode-proposed'; +import { NotebookCell, NotebookDocument } from '../../../../../types/vscode-proposed'; import { NotebookCellLanguageChangeEvent, NotebookCellOutputsChangeEvent, @@ -20,8 +20,13 @@ import { traceError } from '../../../common/logger'; import { sendTelemetryEvent } from '../../../telemetry'; import { VSCodeNativeTelemetry } from '../../constants'; import { VSCodeNotebookModel } from '../../notebookStorage/vscNotebookModel'; +import { INotebookModel } from '../../types'; import { findMappedNotebookCellModel } from './cellMappers'; -import { createCellFromVSCNotebookCell, updateVSCNotebookCellMetadata } from './helpers'; +import { + createCellFromVSCNotebookCell, + createVSCCellOutputsFromOutputs, + updateVSCNotebookCellMetadata +} from './helpers'; // tslint:disable-next-line: no-var-requires no-require-imports const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed'); @@ -51,6 +56,48 @@ export function updateCellModelWithChangesToVSCCell( } } +/** + * Updates a notebook document as a result of trusting it. + */ +export function updateVSCNotebookAfterTrustingNotebook(document: NotebookDocument, model: INotebookModel) { + const areAllCellsEditableAndRunnable = document.cells.every((cell) => { + if (cell.cellKind === vscodeNotebookEnums.CellKind.Markdown) { + return cell.metadata.editable; + } else { + return cell.metadata.editable && cell.metadata.runnable; + } + }); + const isDocumentEditableAndRunnable = + document.metadata.cellEditable && + document.metadata.cellRunnable && + document.metadata.editable && + document.metadata.runnable; + + // If already trusted, then nothing to do. + if (isDocumentEditableAndRunnable && areAllCellsEditableAndRunnable) { + return; + } + + document.metadata.cellEditable = true; + document.metadata.cellRunnable = true; + document.metadata.editable = true; + document.metadata.runnable = true; + + document.cells.forEach((cell) => { + cell.metadata.editable = true; + if (cell.cellKind !== vscodeNotebookEnums.CellKind.Markdown) { + cell.metadata.runnable = true; + } + + // Restore the output once we trust the notebook. + const cellModel = findMappedNotebookCellModel(cell, model.cells); + if (cellModel) { + // tslint:disable-next-line: no-any + cell.outputs = createVSCCellOutputsFromOutputs(cellModel.data.outputs as any); + } + }); +} + export function clearCellForExecution(vscCell: NotebookCell, model: VSCodeNotebookModel) { vscCell.metadata.statusMessage = undefined; vscCell.metadata.executionOrder = undefined; diff --git a/src/client/datascience/notebook/helpers/helpers.ts b/src/client/datascience/notebook/helpers/helpers.ts index 99487d2f94b4..40bfc4085468 100644 --- a/src/client/datascience/notebook/helpers/helpers.ts +++ b/src/client/datascience/notebook/helpers/helpers.ts @@ -26,12 +26,12 @@ import { createCodeCell, createMarkdownCell } from '../../../../datascience-ui/c import { MARKDOWN_LANGUAGE, PYTHON_LANGUAGE } from '../../../common/constants'; import { traceError, traceWarning } from '../../../common/logger'; import { CellState, ICell, INotebookModel } from '../../types'; +import { JupyterNotebookView } from '../constants'; import { mapVSCNotebookCellToCellModel } from './cellMappers'; // tslint:disable-next-line: no-var-requires no-require-imports const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed'); // tslint:disable-next-line: no-require-imports import cloneDeep = require('lodash/cloneDeep'); -import { JupyterNotebookView } from '../constants'; // This is the custom type we are adding into nbformat.IBaseCellMetadata interface IBaseCellVSCodeMetadata { @@ -61,11 +61,11 @@ export function notebookModelToVSCNotebookData(model: INotebookModel): NotebookD cells, languages: [defaultLanguage], metadata: { - cellEditable: true, - cellRunnable: true, - editable: true, + cellEditable: model.isTrusted, + cellRunnable: model.isTrusted, + editable: model.isTrusted, cellHasExecutionOrder: true, - runnable: true, + runnable: model.isTrusted, displayOrder: [ 'application/vnd.*', 'application/vdom.*', @@ -172,23 +172,16 @@ export function createVSCNotebookCellDataFromCell(model: INotebookModel, cell: I runState = vscodeNotebookEnums.NotebookCellRunState.Success; } - const notebookCellData: NotebookCellData = { - cellKind: - cell.data.cell_type === 'code' ? vscodeNotebookEnums.CellKind.Code : vscodeNotebookEnums.CellKind.Markdown, - language: cell.data.cell_type === 'code' ? defaultCodeLanguage : MARKDOWN_LANGUAGE, - metadata: { - editable: true, - executionOrder: typeof cell.data.execution_count === 'number' ? cell.data.execution_count : undefined, - hasExecutionOrder: cell.data.cell_type === 'code', - runState, - runnable: cell.data.cell_type === 'code' - }, - source: concatMultilineStringInput(cell.data.source), - outputs + const notebookCellMetadata: NotebookCellMetadata = { + editable: model.isTrusted, + executionOrder: typeof cell.data.execution_count === 'number' ? cell.data.execution_count : undefined, + hasExecutionOrder: cell.data.cell_type === 'code', + runState, + runnable: cell.data.cell_type === 'code' && model.isTrusted }; if (statusMessage) { - notebookCellData.metadata.statusMessage = statusMessage; + notebookCellMetadata.statusMessage = statusMessage; } const vscodeMetadata = (cell.data.metadata.vscode as unknown) as IBaseCellVSCodeMetadata | undefined; const startExecutionTime = vscodeMetadata?.start_execution_time @@ -199,12 +192,27 @@ export function createVSCNotebookCellDataFromCell(model: INotebookModel, cell: I : undefined; if (startExecutionTime && typeof endExecutionTime === 'number') { - notebookCellData.metadata.runStartTime = startExecutionTime; - notebookCellData.metadata.lastRunDuration = endExecutionTime - startExecutionTime; + notebookCellMetadata.runStartTime = startExecutionTime; + notebookCellMetadata.lastRunDuration = endExecutionTime - startExecutionTime; } - updateVSCNotebookCellMetadata(notebookCellData.metadata, cell); - return notebookCellData; + updateVSCNotebookCellMetadata(notebookCellMetadata, cell); + + // If not trusted, then clear the output in VSC Cell. + // At this point we have the original output in the ICell. + if (!model.isTrusted) { + while (outputs.length) { + outputs.shift(); + } + } + return { + cellKind: + cell.data.cell_type === 'code' ? vscodeNotebookEnums.CellKind.Code : vscodeNotebookEnums.CellKind.Markdown, + language: cell.data.cell_type === 'code' ? defaultCodeLanguage : MARKDOWN_LANGUAGE, + metadata: notebookCellMetadata, + source: concatMultilineStringInput(cell.data.source), + outputs + }; } export function createVSCCellOutputsFromOutputs(outputs?: nbformat.IOutput[]): CellOutput[] { diff --git a/src/client/datascience/notebook/notebookEditorProvider.ts b/src/client/datascience/notebook/notebookEditorProvider.ts index cdf3c0614742..bfe618d46d71 100644 --- a/src/client/datascience/notebook/notebookEditorProvider.ts +++ b/src/client/datascience/notebook/notebookEditorProvider.ts @@ -152,7 +152,7 @@ export class NotebookEditorProvider implements INotebookEditorProvider { return; } const uri = doc.uri; - const model = await this.storage.load(uri, undefined, undefined, true); + const model = await this.storage.get(uri, undefined, undefined, true); mapVSCNotebookCellsToNotebookCellModels(doc, model); // In open method we might be waiting. let editor = this.notebookEditorsByUri.get(uri.toString()); @@ -180,6 +180,9 @@ export class NotebookEditorProvider implements INotebookEditorProvider { const deferred = this.notebooksWaitingToBeOpenedByUri.get(uri.toString())!; deferred.resolve(editor); this.notebookEditorsByUri.set(uri.toString(), editor); + if (!model.isTrusted) { + await this.commandManager.executeCommand(Commands.TrustNotebook, model.file); + } } private onDidChangeActiveVsCodeNotebookEditor(editor: VSCodeNotebookEditor | undefined) { if (!editor) { @@ -229,7 +232,7 @@ export class NotebookEditorProvider implements INotebookEditorProvider { if (!isJupyterNotebook(e.document)) { return; } - const model = await this.storage.load(e.document.uri, undefined, undefined, true); + const model = await this.storage.get(e.document.uri, undefined, undefined, true); if (!(model instanceof VSCodeNotebookModel)) { throw new Error('NotebookModel not of type VSCodeNotebookModel'); } diff --git a/src/client/datascience/notebook/notebookTrustHandler.ts b/src/client/datascience/notebook/notebookTrustHandler.ts new file mode 100644 index 000000000000..06b2a864ef33 --- /dev/null +++ b/src/client/datascience/notebook/notebookTrustHandler.ts @@ -0,0 +1,38 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import { IExtensionSingleActivationService } from '../../activation/types'; +import { IVSCodeNotebook } from '../../common/application/types'; +import { IFileSystem } from '../../common/platform/types'; +import { IDisposableRegistry } from '../../common/types'; +import { INotebookEditorProvider, ITrustService } from '../types'; +import { updateVSCNotebookAfterTrustingNotebook } from './helpers/cellUpdateHelpers'; +import { isJupyterNotebook } from './helpers/helpers'; + +@injectable() +export class NotebookTrustHandler implements IExtensionSingleActivationService { + constructor( + @inject(ITrustService) private readonly trustService: ITrustService, + @inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook, + @inject(INotebookEditorProvider) private readonly editorProvider: INotebookEditorProvider, + @inject(IFileSystem) private readonly fs: IFileSystem, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry + ) {} + public async activate(): Promise { + this.trustService.onDidSetNotebookTrust(this.onDidTrustNotebook, this, this.disposables); + } + private onDidTrustNotebook() { + this.vscNotebook.notebookDocuments.forEach((doc) => { + if (!isJupyterNotebook(doc)) { + return; + } + const editor = this.editorProvider.editors.find((e) => this.fs.arePathsSame(e.file.fsPath, doc.uri.fsPath)); + if (editor && editor.model?.isTrusted) { + updateVSCNotebookAfterTrustingNotebook(doc, editor.model); + } + }); + } +} diff --git a/src/client/datascience/notebook/serviceRegistry.ts b/src/client/datascience/notebook/serviceRegistry.ts index d8b1cea92b4f..6e53197efcb7 100644 --- a/src/client/datascience/notebook/serviceRegistry.ts +++ b/src/client/datascience/notebook/serviceRegistry.ts @@ -10,6 +10,7 @@ import { NotebookContentProvider } from './contentProvider'; import { NotebookExecutionService } from './executionService'; import { NotebookIntegration } from './integration'; import { NotebookKernel } from './notebookKernel'; +import { NotebookTrustHandler } from './notebookTrustHandler'; import { NotebookOutputRenderer } from './renderer'; import { NotebookSurveyBanner, NotebookSurveyDataLogger } from './survey'; import { INotebookContentProvider, INotebookExecutionService } from './types'; @@ -33,4 +34,8 @@ export function registerTypes(serviceManager: IServiceManager) { IExtensionSingleActivationService, NotebookSurveyDataLogger ); + serviceManager.addSingleton( + IExtensionSingleActivationService, + NotebookTrustHandler + ); } diff --git a/src/client/datascience/serviceRegistry.ts b/src/client/datascience/serviceRegistry.ts index 8fa61793f59b..31bfd9a717b3 100644 --- a/src/client/datascience/serviceRegistry.ts +++ b/src/client/datascience/serviceRegistry.ts @@ -66,6 +66,7 @@ import { NativeEditorStorage } from './interactive-ipynb/nativeEditorStorage'; import { NativeEditorSynchronizer } from './interactive-ipynb/nativeEditorSynchronizer'; import { NativeEditorViewTracker } from './interactive-ipynb/nativeEditorViewTracker'; import { INotebookStorageProvider, NotebookStorageProvider } from './interactive-ipynb/notebookStorageProvider'; +import { TrustCommandHandler } from './interactive-ipynb/trustCommandHandler'; import { TrustService } from './interactive-ipynb/trustService'; import { InteractiveWindow } from './interactive-window/interactiveWindow'; import { InteractiveWindowCommandListener } from './interactive-window/interactiveWindowCommandListener'; @@ -246,6 +247,7 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IExtensionSingleActivationService, ServerPreload); serviceManager.addSingleton(IExtensionSingleActivationService, NativeEditorViewTracker); serviceManager.addSingleton(IExtensionSingleActivationService, NotebookUsageTracker); + serviceManager.addSingleton(IExtensionSingleActivationService, TrustCommandHandler); serviceManager.addSingleton(IInteractiveWindowListener, DataScienceSurveyBannerLogger); serviceManager.addSingleton(IInteractiveWindowProvider, InteractiveWindowProvider); serviceManager.addSingleton(IJupyterDebugger, JupyterDebugger, undefined, [ICellHashListener]); diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 65259318741f..d2b4142ee7f6 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -1043,8 +1043,8 @@ export interface INotebookStorage { save(model: INotebookModel, cancellation: CancellationToken): Promise; saveAs(model: INotebookModel, targetResource: Uri): Promise; backup(model: INotebookModel, cancellation: CancellationToken, backupId?: string): Promise; - load(file: Uri, contents?: string, backupId?: string, forVSCodeNotebook?: boolean): Promise; - load( + get(file: Uri, contents?: string, backupId?: string, forVSCodeNotebook?: boolean): Promise; + get( file: Uri, contents?: string, // tslint:disable-next-line: unified-signatures diff --git a/src/test/datascience/.vscode/settings.json b/src/test/datascience/.vscode/settings.json index 84a3ffb84a43..90f4ac4dc843 100644 --- a/src/test/datascience/.vscode/settings.json +++ b/src/test/datascience/.vscode/settings.json @@ -24,5 +24,6 @@ "files.autoSave": "off", // We don't want jupyter to start when testing (DS functionality or anything else). "python.dataScience.disableJupyterAutoStart": true, - "python.logging.level": "debug" + "python.logging.level": "debug", + "python.dataScience.alwaysTrustNotebooks": true // In UI tests we don't want prompts for `Do you want to trust thie nb...` } diff --git a/src/test/datascience/export/exportUtil.test.ts b/src/test/datascience/export/exportUtil.test.ts index 340828976e7e..a287b67ac84e 100644 --- a/src/test/datascience/export/exportUtil.test.ts +++ b/src/test/datascience/export/exportUtil.test.ts @@ -37,7 +37,7 @@ suite('DataScience - Export Util', () => { ); await exportUtil.removeSvgs(file); - const model = await notebookStorage.load(file); + const model = await notebookStorage.get(file); // make sure no svg exists in model const SVG = 'image/svg+xml'; diff --git a/src/test/datascience/interactive-common/trustCommandHandler.unit.test.ts b/src/test/datascience/interactive-common/trustCommandHandler.unit.test.ts new file mode 100644 index 000000000000..3bb98ca5c813 --- /dev/null +++ b/src/test/datascience/interactive-common/trustCommandHandler.unit.test.ts @@ -0,0 +1,127 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; + +import * as fakeTimers from '@sinonjs/fake-timers'; +import { assert } from 'chai'; +import * as sinon from 'sinon'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { Uri } from 'vscode'; +import { IExtensionSingleActivationService } from '../../../client/activation/types'; +import { IApplicationShell, ICommandManager } from '../../../client/common/application/types'; +import { ContextKey } from '../../../client/common/contextKey'; +import { EnableTrustedNotebooks } from '../../../client/common/experiments/groups'; +import { IDisposable, IExperimentService } from '../../../client/common/types'; +import { DataScience } from '../../../client/common/utils/localize'; +import { Commands } from '../../../client/datascience/constants'; +import { INotebookStorageProvider } from '../../../client/datascience/interactive-ipynb/notebookStorageProvider'; +import { TrustCommandHandler } from '../../../client/datascience/interactive-ipynb/trustCommandHandler'; +import { TrustService } from '../../../client/datascience/interactive-ipynb/trustService'; +import { INotebookEditorProvider, INotebookModel, ITrustService } from '../../../client/datascience/types'; +import { noop } from '../../core'; +import { createNotebookModel, disposeAllDisposables } from '../notebook/helper'; + +// tslint:disable: no-any + +suite('DataScience - Trust Command Handler', () => { + let trustCommandHandler: IExtensionSingleActivationService; + let trustService: ITrustService; + let editorProvider: INotebookEditorProvider; + let storageProvider: INotebookStorageProvider; + let commandManager: ICommandManager; + let applicationShell: IApplicationShell; + let disposables: IDisposable[] = []; + let clock: fakeTimers.InstalledClock; + let contextKeySet: sinon.SinonStub<[boolean], Promise>; + let experiments: IExperimentService; + let model: INotebookModel; + let trustNotebookCommandCallback: (uri: Uri) => Promise; + setup(() => { + trustService = mock(); + editorProvider = mock(); + storageProvider = mock(); + commandManager = mock(); + applicationShell = mock(); + model = createNotebookModel(false, Uri.file('a')); + when(storageProvider.get(anything())).thenResolve(model); + disposables = []; + + experiments = mock(); + + when(trustService.trustNotebook(anything(), anything())).thenResolve(); + when(experiments.inExperiment(anything())).thenCall((exp) => + Promise.resolve(exp === EnableTrustedNotebooks.experiment) + ); + when(commandManager.registerCommand(anything(), anything(), anything())).thenCall(() => ({ dispose: noop })); + when(commandManager.registerCommand(Commands.TrustNotebook, anything(), anything())).thenCall((_, cb) => { + trustNotebookCommandCallback = cb.bind(trustCommandHandler); + return { dispose: noop }; + }); + + trustCommandHandler = new TrustCommandHandler( + instance(trustService), + instance(editorProvider), + instance(storageProvider), + instance(commandManager), + instance(applicationShell), + disposables, + instance(experiments) + ); + + clock = fakeTimers.install(); + + contextKeySet = sinon.stub(ContextKey.prototype, 'set'); + contextKeySet.resolves(); + }); + teardown(() => { + sinon.restore(); + disposeAllDisposables(disposables); + clock.uninstall(); + }); + + test('Context not set if not in experiment', async () => { + when(experiments.inExperiment(anything())).thenResolve(false); + + await trustCommandHandler.activate(); + await clock.runAllAsync(); + + assert.equal(contextKeySet.callCount, 0); + }); + test('Context set if in experiment', async () => { + when(experiments.inExperiment(anything())).thenCall((exp) => + Promise.resolve(exp === EnableTrustedNotebooks.experiment) + ); + + await trustCommandHandler.activate(); + await clock.runAllAsync(); + + assert.equal(contextKeySet.callCount, 1); + }); + test('Executing command will not update trust after dismissing the prompt', async () => { + when(applicationShell.showErrorMessage(anything(), anything(), anything(), anything())).thenResolve( + undefined as any + ); + + await trustCommandHandler.activate(); + await clock.runAllAsync(); + await trustNotebookCommandCallback(Uri.file('a')); + + verify(applicationShell.showErrorMessage(anything(), anything(), anything(), anything())).once(); + verify(trustService.trustNotebook(anything(), anything())).never(); + assert.isFalse(model.isTrusted); + }); + test('Executing command will update trust', async () => { + when(applicationShell.showErrorMessage(anything(), anything(), anything(), anything())).thenResolve( + DataScience.trustNotebook() as any + ); + + assert.isFalse(model.isTrusted); + await trustCommandHandler.activate(); + await clock.runAllAsync(); + await trustNotebookCommandCallback(Uri.file('a')); + + verify(applicationShell.showErrorMessage(anything(), anything(), anything(), anything())).once(); + verify(trustService.trustNotebook(anything(), anything())).once(); + assert.isTrue(model.isTrusted); + }); +}); diff --git a/src/test/datascience/trustService.unit.test.ts b/src/test/datascience/interactive-common/trustService.unit.test.ts similarity index 84% rename from src/test/datascience/trustService.unit.test.ts rename to src/test/datascience/interactive-common/trustService.unit.test.ts index 2ff31348abf6..beb3c7c88fad 100644 --- a/src/test/datascience/trustService.unit.test.ts +++ b/src/test/datascience/interactive-common/trustService.unit.test.ts @@ -8,12 +8,12 @@ import * as os from 'os'; import { anything, instance, mock, when } from 'ts-mockito'; import * as typemoq from 'typemoq'; import { Uri } from 'vscode'; -import { ConfigurationService } from '../../client/common/configuration/service'; -import { ExperimentService } from '../../client/common/experiments/service'; -import { FileSystem } from '../../client/common/platform/fileSystem'; -import { IExtensionContext } from '../../client/common/types'; -import { DigestStorage } from '../../client/datascience/interactive-ipynb/digestStorage'; -import { TrustService } from '../../client/datascience/interactive-ipynb/trustService'; +import { ConfigurationService } from '../../../client/common/configuration/service'; +import { ExperimentService } from '../../../client/common/experiments/service'; +import { FileSystem } from '../../../client/common/platform/fileSystem'; +import { IExtensionContext } from '../../../client/common/types'; +import { DigestStorage } from '../../../client/datascience/interactive-ipynb/digestStorage'; +import { TrustService } from '../../../client/datascience/interactive-ipynb/trustService'; suite('DataScience - TrustService', () => { let trustService: TrustService; diff --git a/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts b/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts index 98b7a9fae312..3db2eecd0ee4 100644 --- a/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts +++ b/src/test/datascience/interactive-ipynb/nativeEditorStorage.unit.test.ts @@ -409,7 +409,7 @@ suite('DataScience - Native Editor Storage', () => { } test('Create new editor and add some cells', async () => { - model = await storage.load(baseUri); + model = await storage.get(baseUri); insertCell(0, '1'); const cells = model.cells; expect(cells).to.be.lengthOf(4); @@ -418,7 +418,7 @@ suite('DataScience - Native Editor Storage', () => { }); test('Move cells around', async () => { - model = await storage.load(baseUri); + model = await storage.get(baseUri); swapCells('NotebookImport#0', 'NotebookImport#1'); const cells = model.cells; expect(cells).to.be.lengthOf(3); @@ -427,7 +427,7 @@ suite('DataScience - Native Editor Storage', () => { }); test('Edit/delete cells', async () => { - model = await storage.load(baseUri); + model = await storage.get(baseUri); expect(model.isDirty).to.be.equal(false, 'Editor should not be dirty'); editCell( [ @@ -467,7 +467,7 @@ suite('DataScience - Native Editor Storage', () => { test('Editing a file and closing will keep contents', async () => { await filesConfig?.update('autoSave', 'off'); - model = await storage.load(baseUri); + model = await storage.get(baseUri); expect(model.isDirty).to.be.equal(false, 'Editor should not be dirty'); editCell( [ @@ -496,7 +496,7 @@ suite('DataScience - Native Editor Storage', () => { // Recreate storage = createStorage(); - model = await storage.load(baseUri); + model = await storage.get(baseUri); const cells = model.cells; expect(cells).to.be.lengthOf(3); @@ -506,7 +506,7 @@ suite('DataScience - Native Editor Storage', () => { }); test('Editing a new file and closing will keep contents', async () => { - model = await storage.load(untiledUri, undefined, true); + model = await storage.get(untiledUri, undefined, true); expect(model.isDirty).to.be.equal(false, 'Editor should not be dirty'); insertCell(0, 'a=1'); @@ -515,7 +515,7 @@ suite('DataScience - Native Editor Storage', () => { // Recreate storage = createStorage(); - model = await storage.load(untiledUri); + model = await storage.get(untiledUri); const cells = model.cells; expect(cells).to.be.lengthOf(2); @@ -534,7 +534,7 @@ suite('DataScience - Native Editor Storage', () => { // Put the regular file into the local storage await localMemento.update(`notebook-storage-${file.toString()}`, differentFile); - model = await storage.load(file); + model = await storage.get(file); // It should load with that value const cells = model.cells; @@ -555,7 +555,7 @@ suite('DataScience - Native Editor Storage', () => { contents: differentFile, lastModifiedTimeMs: Date.now() }); - model = await storage.load(file); + model = await storage.get(file); // It should load with that value const cells = model.cells; @@ -585,7 +585,7 @@ suite('DataScience - Native Editor Storage', () => { lastModifiedTimeMs: Date.now() }); - model = await storage.load(file); + model = await storage.get(file); // It should load with that value const cells = model.cells; diff --git a/src/test/datascience/notebook/contentProvider.unit.test.ts b/src/test/datascience/notebook/contentProvider.unit.test.ts index b7c27604c456..b85a4715ebcf 100644 --- a/src/test/datascience/notebook/contentProvider.unit.test.ts +++ b/src/test/datascience/notebook/contentProvider.unit.test.ts @@ -33,157 +33,173 @@ suite('Data Science - NativeNotebook ContentProvider', () => { instance(compatSupport) ); }); + [true, false].forEach((isNotebookTrusted) => { + suite(isNotebookTrusted ? 'Trusted Notebook' : 'Un-trusted notebook', () => { + test('Return notebook with 2 cells', async () => { + const model: Partial = { + cells: [ + { + data: { + cell_type: 'code', + execution_count: 10, + hasExecutionOrder: true, + outputs: [], + source: 'print(1)', + metadata: {} + }, + file: 'a.ipynb', + id: 'MyCellId1', + line: 0, + state: CellState.init + }, + { + data: { + cell_type: 'markdown', + hasExecutionOrder: false, + source: '# HEAD', + metadata: {} + }, + file: 'a.ipynb', + id: 'MyCellId2', + line: 0, + state: CellState.init + } + ], + isTrusted: isNotebookTrusted + }; + when(storageProvider.get(anything(), anything(), anything(), anything())).thenResolve( + (model as unknown) as INotebookModel + ); - test('Return notebook with 2 cells', async () => { - const model: Partial = { - cells: [ - { - data: { - cell_type: 'code', - execution_count: 10, - hasExecutionOrder: true, + const notebook = await contentProvider.openNotebook(fileUri, {}); + + assert.isOk(notebook); + assert.deepEqual(notebook.languages, [PYTHON_LANGUAGE]); + // ignore metadata we add. + notebook.cells.forEach((cell) => delete cell.metadata.custom); + + assert.equal(notebook.metadata.cellEditable, isNotebookTrusted); + assert.equal(notebook.metadata.cellRunnable, isNotebookTrusted); + assert.equal(notebook.metadata.editable, isNotebookTrusted); + assert.equal(notebook.metadata.runnable, isNotebookTrusted); + + assert.deepEqual(notebook.cells, [ + { + cellKind: (vscodeNotebookEnums as any).CellKind.Code, + language: PYTHON_LANGUAGE, outputs: [], source: 'print(1)', - metadata: {} + metadata: { + editable: isNotebookTrusted, + executionOrder: 10, + hasExecutionOrder: true, + runState: (vscodeNotebookEnums as any).NotebookCellRunState.Success, + runnable: isNotebookTrusted + } }, - file: 'a.ipynb', - id: 'MyCellId1', - line: 0, - state: CellState.init - }, - { - data: { - cell_type: 'markdown', - hasExecutionOrder: false, + { + cellKind: (vscodeNotebookEnums as any).CellKind.Markdown, + language: MARKDOWN_LANGUAGE, + outputs: [], source: '# HEAD', - metadata: {} + metadata: { + editable: isNotebookTrusted, + executionOrder: undefined, + hasExecutionOrder: false, + runState: (vscodeNotebookEnums as any).NotebookCellRunState.Idle, + runnable: false + } + } + ]); + }); + + test('Return notebook with csharp language', async () => { + const model: Partial = { + metadata: { + language_info: { + name: 'csharp' + }, + orig_nbformat: 5 }, - file: 'a.ipynb', - id: 'MyCellId2', - line: 0, - state: CellState.init - } - ] - }; - when(storageProvider.load(anything(), anything(), anything(), anything())).thenResolve( - (model as unknown) as INotebookModel - ); + cells: [ + { + data: { + cell_type: 'code', + execution_count: 10, + hasExecutionOrder: true, + outputs: [], + source: 'Console.WriteLine("1")', + metadata: {} + }, + file: 'a.ipynb', + id: 'MyCellId1', + line: 0, + state: CellState.init + }, + { + data: { + cell_type: 'markdown', + hasExecutionOrder: false, + source: '# HEAD', + metadata: {} + }, + file: 'a.ipynb', + id: 'MyCellId2', + line: 0, + state: CellState.init + } + ], + isTrusted: isNotebookTrusted + }; + when(storageProvider.get(anything(), anything(), anything(), anything())).thenResolve( + (model as unknown) as INotebookModel + ); - const notebook = await contentProvider.openNotebook(fileUri, {}); + const notebook = await contentProvider.openNotebook(fileUri, {}); - assert.isOk(notebook); - assert.deepEqual(notebook.languages, [PYTHON_LANGUAGE]); - // ignore metadata we add. - notebook.cells.forEach((cell) => delete cell.metadata.custom); + assert.isOk(notebook); + assert.deepEqual(notebook.languages, ['csharp']); - assert.deepEqual(notebook.cells, [ - { - cellKind: (vscodeNotebookEnums as any).CellKind.Code, - language: PYTHON_LANGUAGE, - outputs: [], - source: 'print(1)', - metadata: { - editable: true, - executionOrder: 10, - hasExecutionOrder: true, - runState: (vscodeNotebookEnums as any).NotebookCellRunState.Success, - runnable: true - } - }, - { - cellKind: (vscodeNotebookEnums as any).CellKind.Markdown, - language: MARKDOWN_LANGUAGE, - outputs: [], - source: '# HEAD', - metadata: { - editable: true, - executionOrder: undefined, - hasExecutionOrder: false, - runState: (vscodeNotebookEnums as any).NotebookCellRunState.Idle, - runnable: false - } - } - ]); - }); + assert.equal(notebook.metadata.cellEditable, isNotebookTrusted); + assert.equal(notebook.metadata.cellRunnable, isNotebookTrusted); + assert.equal(notebook.metadata.editable, isNotebookTrusted); + assert.equal(notebook.metadata.runnable, isNotebookTrusted); + + // ignore metadata we add. + notebook.cells.forEach((cell: NotebookCellData) => delete cell.metadata.custom); - test('Return notebook with csharp language', async () => { - const model: Partial = { - metadata: { - language_info: { - name: 'csharp' - }, - orig_nbformat: 5 - }, - cells: [ - { - data: { - cell_type: 'code', - execution_count: 10, - hasExecutionOrder: true, + assert.deepEqual(notebook.cells, [ + { + cellKind: (vscodeNotebookEnums as any).CellKind.Code, + language: 'csharp', outputs: [], source: 'Console.WriteLine("1")', - metadata: {} + metadata: { + editable: isNotebookTrusted, + executionOrder: 10, + hasExecutionOrder: true, + runState: (vscodeNotebookEnums as any).NotebookCellRunState.Success, + runnable: isNotebookTrusted + } }, - file: 'a.ipynb', - id: 'MyCellId1', - line: 0, - state: CellState.init - }, - { - data: { - cell_type: 'markdown', - hasExecutionOrder: false, + { + cellKind: (vscodeNotebookEnums as any).CellKind.Markdown, + language: MARKDOWN_LANGUAGE, + outputs: [], source: '# HEAD', - metadata: {} - }, - file: 'a.ipynb', - id: 'MyCellId2', - line: 0, - state: CellState.init - } - ] - }; - when(storageProvider.load(anything(), anything(), anything(), anything())).thenResolve( - (model as unknown) as INotebookModel - ); - - const notebook = await contentProvider.openNotebook(fileUri, {}); - - assert.isOk(notebook); - assert.deepEqual(notebook.languages, ['csharp']); - // ignore metadata we add. - notebook.cells.forEach((cell: NotebookCellData) => delete cell.metadata.custom); - - assert.deepEqual(notebook.cells, [ - { - cellKind: (vscodeNotebookEnums as any).CellKind.Code, - language: 'csharp', - outputs: [], - source: 'Console.WriteLine("1")', - metadata: { - editable: true, - executionOrder: 10, - hasExecutionOrder: true, - runState: (vscodeNotebookEnums as any).NotebookCellRunState.Success, - runnable: true - } - }, - { - cellKind: (vscodeNotebookEnums as any).CellKind.Markdown, - language: MARKDOWN_LANGUAGE, - outputs: [], - source: '# HEAD', - metadata: { - editable: true, - executionOrder: undefined, - hasExecutionOrder: false, - runState: (vscodeNotebookEnums as any).NotebookCellRunState.Idle, - runnable: false - } - } - ]); - }); - test('Verify mime types and order', () => { - // https://github.com/microsoft/vscode-python/issues/11880 + metadata: { + editable: isNotebookTrusted, + executionOrder: undefined, + hasExecutionOrder: false, + runState: (vscodeNotebookEnums as any).NotebookCellRunState.Idle, + runnable: false + } + } + ]); + }); + test('Verify mime types and order', () => { + // https://github.com/microsoft/vscode-python/issues/11880 + }); + }); }); }); diff --git a/src/test/datascience/notebook/helper.ts b/src/test/datascience/notebook/helper.ts index 8f1d66148b59..cb77fdb87749 100644 --- a/src/test/datascience/notebook/helper.ts +++ b/src/test/datascience/notebook/helper.ts @@ -9,16 +9,30 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import * as sinon from 'sinon'; import * as tmp from 'tmp'; -import { commands, Uri } from 'vscode'; -import { NotebookCell } from '../../../../types/vscode-proposed'; +import { instance, mock } from 'ts-mockito'; +import { commands, TextDocument, Uri } from 'vscode'; +import { NotebookCell, NotebookDocument } from '../../../../types/vscode-proposed'; import { CellDisplayOutput } from '../../../../typings/vscode-proposed'; import { IApplicationEnvironment, IVSCodeNotebook } from '../../../client/common/application/types'; import { MARKDOWN_LANGUAGE, PYTHON_LANGUAGE } from '../../../client/common/constants'; import { IDisposable } from '../../../client/common/types'; import { noop, swallowExceptions } from '../../../client/common/utils/misc'; -import { findMappedNotebookCellModel } from '../../../client/datascience/notebook/helpers/cellMappers'; +import { Identifiers } from '../../../client/datascience/constants'; +import { JupyterNotebookView } from '../../../client/datascience/notebook/constants'; +import { + findMappedNotebookCellModel, + mapVSCNotebookCellsToNotebookCellModels +} from '../../../client/datascience/notebook/helpers/cellMappers'; +import { createVSCNotebookCellDataFromCell } from '../../../client/datascience/notebook/helpers/helpers'; import { INotebookContentProvider } from '../../../client/datascience/notebook/types'; -import { ICell, INotebookEditorProvider, INotebookModel, INotebookProvider } from '../../../client/datascience/types'; +import { VSCodeNotebookModel } from '../../../client/datascience/notebookStorage/vscNotebookModel'; +import { + CellState, + ICell, + INotebookEditorProvider, + INotebookModel, + INotebookProvider +} from '../../../client/datascience/types'; import { createEventHandler, waitForCondition } from '../../common'; import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants'; import { closeActiveWindows, initialize } from '../../initialize'; @@ -365,3 +379,61 @@ export async function saveActiveNotebook(disposables: IDisposable[]) { await waitForCondition(async () => savedEvent.all.some((e) => e.kind === 'save'), 5_000, 'Not saved'); } + +export function createNotebookModel(trusted: boolean, uri: Uri, nb?: Partial) { + const nbJson: nbformat.INotebookContent = { + cells: [], + metadata: { + orig_nbformat: 4 + }, + nbformat: 4, + nbformat_minor: 4, + ...(nb || {}) + }; + + const cells = nbJson.cells.map((c, index) => { + return { + id: `NotebookImport#${index}`, + file: Identifiers.EmptyFileName, + line: 0, + state: CellState.finished, + data: c + }; + }); + return new VSCodeNotebookModel(trusted, uri, JSON.parse(JSON.stringify(cells))); +} + +export function createNotebookDocument( + model: INotebookModel, + viewType: string = JupyterNotebookView +): NotebookDocument { + const doc: NotebookDocument = { + cells: [], + fileName: model.file.fsPath, + isDirty: false, + languages: [], + uri: model.file, + viewType, + metadata: { + cellEditable: model.isTrusted, + cellHasExecutionOrder: true, + cellRunnable: model.isTrusted, + editable: model.isTrusted, + runnable: model.isTrusted + } + }; + model.cells.forEach((cell, index) => { + const vscCell = createVSCNotebookCellDataFromCell(model, cell)!; + const vscDocumentCell: NotebookCell = { + ...vscCell, + uri: model.file.with({ fragment: `cell${index}` }), + notebook: doc, + document: instance(mock()) + }; + doc.cells.push(vscDocumentCell); + }); + if (viewType === JupyterNotebookView) { + mapVSCNotebookCellsToNotebookCellModels(doc, model); + } + return doc; +} diff --git a/src/test/datascience/notebook/helpers.unit.test.ts b/src/test/datascience/notebook/helpers.unit.test.ts index 63174224c3b3..bb48bc6c30c2 100644 --- a/src/test/datascience/notebook/helpers.unit.test.ts +++ b/src/test/datascience/notebook/helpers.unit.test.ts @@ -40,7 +40,8 @@ suite('Data Science - NativeNotebook helpers', () => { line: 0, state: CellState.init } - ] + ], + isTrusted: true }; const notebook = notebookModelToVSCNotebookData((model as unknown) as INotebookModel); @@ -95,7 +96,8 @@ suite('Data Science - NativeNotebook helpers', () => { line: 0, state: CellState.init } - ] + ], + isTrusted: true }; const notebook = notebookModelToVSCNotebookData((model as unknown) as INotebookModel); diff --git a/src/test/datascience/notebook/notebookTrustHandler.unit.test.ts b/src/test/datascience/notebook/notebookTrustHandler.unit.test.ts new file mode 100644 index 000000000000..7d3f6e821b4e --- /dev/null +++ b/src/test/datascience/notebook/notebookTrustHandler.unit.test.ts @@ -0,0 +1,142 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { nbformat } from '@jupyterlab/coreutils'; +import { assert } from 'chai'; +import { teardown } from 'mocha'; +import { anything, instance, mock, when } from 'ts-mockito'; +import { EventEmitter, Uri } from 'vscode'; +import { NotebookDocument } from '../../../../types/vscode-proposed'; +import { IExtensionSingleActivationService } from '../../../client/activation/types'; +import { IVSCodeNotebook } from '../../../client/common/application/types'; +import { IFileSystem } from '../../../client/common/platform/types'; +import { IDisposable } from '../../../client/common/types'; +import { NotebookTrustHandler } from '../../../client/datascience/notebook/notebookTrustHandler'; +import { INotebookEditor, INotebookEditorProvider, ITrustService } from '../../../client/datascience/types'; +import { createNotebookDocument, createNotebookModel, disposeAllDisposables } from './helper'; +// tslint:disable-next-line: no-var-requires no-require-imports +const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed'); + +// tslint:disable: no-any +suite('Data Science - NativeNotebook TrustHandler', () => { + let trustHandler: IExtensionSingleActivationService; + let trustService: ITrustService; + let vscNotebook: IVSCodeNotebook; + let editorProvider: INotebookEditorProvider; + let fs: IFileSystem; + let disposables: IDisposable[]; + let onDidTrustNotebook: EventEmitter; + setup(async () => { + disposables = []; + trustService = mock(); + vscNotebook = mock(); + editorProvider = mock(); + fs = mock(); + onDidTrustNotebook = new EventEmitter(); + when(trustService.onDidSetNotebookTrust).thenReturn(onDidTrustNotebook.event); + when(fs.arePathsSame(anything(), anything())).thenCall((a, b) => a === b); // Dirty simple file compare. + trustHandler = new NotebookTrustHandler( + instance(trustService), + instance(vscNotebook), + instance(editorProvider), + instance(fs), + disposables + ); + + await trustHandler.activate(); + }); + teardown(() => disposeAllDisposables(disposables)); + function assertDocumentTrust(document: NotebookDocument, trusted: boolean) { + assert.equal(document.metadata.cellEditable, trusted); + assert.equal(document.metadata.cellRunnable, trusted); + assert.equal(document.metadata.editable, trusted); + assert.equal(document.metadata.runnable, trusted); + + document.cells.forEach((cell) => { + assert.equal(cell.metadata.editable, trusted); + if (cell.cellKind === vscodeNotebookEnums.CellKind.Code) { + assert.equal(cell.metadata.runnable, trusted); + // In our test all code cells have outputs. + if (trusted) { + assert.ok(cell.outputs.length, 'No output in trusted cell'); + } else { + assert.lengthOf(cell.outputs, 0, 'Cannot have output in non-trusted notebook'); + } + } + }); + } + function createModels() { + const nbJson: Partial = { + cells: [ + { + cell_type: 'markdown', + source: [], + metadata: {} + }, + { + cell_type: 'code', + source: [], + metadata: {}, + execution_count: 1, + outputs: [ + { + output_type: 'stream', + name: 'stdout', + text: 'Hello World' + } + ] + } + ] + }; + + return [createNotebookModel(false, Uri.file('a'), nbJson), createNotebookModel(false, Uri.file('b'), nbJson)]; + } + test('When a notebook is trusted, the Notebook document is updated accordingly', async () => { + const [model1, model2] = createModels(); + const [nb1, nb2, nbAnotherExtension] = [ + createNotebookDocument(model1), + createNotebookDocument(model2), + createNotebookDocument(model2, 'AnotherExtensionNotebookEditorForIpynbFile') + ]; + + // Initially un-trusted. + assertDocumentTrust(nb1, false); + assertDocumentTrust(nb2, false); + assertDocumentTrust(nbAnotherExtension, false); + + when(vscNotebook.notebookDocuments).thenReturn([nb1, nb2]); + const editor1 = mock(); + const editor2 = mock(); + when(editor1.file).thenReturn(model1.file); + when(editor2.file).thenReturn(model2.file); + when(editor1.model).thenReturn(model1); + when(editor2.model).thenReturn(model2); + when(editorProvider.editors).thenReturn([instance(editor1), instance(editor2)]); + + // Trigger a change, even though none of the models are still trusted. + onDidTrustNotebook.fire(); + + // Still un-trusted. + assertDocumentTrust(nb1, false); + assertDocumentTrust(nb2, false); + assertDocumentTrust(nbAnotherExtension, false); + + // Trigger a change, after trusting second nb/model. + model2.update({ + source: 'user', + kind: 'updateTrust', + oldDirty: model2.isDirty, + newDirty: model2.isDirty, + isNotebookTrusted: true + }); + + onDidTrustNotebook.fire(); + + // Nb1 is still un-trusted and nb1 is trusted. + assertDocumentTrust(nb1, false); + assertDocumentTrust(nb2, true); + assertDocumentTrust(nbAnotherExtension, false); // This is a document from a different content provider, we should modify this. + }); +}); From e9316da5734bace4623fc4580f4c4f9c96e3258e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 10 Jul 2020 12:06:06 -0700 Subject: [PATCH 09/11] Update warning message when opening nb in two different editors (#12883) --- package.nls.json | 2 +- src/client/common/utils/localize.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.nls.json b/package.nls.json index b8941cffecd4..76596c0c8fdb 100644 --- a/package.nls.json +++ b/package.nls.json @@ -544,7 +544,7 @@ "DataScience.reloadCustomEditor": "Please reload VS Code to use the custom editor API", "DataScience.reloadVSCodeNotebookEditor": "Please reload VS Code to use the Notebook Editor", "DataScience.step": "Run next line (F10)", - "DataScience.usingPreviewNotebookWithOtherNotebookWarning": "Using the Preview Notebook Editor along with the stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks.", + "DataScience.usingPreviewNotebookWithOtherNotebookWarning": "Opening the same file in the Preview Notebook Editor and stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks.", "DataScience.previewNotebookOnlySupportedInVSCInsiders": "The Preview Notebook Editor is supported only in the Insiders version of Visual Studio Code.", "DataScienceNotebookSurveyBanner.bannerMessage": "Can you please take 2 minutes to tell us how the Preview Notebook Editor is working for you?", "DataScience.unknownServerUri": "Server URI cannot be used. Did you uninstall an extension that provided a Jupyter server connection?", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 0f7629b79ed6..43fada222f62 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -1020,7 +1020,7 @@ export namespace DataScience { ); export const usingPreviewNotebookWithOtherNotebookWarning = localize( 'DataScience.usingPreviewNotebookWithOtherNotebookWarning', - 'Using the Preview Notebook Editor along with the stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks.' + 'Opening the same file in the Preview Notebook Editor and stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks.' ); export const launchNotebookTrustPrompt = localize( 'DataScience.launchNotebookTrustPrompt', From 71eae3998a4cfd07f5078ece614121d5fd9eecaf Mon Sep 17 00:00:00 2001 From: Timothy Ruscica <35348871+techwithtim@users.noreply.github.com> Date: Fri, 10 Jul 2020 15:15:50 -0400 Subject: [PATCH 10/11] Made exporting notebooks cancellable (#12863) * made export cancellable * some small fixes * removed monaco editor import * changed deleting method * made export to temp file first * removed unused action * removed unused types --- package.nls.json | 2 +- src/client/common/utils/localize.ts | 2 +- src/client/datascience/export/exportBase.ts | 57 ++++++++++++------- .../datascience/export/exportManager.ts | 14 +++-- src/client/datascience/export/exportToHTML.ts | 17 ++---- src/client/datascience/export/exportToPDF.ts | 32 ++--------- .../datascience/export/exportToPython.ts | 7 ++- src/client/datascience/export/types.ts | 4 +- .../datascience/progress/progressReporter.ts | 6 +- .../datascience/export/exportManager.test.ts | 2 +- .../datascience/export/exportToHTML.test.ts | 6 +- .../datascience/export/exportToPython.test.ts | 6 +- 12 files changed, 77 insertions(+), 78 deletions(-) diff --git a/package.nls.json b/package.nls.json index 76596c0c8fdb..2c7b4c709c71 100644 --- a/package.nls.json +++ b/package.nls.json @@ -23,7 +23,7 @@ "DataScience.checkingIfImportIsSupported": "Checking if import is supported", "DataScience.installingMissingDependencies": "Installing missing dependencies", "DataScience.exportNotebookToPython": "Exporting Notebook to Python", - "DataScience.performingExport": "Performing export", + "DataScience.performingExport": "Performing Export", "DataScience.convertingToPDF": "Converting to PDF", "DataScience.exportHTMLQuickPickLabel": "HTML", "DataScience.exportPDFQuickPickLabel": "PDF", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 43fada222f62..e5ae37dce774 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -331,7 +331,7 @@ export namespace DataScience { 'DataScience.installingMissingDependencies', 'Installing missing dependencies' ); - export const performingExport = localize('DataScience.performingExport', 'Performing export'); + export const performingExport = localize('DataScience.performingExport', 'Performing Export'); export const convertingToPDF = localize('DataScience.convertingToPDF', 'Converting to PDF'); export const exportNotebookToPython = localize( 'DataScience.exportNotebookToPython', diff --git a/src/client/datascience/export/exportBase.ts b/src/client/datascience/export/exportBase.ts index a5ed31449b51..2614433f68e6 100644 --- a/src/client/datascience/export/exportBase.ts +++ b/src/client/datascience/export/exportBase.ts @@ -1,11 +1,12 @@ import { inject, injectable } from 'inversify'; -import { Uri } from 'vscode'; +import * as path from 'path'; +import { CancellationToken, Uri } from 'vscode'; import { IFileSystem } from '../../common/platform/types'; import { IPythonExecutionFactory, IPythonExecutionService } from '../../common/process/types'; import { reportAction } from '../progress/decorator'; import { ReportableAction } from '../progress/types'; import { IJupyterSubCommandExecutionService, INotebookImporter } from '../types'; -import { IExport } from './types'; +import { ExportFormat, IExport } from './types'; @injectable() export class ExportBase implements IExport { @@ -18,37 +19,55 @@ export class ExportBase implements IExport { ) {} // tslint:disable-next-line: no-empty - public async export(_source: Uri, _target: Uri): Promise {} + public async export(_source: Uri, _target: Uri, _token: CancellationToken): Promise {} @reportAction(ReportableAction.PerformingExport) - public async executeCommand(source: Uri, target: Uri, args: string[]): Promise { + public async executeCommand( + source: Uri, + target: Uri, + format: ExportFormat, + token: CancellationToken + ): Promise { + if (token.isCancellationRequested) { + return; + } + const service = await this.getExecutionService(source); if (!service) { return; } - const oldFileExists = await this.fileSystem.fileExists(target.fsPath); - let oldFileTime; - if (oldFileExists) { - oldFileTime = (await this.fileSystem.stat(target.fsPath)).mtime; + if (token.isCancellationRequested) { + return; } + const tempTarget = await this.fileSystem.createTemporaryFile(path.extname(target.fsPath)); + const args = [ + source.fsPath, + '--to', + format, + '--output', + path.basename(tempTarget.filePath), + '--output-dir', + path.dirname(tempTarget.filePath) + ]; const result = await service.execModule('jupyter', ['nbconvert'].concat(args), { throwOnStdErr: false, - encoding: 'utf8' + encoding: 'utf8', + token: token }); - // Need to check if export failed, since throwOnStdErr is not an - // indicator of a failed export. - if (!(await this.fileSystem.fileExists(target.fsPath))) { + if (token.isCancellationRequested) { + tempTarget.dispose(); + return; + } + + try { + await this.fileSystem.copyFile(tempTarget.filePath, target.fsPath); + } catch { throw new Error(result.stderr); - } else if (oldFileExists) { - // If we exported to a file that already exists we need to check that - // this file was actually overridden during export - const newFileTime = (await this.fileSystem.stat(target.fsPath)).mtime; - if (newFileTime === oldFileTime) { - throw new Error(result.stderr); - } + } finally { + tempTarget.dispose(); } } diff --git a/src/client/datascience/export/exportManager.ts b/src/client/datascience/export/exportManager.ts index dcfb47f80183..d58367975728 100644 --- a/src/client/datascience/export/exportManager.ts +++ b/src/client/datascience/export/exportManager.ts @@ -51,27 +51,31 @@ export class ExportManager implements IExportManager { await this.exportUtil.removeSvgs(source); } - const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`); + const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`, true); try { switch (format) { case ExportFormat.python: - await this.exportToPython.export(source, target); + await this.exportToPython.export(source, target, reporter.token); break; case ExportFormat.pdf: - await this.exportToPDF.export(source, target); + await this.exportToPDF.export(source, target, reporter.token); break; case ExportFormat.html: - await this.exportToHTML.export(source, target); + await this.exportToHTML.export(source, target, reporter.token); break; default: break; } } finally { - reporter.dispose(); tempDir.dispose(); + reporter.dispose(); + } + + if (reporter.token.isCancellationRequested) { + return; } return target; diff --git a/src/client/datascience/export/exportToHTML.ts b/src/client/datascience/export/exportToHTML.ts index 005addcc4cfc..d12c92502402 100644 --- a/src/client/datascience/export/exportToHTML.ts +++ b/src/client/datascience/export/exportToHTML.ts @@ -1,20 +1,11 @@ import { injectable } from 'inversify'; -import * as path from 'path'; -import { Uri } from 'vscode'; +import { CancellationToken, Uri } from 'vscode'; import { ExportBase } from './exportBase'; +import { ExportFormat } from './types'; @injectable() export class ExportToHTML extends ExportBase { - public async export(source: Uri, target: Uri): Promise { - const args = [ - source.fsPath, - '--to', - 'html', - '--output', - path.basename(target.fsPath), - '--output-dir', - path.dirname(target.fsPath) - ]; - await this.executeCommand(source, target, args); + public async export(source: Uri, target: Uri, token: CancellationToken): Promise { + await this.executeCommand(source, target, ExportFormat.html, token); } } diff --git a/src/client/datascience/export/exportToPDF.ts b/src/client/datascience/export/exportToPDF.ts index 550e4b1c9e67..0b4b6afba335 100644 --- a/src/client/datascience/export/exportToPDF.ts +++ b/src/client/datascience/export/exportToPDF.ts @@ -1,33 +1,11 @@ -import { inject, injectable } from 'inversify'; -import * as path from 'path'; -import { Uri } from 'vscode'; -import { IFileSystem } from '../../common/platform/types'; -import { IPythonExecutionFactory } from '../../common/process/types'; -import { IJupyterSubCommandExecutionService, INotebookImporter } from '../types'; +import { injectable } from 'inversify'; +import { CancellationToken, Uri } from 'vscode'; import { ExportBase } from './exportBase'; +import { ExportFormat } from './types'; @injectable() export class ExportToPDF extends ExportBase { - constructor( - @inject(IPythonExecutionFactory) protected readonly pythonExecutionFactory: IPythonExecutionFactory, - @inject(IJupyterSubCommandExecutionService) - protected jupyterService: IJupyterSubCommandExecutionService, - @inject(IFileSystem) protected readonly fileSystem: IFileSystem, - @inject(INotebookImporter) protected readonly importer: INotebookImporter - ) { - super(pythonExecutionFactory, jupyterService, fileSystem, importer); - } - - public async export(source: Uri, target: Uri): Promise { - const args = [ - source.fsPath, - '--to', - 'pdf', - '--output', - path.basename(target.fsPath), - '--output-dir', - path.dirname(target.fsPath) - ]; - await this.executeCommand(source, target, args); + public async export(source: Uri, target: Uri, token: CancellationToken): Promise { + await this.executeCommand(source, target, ExportFormat.pdf, token); } } diff --git a/src/client/datascience/export/exportToPython.ts b/src/client/datascience/export/exportToPython.ts index 607026982710..58e010ead4f9 100644 --- a/src/client/datascience/export/exportToPython.ts +++ b/src/client/datascience/export/exportToPython.ts @@ -1,10 +1,13 @@ import { injectable } from 'inversify'; -import { Uri } from 'vscode'; +import { CancellationToken, Uri } from 'vscode'; import { ExportBase } from './exportBase'; @injectable() export class ExportToPython extends ExportBase { - public async export(source: Uri, target: Uri): Promise { + public async export(source: Uri, target: Uri, token: CancellationToken): Promise { + if (token.isCancellationRequested) { + return; + } const contents = await this.importer.importFromFile(source.fsPath); await this.fileSystem.writeFile(target.fsPath, contents); } diff --git a/src/client/datascience/export/types.ts b/src/client/datascience/export/types.ts index cb632e57ebc1..07ff4eee7f61 100644 --- a/src/client/datascience/export/types.ts +++ b/src/client/datascience/export/types.ts @@ -1,4 +1,4 @@ -import { Uri } from 'vscode'; +import { CancellationToken, Uri } from 'vscode'; import { INotebookModel } from '../types'; export enum ExportFormat { @@ -14,7 +14,7 @@ export interface IExportManager { export const IExport = Symbol('IExport'); export interface IExport { - export(source: Uri, target: Uri): Promise; + export(source: Uri, target: Uri, token: CancellationToken): Promise; } export const IExportManagerFilePicker = Symbol('IExportManagerFilePicker'); diff --git a/src/client/datascience/progress/progressReporter.ts b/src/client/datascience/progress/progressReporter.ts index 45098c9a0ff8..87ccadf9f450 100644 --- a/src/client/datascience/progress/progressReporter.ts +++ b/src/client/datascience/progress/progressReporter.ts @@ -33,15 +33,15 @@ export class ProgressReporter implements IProgressReporter { * @returns {(IDisposable & { token: CancellationToken })} * @memberof ProgressReporter */ - public createProgressIndicator(message: string): IDisposable & { token: CancellationToken } { + public createProgressIndicator(message: string, cancellable = false): IDisposable & { token: CancellationToken } { const cancellation = new CancellationTokenSource(); const deferred = createDeferred(); - const options = { location: ProgressLocation.Notification, cancellable: false, title: message }; + const options = { location: ProgressLocation.Notification, cancellable: cancellable, title: message }; this.appShell .withProgress(options, async (progress, cancelToken) => { cancelToken.onCancellationRequested(() => { - if (!cancelToken.isCancellationRequested) { + if (cancelToken.isCancellationRequested) { cancellation.cancel(); } deferred.resolve(); diff --git a/src/test/datascience/export/exportManager.test.ts b/src/test/datascience/export/exportManager.test.ts index 359fa59d6ba5..189b823e973d 100644 --- a/src/test/datascience/export/exportManager.test.ts +++ b/src/test/datascience/export/exportManager.test.ts @@ -40,7 +40,7 @@ suite('Data Science - Export Manager', () => { when(exportUtil.generateTempDir()).thenResolve({ path: 'test', dispose: () => {} }); when(exportUtil.makeFileInDirectory(anything(), anything(), anything())).thenResolve('foo'); when(fileSystem.createTemporaryFile(anything())).thenResolve(instance(tempFile)); - when(exportPdf.export(anything(), anything())).thenResolve(); + when(exportPdf.export(anything(), anything(), anything())).thenResolve(); when(filePicker.getExportFileLocation(anything(), anything())).thenResolve(Uri.file('foo')); // tslint:disable-next-line: no-any when(reporter.createProgressIndicator(anything())).thenReturn(instance(mock()) as any); diff --git a/src/test/datascience/export/exportToHTML.test.ts b/src/test/datascience/export/exportToHTML.test.ts index ec1b0b225543..4412881dea0e 100644 --- a/src/test/datascience/export/exportToHTML.test.ts +++ b/src/test/datascience/export/exportToHTML.test.ts @@ -4,7 +4,7 @@ // tslint:disable: no-var-requires no-require-imports no-invalid-this no-any import { assert } from 'chai'; import * as path from 'path'; -import { Uri } from 'vscode'; +import { CancellationTokenSource, Uri } from 'vscode'; import { IFileSystem } from '../../../client/common/platform/types'; import { ExportFormat, IExport } from '../../../client/datascience/export/types'; import { IExtensionTestApi } from '../../common'; @@ -34,9 +34,11 @@ suite('DataScience - Export HTML', () => { const file = await fileSystem.createTemporaryFile('.html'); const target = Uri.file(file.filePath); await file.dispose(); + const token = new CancellationTokenSource(); await exportToHTML.export( Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'datascience', 'export', 'test.ipynb')), - target + target, + token.token ); assert.equal(await fileSystem.fileExists(target.fsPath), true); diff --git a/src/test/datascience/export/exportToPython.test.ts b/src/test/datascience/export/exportToPython.test.ts index 0bcf74ab912e..d7d130f141e5 100644 --- a/src/test/datascience/export/exportToPython.test.ts +++ b/src/test/datascience/export/exportToPython.test.ts @@ -4,7 +4,7 @@ // tslint:disable: no-var-requires no-require-imports no-invalid-this no-any import { assert } from 'chai'; import * as path from 'path'; -import { Uri } from 'vscode'; +import { CancellationTokenSource, Uri } from 'vscode'; import { IDocumentManager } from '../../../client/common/application/types'; import { IFileSystem } from '../../../client/common/platform/types'; import { ExportFormat, IExport } from '../../../client/datascience/export/types'; @@ -33,9 +33,11 @@ suite('DataScience - Export Python', () => { const fileSystem = api.serviceContainer.get(IFileSystem); const exportToPython = api.serviceContainer.get(IExport, ExportFormat.python); const target = Uri.file((await fileSystem.createTemporaryFile('.py')).filePath); + const token = new CancellationTokenSource(); await exportToPython.export( Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'datascience', 'export', 'test.ipynb')), - target + target, + token.token ); const documentManager = api.serviceContainer.get(IDocumentManager); From 2dd570b62c262bd0497b79ba9b64a003f5715b44 Mon Sep 17 00:00:00 2001 From: Timothy Ruscica <35348871+techwithtim@users.noreply.github.com> Date: Fri, 10 Jul 2020 16:12:51 -0400 Subject: [PATCH 11/11] fixed tests (#12886) --- 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 189b823e973d..e98ef9d932aa 100644 --- a/src/test/datascience/export/exportManager.test.ts +++ b/src/test/datascience/export/exportManager.test.ts @@ -43,7 +43,7 @@ suite('Data Science - Export Manager', () => { when(exportPdf.export(anything(), anything(), anything())).thenResolve(); when(filePicker.getExportFileLocation(anything(), anything())).thenResolve(Uri.file('foo')); // tslint:disable-next-line: no-any - when(reporter.createProgressIndicator(anything())).thenReturn(instance(mock()) as any); + when(reporter.createProgressIndicator(anything(), anything())).thenReturn(instance(mock()) as any); exporter = new ExportManager( instance(exportPdf), instance(exportHtml),