From 8bab9c277dedf337a81ba6b461bda41cd14f6a58 Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Wed, 8 Jul 2020 19:06:56 -0700 Subject: [PATCH 01/17] Update model.isTrusted on trust change (#12820) (#12823) --- src/client/datascience/notebookStorage/baseModel.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/client/datascience/notebookStorage/baseModel.ts b/src/client/datascience/notebookStorage/baseModel.ts index b9a95660b34c..89fbaf7c5bad 100644 --- a/src/client/datascience/notebookStorage/baseModel.ts +++ b/src/client/datascience/notebookStorage/baseModel.ts @@ -78,6 +78,9 @@ export abstract class BaseNotebookModel implements INotebookModel { case 'version': changed = this.updateVersionInfo(change.interpreter, change.kernelSpec); break; + case 'updateTrust': + this._isTrusted = change.isNotebookTrusted; + break; default: break; } From 03ded1cca7a4cae82d5e1fa0a3ce9498f1219c6e Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Thu, 9 Jul 2020 15:04:43 -0700 Subject: [PATCH 02/17] Reduce visual complexity of trust prompt (#12839) (#12847) --- package.nls.json | 3 ++- src/client/common/utils/localize.ts | 6 +++++- .../datascience/interactive-ipynb/digestStorage.ts | 1 - .../datascience/interactive-ipynb/nativeEditor.ts | 10 +++++++++- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/package.nls.json b/package.nls.json index c3638945435e..244a4ffefc9b 100644 --- a/package.nls.json +++ b/package.nls.json @@ -32,9 +32,10 @@ "DataScience.openExportFileNo": "No", "DataScience.failedExportMessage": "Export failed.", "DataScience.exportToPDFDependencyMessage": "If you have not installed xelatex (TeX) you will need to do so before you can export to PDF, for further instructions please look [here](https://nbconvert.readthedocs.io/en/latest/install.html#installing-tex). \r\nTo avoid installing xelatex (TeX) you might want to try exporting to HTML and using your browsers \"Print to PDF\" feature.", - "DataScience.launchNotebookTrustPrompt": "A notebook could execute harmful code when opened. Some cells & outputs have been hidden. Do you trust this notebook? (To trust all notebooks by default, click [here](command:workbench.action.openSettings?%5B%22python.dataScience.alwaysTrustNotebooks%22%5D).) [Learn more.](https://aka.ms/trusted-notebooks)", + "DataScience.launchNotebookTrustPrompt": "A notebook could execute harmful code when opened. Some outputs have been hidden. Do you trust this notebook? [Learn more.](https://aka.ms/trusted-notebooks)", "DataScience.launchNotebookTrustPrompt.yes": "Trust", "DataScience.launchNotebookTrustPrompt.no": "Do not trust", + "DataScience.launchNotebookTrustPrompt.trustAllNotebooks": "Trust all notebooks", "python.command.python.viewLanguageServerOutput.title": "Show Language Server Output", "python.command.python.selectAndRunTestMethod.title": "Run Test Method ...", "python.command.python.selectAndDebugTestMethod.title": "Debug Test Method ...", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 455069417e30..0f7629b79ed6 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -1024,10 +1024,14 @@ export namespace DataScience { ); export const launchNotebookTrustPrompt = localize( 'DataScience.launchNotebookTrustPrompt', - 'A notebook could execute harmful code when opened. Some cells & outputs have been hidden. Do you trust this notebook? (To trust all notebooks by default, click [here](command:workbench.action.openSettings?%5B%22python.dataScience.alwaysTrustNotebooks%22%5D).) [Learn more.](https://aka.ms/trusted-notebooks)' + 'A notebook could execute harmful code when opened. Some outputs have been hidden. Do you trust this notebook? [Learn more.](https://aka.ms/trusted-notebooks)' ); export const trustNotebook = localize('DataScience.launchNotebookTrustPrompt.yes', 'Trust'); export const doNotTrustNotebook = localize('DataScience.launchNotebookTrustPrompt.no', 'Do not trust'); + export const trustAllNotebooks = localize( + 'DataScience.launchNotebookTrustPrompt.trustAllNotebooks', + 'Trust all notebooks' + ); export const previewNotebookOnlySupportedInVSCInsiders = localize( 'DataScience.previewNotebookOnlySupportedInVSCInsiders', 'The Preview Notebook Editor is supported only in the Insiders version of Visual Studio Code.' diff --git a/src/client/datascience/interactive-ipynb/digestStorage.ts b/src/client/datascience/interactive-ipynb/digestStorage.ts index cb6887c81725..cba0fa78c91e 100644 --- a/src/client/datascience/interactive-ipynb/digestStorage.ts +++ b/src/client/datascience/interactive-ipynb/digestStorage.ts @@ -9,7 +9,6 @@ import { IFileSystem } from '../../common/platform/types'; import { IExtensionContext } from '../../common/types'; import { IDigestStorage } from '../types'; -// NB: still need to implement automatic culling of least recently used entries @injectable() export class DigestStorage implements IDigestStorage { public readonly key: Promise; diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index ef299a60b1ed..7e2953cf8b3e 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -6,6 +6,7 @@ import * as path from 'path'; import { CancellationToken, CancellationTokenSource, + commands, Event, EventEmitter, Memento, @@ -611,7 +612,11 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } private async launchNotebookTrustPrompt() { - const prompts = [localize.DataScience.trustNotebook(), localize.DataScience.doNotTrustNotebook()]; + const prompts = [ + localize.DataScience.trustNotebook(), + localize.DataScience.doNotTrustNotebook(), + localize.DataScience.trustAllNotebooks() + ]; const selection = await this.applicationShell.showErrorMessage( localize.DataScience.launchNotebookTrustPrompt(), ...prompts @@ -636,6 +641,9 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } 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'); } } From 0659d6f3007fce14c3865b889bb9745181e47ec3 Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Fri, 10 Jul 2020 12:48:38 -0700 Subject: [PATCH 03/17] Port python 2.7 fix to release (#12877) --- CHANGELOG.md | 2 ++ .../kernel-launcher/kernelLauncherDaemon.ts | 34 ++++++++++++------- .../kernel-launcher/kernelProcess.ts | 8 ++--- .../kernelLauncherDaemon.unit.test.ts | 21 +++++++++--- 4 files changed, 44 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99392e6969c7..1baed2264b61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,8 @@ ([#12588](https://github.com/Microsoft/vscode-python/issues/12588)) 1. Open variable explorer when opening variable explorer during debugging. ([#12773](https://github.com/Microsoft/vscode-python/issues/12773)) +1. Use the given interpreter for launching the non-daemon python + ([#12821](https://github.com/Microsoft/vscode-python/issues/12821)) ### Code Health diff --git a/src/client/datascience/kernel-launcher/kernelLauncherDaemon.ts b/src/client/datascience/kernel-launcher/kernelLauncherDaemon.ts index 1ef80d368189..09955c7ef402 100644 --- a/src/client/datascience/kernel-launcher/kernelLauncherDaemon.ts +++ b/src/client/datascience/kernel-launcher/kernelLauncherDaemon.ts @@ -6,7 +6,7 @@ import { ChildProcess } from 'child_process'; import { inject, injectable } from 'inversify'; import { IDisposable } from 'monaco-editor'; -import { ObservableExecutionResult } from '../../common/process/types'; +import { IPythonExecutionService, ObservableExecutionResult } from '../../common/process/types'; import { Resource } from '../../common/types'; import { noop } from '../../common/utils/misc'; import { PythonInterpreter } from '../../pythonEnvironments/info'; @@ -27,15 +27,10 @@ export class PythonKernelLauncherDaemon implements IDisposable { resource: Resource, kernelSpec: IJupyterKernelSpec, interpreter?: PythonInterpreter - ): Promise<{ observableOutput: ObservableExecutionResult; daemon: IPythonKernelDaemon } | undefined> { + ): Promise<{ observableOutput: ObservableExecutionResult; daemon: IPythonKernelDaemon | undefined }> { const daemon = await this.daemonPool.get(resource, kernelSpec, interpreter); - // The daemon pool can return back a non-IPythonKernelDaemon if daemon service is not supported or for Python 2. - // Use a check for the daemon.start function here before we call it. - if (!daemon.start) { - return undefined; - } - + // Check to see if we have the type of kernelspec that we expect const args = kernelSpec.argv.slice(); const modulePrefixIndex = args.findIndex((item) => item === '-m'); if (modulePrefixIndex === -1) { @@ -49,11 +44,26 @@ export class PythonKernelLauncherDaemon implements IDisposable { const moduleArgs = args.slice(modulePrefixIndex + 2); const env = kernelSpec.env && Object.keys(kernelSpec.env).length > 0 ? kernelSpec.env : undefined; - const observableOutput = await daemon.start(moduleName, moduleArgs, { env }); - if (observableOutput.proc) { - this.processesToDispose.push(observableOutput.proc); + // The daemon pool can return back a non-IPythonKernelDaemon if daemon service is not supported or for Python 2. + // Use a check for the daemon.start function here before we call it. + if (!daemon.start) { + // If we don't have a KernelDaemon here then we have an execution service and should use that to launch + // Typing is a bit funk here, as createDaemon can return an execution service instead of the requested + // daemon class + // tslint:disable-next-line:no-any + const executionService = (daemon as any) as IPythonExecutionService; + + const observableOutput = executionService.execModuleObservable(moduleName, moduleArgs, { env }); + + return { observableOutput, daemon: undefined }; + } else { + // In the case that we do have a kernel deamon, just return it + const observableOutput = await daemon.start(moduleName, moduleArgs, { env }); + if (observableOutput.proc) { + this.processesToDispose.push(observableOutput.proc); + } + return { observableOutput, daemon }; } - return { observableOutput, daemon }; } public dispose() { while (this.processesToDispose.length) { diff --git a/src/client/datascience/kernel-launcher/kernelProcess.ts b/src/client/datascience/kernel-launcher/kernelProcess.ts index 9b88f0e57947..3996d63ee20b 100644 --- a/src/client/datascience/kernel-launcher/kernelProcess.ts +++ b/src/client/datascience/kernel-launcher/kernelProcess.ts @@ -224,13 +224,11 @@ export class KernelProcess implements IKernelProcess { this.interpreter ); - if (kernelDaemonLaunch) { - this.kernelDaemon = kernelDaemonLaunch.daemon; - exeObs = kernelDaemonLaunch.observableOutput; - } + this.kernelDaemon = kernelDaemonLaunch.daemon; + exeObs = kernelDaemonLaunch.observableOutput; } - // If we are not python or if we failed with our daemon launch just use the ProcessExecutionFactory + // If we are not python just use the ProcessExecutionFactory if (!exeObs) { // First part of argument is always the executable. const executable = this._kernelSpec.argv[0]; diff --git a/src/test/datascience/kernel-launcher/kernelLauncherDaemon.unit.test.ts b/src/test/datascience/kernel-launcher/kernelLauncherDaemon.unit.test.ts index c03aa732d229..eaef32c25385 100644 --- a/src/test/datascience/kernel-launcher/kernelLauncherDaemon.unit.test.ts +++ b/src/test/datascience/kernel-launcher/kernelLauncherDaemon.unit.test.ts @@ -3,7 +3,7 @@ import { assert } from 'chai'; import { anything, deepEqual, instance, mock, when } from 'ts-mockito'; -import { ObservableExecutionResult } from '../../../client/common/process/types'; +import { IPythonExecutionService, ObservableExecutionResult } from '../../../client/common/process/types'; import { ReadWrite } from '../../../client/common/types'; import { KernelDaemonPool } from '../../../client/datascience/kernel-launcher/kernelDaemonPool'; import { PythonKernelLauncherDaemon } from '../../../client/datascience/kernel-launcher/kernelLauncherDaemon'; @@ -61,10 +61,23 @@ suite('Data Science - Kernel Launcher Daemon', () => { assert.equal(daemonCreationOutput.daemon, instance(kernelDaemon)); } }); - test('If our daemon pool return does not support start, return undefined', async () => { - when(daemonPool.get(anything(), anything(), anything())).thenResolve({} as any); + test('If our daemon pool returns an execution service, then use it and return the daemon as undefined', async () => { + const executionService = mock(); + when( + executionService.execModuleObservable( + 'ipkernel_launcher', + deepEqual(['-f', 'file.json']), + deepEqual({ env: kernelSpec.env }) + ) + ).thenReturn(instance(observableOutputForDaemon)); + // Make sure that it doesn't have a start function. Normally the proxy will pretend to have a start function, which we are checking for non-existance + when((executionService as any).start).thenReturn(false); + // Else ts-mockit doesn't allow us to return an instance of a mock as a return value from an async function. + (instance(executionService) as any).then = undefined; + when(daemonPool.get(anything(), anything(), anything())).thenResolve(instance(executionService) as any); const daemonCreationOutput = await launcher.launch(undefined, kernelSpec, interpreter); - assert.isUndefined(daemonCreationOutput); + assert.equal(daemonCreationOutput.observableOutput, instance(observableOutputForDaemon)); + assert.isUndefined(daemonCreationOutput.daemon); }); }); From 7bb4c50258369d875e2e21d751135ae4524e24f7 Mon Sep 17 00:00:00 2001 From: David Kutugata Date: Fri, 10 Jul 2020 17:59:16 -0700 Subject: [PATCH 04/17] port color fix on collapse all (#12895) (#12897) * fix a color on collapse all (#12895) * update changelog --- CHANGELOG.md | 2 ++ .../images/CollapseAll/CollapseAll_16x_vscode_dark.svg | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1baed2264b61..dfd396dabe59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,8 @@ ([#12773](https://github.com/Microsoft/vscode-python/issues/12773)) 1. Use the given interpreter for launching the non-daemon python ([#12821](https://github.com/Microsoft/vscode-python/issues/12821)) +1. Correct the color of the 'Collapse All' button in the Interactive Window + ([#12838](https://github.com/microsoft/vscode-python/issues/12838)) ### Code Health diff --git a/src/datascience-ui/react-common/images/CollapseAll/CollapseAll_16x_vscode_dark.svg b/src/datascience-ui/react-common/images/CollapseAll/CollapseAll_16x_vscode_dark.svg index ffddd4cbdb95..11e1f39ad968 100644 --- a/src/datascience-ui/react-common/images/CollapseAll/CollapseAll_16x_vscode_dark.svg +++ b/src/datascience-ui/react-common/images/CollapseAll/CollapseAll_16x_vscode_dark.svg @@ -1,4 +1,4 @@ - - + + From bfa3c82541f0640b16a6668ca11abf58cf050a80 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 13 Jul 2020 08:09:50 -0700 Subject: [PATCH 05/17] Merge fixes into July release (#12889) Co-authored-by: Timothy Ruscica <35348871+techwithtim@users.noreply.github.com> --- build/ci/vscode-python-pr-validation.yaml | 1 + gulpfile.js | 1 + news/1 Enhancements/10496.md | 4 + news/3 Code Health/12844.md | 1 + package.json | 112 ++++++- package.nls.json | 5 +- resources/dark/export_to_python.svg | 4 + resources/dark/trusted.svg | 4 + resources/dark/un-trusted.svg | 4 + resources/light/export_to_python.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 | 8 +- src/client/common/utils/localize.ts | 4 +- .../datascience/commands/exportCommands.ts | 53 +++- src/client/datascience/constants.ts | 2 + .../context/activeEditorContext.ts | 2 +- src/client/datascience/export/exportBase.ts | 62 ++-- .../datascience/export/exportManager.ts | 24 +- .../export/exportManagerDependencyChecker.ts | 8 +- .../export/exportManagerFileOpener.ts | 8 +- .../export/exportManagerFilePicker.ts | 17 +- src/client/datascience/export/exportToHTML.ts | 17 +- src/client/datascience/export/exportToPDF.ts | 32 +- .../datascience/export/exportToPython.ts | 7 +- src/client/datascience/export/exportUtil.ts | 14 +- src/client/datascience/export/types.ts | 8 +- .../interactive-ipynb/nativeEditor.ts | 50 +-- .../interactive-ipynb/nativeEditorProvider.ts | 2 +- .../interactive-ipynb/nativeEditorStorage.ts | 6 +- .../notebookStorageProvider.ts | 10 +- .../interactive-ipynb/trustCommandHandler.ts | 76 +++++ .../interactive-window/interactiveWindow.ts | 6 +- .../datascience/notebook/contentProvider.ts | 10 +- .../datascience/notebook/executionService.ts | 2 +- .../notebook/helpers/cellUpdateHelpers.ts | 51 ++- .../datascience/notebook/helpers/helpers.ts | 54 ++-- .../datascience/notebook/integration.ts | 131 ++++---- .../notebook/notebookEditorProvider.ts | 15 +- .../notebook/notebookTrustHandler.ts | 38 +++ .../datascience/notebook/serviceRegistry.ts | 5 + .../datascience/progress/progressReporter.ts | 6 +- src/client/datascience/serviceRegistry.ts | 2 + src/client/datascience/types.ts | 4 +- src/test/datascience/.vscode/settings.json | 3 +- .../datascience/export/exportManager.test.ts | 6 +- .../exportManagerFileOpener.unit.test.ts | 12 +- .../datascience/export/exportToHTML.test.ts | 6 +- .../datascience/export/exportToPython.test.ts | 6 +- .../datascience/export/exportUtil.test.ts | 2 +- .../trustCommandHandler.unit.test.ts | 127 ++++++++ .../trustService.unit.test.ts | 12 +- .../nativeEditorStorage.unit.test.ts | 20 +- .../nativeEditor.functional.test.tsx | 2 +- .../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 +++++++++ 59 files changed, 1145 insertions(+), 458 deletions(-) create mode 100644 news/1 Enhancements/10496.md create mode 100644 news/3 Code Health/12844.md create mode 100644 resources/dark/export_to_python.svg create mode 100644 resources/dark/trusted.svg create mode 100644 resources/dark/un-trusted.svg create mode 100644 resources/light/export_to_python.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/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/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`. 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 b18fd8ed816d..1a7fdca8262e 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" }, @@ -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", @@ -60,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", @@ -358,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%", @@ -681,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%", @@ -846,6 +878,24 @@ "title": "%python.command.python.datascience.restartkernel.title%", "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%", + "group": "navigation", + "when": "notebookEditorFocused" } ], "explorer/context": [ @@ -1119,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%", @@ -1233,6 +1295,12 @@ "title": "%DataScience.gatherQuality%", "category": "Python", "when": "false" + }, + { + "command": "python.datascience.export", + "title": "%DataScience.notebookExportAs%", + "category": "Python", + "when": "false" } ], "view/title": [ @@ -3085,7 +3153,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/package.nls.json b/package.nls.json index 244a4ffefc9b..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", @@ -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", @@ -543,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/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/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/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/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 d18b5cf737a5..dc310105fdb1 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]: [Uri | INotebookModel, string | undefined]; [DSCommands.SwitchJupyterKernel]: [INotebook | undefined, 'raw' | 'jupyter']; [DSCommands.SelectJupyterCommandLine]: [undefined | Uri]; [DSCommands.SaveNotebookNonCustomEditor]: [Uri]; @@ -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/common/utils/localize.ts b/src/client/common/utils/localize.ts index 0f7629b79ed6..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', @@ -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', diff --git a/src/client/datascience/commands/exportCommands.ts b/src/client/datascience/commands/exportCommands.ts index c82c4d2b58cb..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,13 +29,20 @@ 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)); - 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,9 +58,22 @@ export class ExportCommands implements IDisposable { this.disposables.push(disposable); } - private async export(model: INotebookModel, exportMethod?: ExportFormat) { + 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) { @@ -65,11 +87,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 +100,7 @@ export class ExportCommands implements IDisposable { } } - private getExportQuickPickItems(model: INotebookModel): IExportQuickPickItem[] { + private getExportQuickPickItems(model: INotebookModel, defaultFileName?: string): IExportQuickPickItem[] { return [ { label: DataScience.exportPythonQuickPickLabel(), @@ -97,7 +119,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 +129,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/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/exportBase.ts b/src/client/datascience/export/exportBase.ts index 7bf957085fed..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,43 +19,66 @@ 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 overriden during export - const newFileTime = (await this.fileSystem.stat(target.fsPath)).mtime; - if (newFileTime === oldFileTime) { - throw new Error(result.stderr); - } + } finally { + tempTarget.dispose(); } } 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 41eab797fe66..d58367975728 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; } @@ -38,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) { @@ -47,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/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..c92436c295dd 100644 --- a/src/client/datascience/export/exportManagerFilePicker.ts +++ b/src/client/datascience/export/exportManagerFilePicker.ts @@ -20,19 +20,27 @@ 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; + 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; @@ -40,8 +48,11 @@ export class ExportManagerFilePicker implements IExportManagerFilePicker { return; } - const notebookFileName = path.basename(source.fsPath, path.extname(source.fsPath)); - const dialogUri = Uri.file(path.join(this.getLastFileSaveLocation().fsPath, notebookFileName)); + const targetFileName = defaultFileName + ? defaultFileName + : `${path.basename(source.fsPath, path.extname(source.fsPath))}${extension}`; + + const dialogUri = Uri.file(path.join(this.getLastFileSaveLocation().fsPath, targetFileName)); const options: SaveDialogOptions = { defaultUri: dialogUri, saveLabel: 'Export', 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/exportUtil.ts b/src/client/datascience/export/exportUtil.ts index d7fe774d9923..48bcf44f1535 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; } @@ -63,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(); @@ -73,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/export/types.ts b/src/client/datascience/export/types.ts index a8de0fa1cf95..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 { @@ -9,15 +9,15 @@ 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'); export interface IExport { - export(source: Uri, target: Uri): Promise; + export(source: Uri, target: Uri, token: CancellationToken): Promise; } 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..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); } } @@ -738,7 +716,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-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/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/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/integration.ts b/src/client/datascience/notebook/integration.ts index 2ba2c55ff379..e9a073f1c2bf 100644 --- a/src/client/datascience/notebook/integration.ts +++ b/src/client/datascience/notebook/integration.ts @@ -2,17 +2,16 @@ // 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 { DataScience } from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; @@ -21,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. @@ -33,36 +34,23 @@ 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, + @inject(IExtensionContext) private readonly extensionContext: IExtensionContext ) {} - 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(); + } 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; @@ -108,62 +96,61 @@ 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)); + 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 { + viewType: string; + filenamePattern: string; + }[]; + + // Update the settings. if ( - !content.enableProposedApi || - !Array.isArray(content.contributes.notebookOutputRenderer) || - !Array.isArray(content.contributes.notebookProvider) + enable && + (!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' - } - ]; + editorAssociations.push({ + viewType: 'jupyter-notebook', + filenamePattern: '*.ipynb' + }); + await Promise.all([ + this.extensionContext.globalState.update(EditorAssociationUpdatedKey, true), + settings.update('editorAssociations', editorAssociations, ConfigurationTarget.Global) + ]); + } - await this.fs.writeFile(packageJsonFile, JSON.stringify(content, undefined, 4)); - await this.commandManager - .executeCommand('python.reloadVSCode', DataScience.reloadVSCodeNotebookEditor()) - .then(noop, noop); + // 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); } } diff --git a/src/client/datascience/notebook/notebookEditorProvider.ts b/src/client/datascience/notebook/notebookEditorProvider.ts index a5a88521b6d2..bfe618d46d71 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(); } }) @@ -160,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()); @@ -188,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) { @@ -237,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/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/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/exportManager.test.ts b/src/test/datascience/export/exportManager.test.ts index 862f3e06192c..e98ef9d932aa 100644 --- a/src/test/datascience/export/exportManager.test.ts +++ b/src/test/datascience/export/exportManager.test.ts @@ -33,17 +33,17 @@ 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 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); + when(reporter.createProgressIndicator(anything(), anything())).thenReturn(instance(mock()) as any); exporter = new ExportManager( instance(exportPdf), instance(exportHtml), 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/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); 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/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(); 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 f0bad89dd2d52e2a8e5bf54dcd36527fd10e471c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 13 Jul 2020 11:36:48 -0700 Subject: [PATCH 06/17] Merge more fixes into july release (#12918) Co-authored-by: Joyce Er --- CHANGELOG.md | 9 +++++ news/1 Enhancements/10496.md | 4 -- news/3 Code Health/12554.md | 1 - news/3 Code Health/12844.md | 1 - package.json | 38 +++++-------------- src/client/common/application/commands.ts | 1 - src/client/datascience/constants.ts | 1 - .../interactive-ipynb/digestStorage.ts | 7 +++- .../interactive-ipynb/trustCommandHandler.ts | 2 - .../notebookStorage/notebookModel.ts | 2 +- .../interactive-common/trustMessage.tsx | 4 +- .../native-editor/addCellLine.tsx | 10 +++-- 12 files changed, 35 insertions(+), 45 deletions(-) delete mode 100644 news/1 Enhancements/10496.md delete mode 100644 news/3 Code Health/12554.md delete mode 100644 news/3 Code Health/12844.md diff --git a/CHANGELOG.md b/CHANGELOG.md index dfd396dabe59..048f460119d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,11 @@ ([#12732](https://github.com/Microsoft/vscode-python/issues/12732)) 1. Show a prompt asking user to upgrade Code runner to new version to keep using it when in Deprecate PythonPath experiment. ([#12764](https://github.com/Microsoft/vscode-python/issues/12764)) +1. 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`. + ([#10496](https://github.com/Microsoft/vscode-python/issues/10496)) ### Fixes @@ -119,6 +124,10 @@ ([#12656](https://github.com/Microsoft/vscode-python/issues/12656)) 1. Add more telemetry for "Select Interpreter" command. ([#12722](https://github.com/Microsoft/vscode-python/issues/12722)) +1. Add tests for trusted notebooks. + ([#12554](https://github.com/Microsoft/vscode-python/issues/12554)) +1. Update categories in `package.json`. + ([#12844](https://github.com/Microsoft/vscode-python/issues/12844)) ### Thanks diff --git a/news/1 Enhancements/10496.md b/news/1 Enhancements/10496.md deleted file mode 100644 index e486012325cb..000000000000 --- a/news/1 Enhancements/10496.md +++ /dev/null @@ -1,4 +0,0 @@ -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`. diff --git a/news/3 Code Health/12554.md b/news/3 Code Health/12554.md deleted file mode 100644 index 926f9f320057..000000000000 --- a/news/3 Code Health/12554.md +++ /dev/null @@ -1 +0,0 @@ -Add tests for trusted notebooks. diff --git a/news/3 Code Health/12844.md b/news/3 Code Health/12844.md deleted file mode 100644 index 004246586d91..000000000000 --- a/news/3 Code Health/12844.md +++ /dev/null @@ -1 +0,0 @@ -Update categories in `package.json`. diff --git a/package.json b/package.json index 1a7fdca8262e..36674cbae8dd 100644 --- a/package.json +++ b/package.json @@ -369,7 +369,8 @@ "icon": { "light": "resources/light/export_to_python.svg", "dark": "resources/dark/export_to_python.svg" - } + }, + "enablement": "notebookViewType == jupyter-notebook && python.datascience.isnotebooktrusted" }, { "command": "python.datascience.exportAsPythonScript", @@ -702,16 +703,7 @@ "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" - } + "enablement": "notebookViewType == jupyter-notebook && !python.datascience.isnotebooktrusted && python.datascience.trustfeatureenabled" }, { "command": "python.datascience.notebookeditor.runallcells", @@ -877,25 +869,19 @@ "command": "python.datascience.notebookeditor.restartkernel", "title": "%python.command.python.datascience.restartkernel.title%", "group": "navigation", - "when": "notebookEditorFocused" + "when": "notebookViewType == jupyter-notebook" }, { "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" + "when": "notebookViewType == jupyter-notebook && !python.datascience.isnotebooktrusted && python.datascience.trustfeatureenabled" }, { "command": "python.datascience.export", "title": "%DataScience.notebookExportAs%", "group": "navigation", - "when": "notebookEditorFocused" + "when": "notebookViewType == jupyter-notebook" } ], "explorer/context": [ @@ -940,19 +926,19 @@ "command": "python.datascience.exportAsPythonScript", "title": "%python.command.python.datascience.exportAsPythonScript.title%", "category": "Python", - "when": "python.datascience.isnativeactive && python.datascience.featureenabled" + "when": "python.datascience.isnativeactive && python.datascience.featureenabled && python.datascience.isnotebooktrusted" }, { "command": "python.datascience.exportToHTML", "title": "%python.command.python.datascience.exportToHTML.title%", "category": "Python", - "when": "python.datascience.isnativeactive && python.datascience.featureenabled" + "when": "python.datascience.isnativeactive && python.datascience.featureenabled && python.datascience.isnotebooktrusted" }, { "command": "python.datascience.exportToPDF", "title": "%python.command.python.datascience.exportToPDF.title%", "category": "Python", - "when": "python.datascience.isnativeactive && python.datascience.featureenabled" + "when": "python.datascience.isnativeactive && python.datascience.featureenabled && python.datascience.isnotebooktrusted" }, { "command": "python.switchOffInsidersChannel", @@ -1175,12 +1161,6 @@ "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/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index dc310105fdb1..9e35df989c56 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -184,5 +184,4 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu [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 7d7b732a1e4c..5a371aa92ff5 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -91,7 +91,6 @@ export namespace Commands { 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/interactive-ipynb/digestStorage.ts b/src/client/datascience/interactive-ipynb/digestStorage.ts index cba0fa78c91e..0db3feed5fa5 100644 --- a/src/client/datascience/interactive-ipynb/digestStorage.ts +++ b/src/client/datascience/interactive-ipynb/digestStorage.ts @@ -3,7 +3,7 @@ import { inject, injectable } from 'inversify'; import * as os from 'os'; import * as path from 'path'; import { Uri } from 'vscode'; -import { traceError } from '../../common/logger'; +import { traceError, traceInfo } from '../../common/logger'; import { isFileNotFoundError } from '../../common/platform/errors'; import { IFileSystem } from '../../common/platform/types'; import { IExtensionContext } from '../../common/types'; @@ -13,6 +13,7 @@ import { IDigestStorage } from '../types'; export class DigestStorage implements IDigestStorage { public readonly key: Promise; private digestDir: Promise; + private loggedFileLocations = new Set(); constructor( @inject(IFileSystem) private fs: IFileSystem, @@ -26,6 +27,10 @@ export class DigestStorage implements IDigestStorage { const fileLocation = await this.getFileLocation(uri); // Since the signature is a hex digest, the character 'z' is being used to delimit the start and end of a single digest await this.fs.appendFile(fileLocation, `z${signature}z\n`); + if (!this.loggedFileLocations.has(fileLocation)) { + traceInfo(`Wrote trust for ${uri.toString()} to ${fileLocation}`); + this.loggedFileLocations.add(fileLocation); + } } public async containsDigest(uri: Uri, signature: string) { diff --git a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts index 09ee31f49e62..f7834234477f 100644 --- a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts +++ b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts @@ -13,7 +13,6 @@ 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'; @@ -39,7 +38,6 @@ export class TrustCommandHandler implements IExtensionSingleActivationService { 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) { diff --git a/src/client/datascience/notebookStorage/notebookModel.ts b/src/client/datascience/notebookStorage/notebookModel.ts index 0bbe75705836..3b031870d81d 100644 --- a/src/client/datascience/notebookStorage/notebookModel.ts +++ b/src/client/datascience/notebookStorage/notebookModel.ts @@ -90,7 +90,7 @@ export class NativeEditorNotebookModel extends BaseNotebookModel { // Dirty state comes from undo. At least VS code will track it that way. However // skip file changes as we don't forward those to VS code - if (change.kind !== 'save' && change.kind !== 'saveAs') { + if (change.kind !== 'save' && change.kind !== 'saveAs' && change.kind !== 'updateTrust') { this.changeCount += 1; } diff --git a/src/datascience-ui/interactive-common/trustMessage.tsx b/src/datascience-ui/interactive-common/trustMessage.tsx index a5d55993082b..c97a97e6c6cb 100644 --- a/src/datascience-ui/interactive-common/trustMessage.tsx +++ b/src/datascience-ui/interactive-common/trustMessage.tsx @@ -22,7 +22,9 @@ export class TrustMessage extends React.PureComponent { }; const dynamicStyle: React.CSSProperties = { maxWidth: getMaxWidth(textSize), - color: this.props.isNotebookTrusted ? undefined : 'var(--vscode-editorError-foreground)', + color: this.props.isNotebookTrusted + ? 'var(--vscode-editor-foreground)' + : 'var(--vscode-editorError-foreground)', cursor: this.props.isNotebookTrusted ? undefined : 'pointer' }; diff --git a/src/datascience-ui/native-editor/addCellLine.tsx b/src/datascience-ui/native-editor/addCellLine.tsx index 8f275cc072ce..f5513aafecc9 100644 --- a/src/datascience-ui/native-editor/addCellLine.tsx +++ b/src/datascience-ui/native-editor/addCellLine.tsx @@ -25,19 +25,23 @@ export class AddCellLine extends React.Component { const plus = this.props.includePlus ? ( ) : null; + const disabled = !this.props.isNotebookTrusted; + const innerFilter = disabled ? 'image-button-inner-disabled-filter' : ''; return (
); From 656fb618e7376ec638a0d82519fa709d1617c8be Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Mon, 13 Jul 2020 13:45:32 -0700 Subject: [PATCH 07/17] Port trust fixes (#12929) * Fix regressions in trusted notebooks (#12902) * Handle trustAllNotebooks selection * Fix bug where after trusting, UI didn't update * Recover from ENOENT due to missing parent directory when trusting notebook (#12913) * Disable keydown on native cells in untrusted notebooks (#12914) --- .../interactive-ipynb/digestStorage.ts | 26 ++++++++++++--- .../interactive-ipynb/nativeEditor.ts | 2 +- .../interactive-ipynb/trustCommandHandler.ts | 33 +++++++++++-------- .../native-editor/nativeCell.tsx | 15 +++++++++ 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/src/client/datascience/interactive-ipynb/digestStorage.ts b/src/client/datascience/interactive-ipynb/digestStorage.ts index 0db3feed5fa5..620953bff53d 100644 --- a/src/client/datascience/interactive-ipynb/digestStorage.ts +++ b/src/client/datascience/interactive-ipynb/digestStorage.ts @@ -26,10 +26,20 @@ export class DigestStorage implements IDigestStorage { public async saveDigest(uri: Uri, signature: string) { const fileLocation = await this.getFileLocation(uri); // Since the signature is a hex digest, the character 'z' is being used to delimit the start and end of a single digest - await this.fs.appendFile(fileLocation, `z${signature}z\n`); - if (!this.loggedFileLocations.has(fileLocation)) { - traceInfo(`Wrote trust for ${uri.toString()} to ${fileLocation}`); - this.loggedFileLocations.add(fileLocation); + try { + await this.saveDigestInner(uri, fileLocation, signature); + } catch (err) { + // The nbsignatures dir is only initialized on extension activation. + // If the user deletes it to reset trust, the next attempt to trust + // an untrusted notebook in the same session will fail because the parent + // directory does not exist. + if (isFileNotFoundError(err)) { + // Gracefully recover from such errors by reinitializing directory and retrying + await this.initDir(); + await this.saveDigestInner(uri, fileLocation, signature); + } else { + traceError(err); + } } } @@ -46,6 +56,14 @@ export class DigestStorage implements IDigestStorage { } } + private async saveDigestInner(uri: Uri, fileLocation: string, signature: string) { + await this.fs.appendFile(fileLocation, `z${signature}z\n`); + if (!this.loggedFileLocations.has(fileLocation)) { + traceInfo(`Wrote trust for ${uri.toString()} to ${fileLocation}`); + this.loggedFileLocations.add(fileLocation); + } + } + private async getFileLocation(uri: Uri): Promise { const normalizedName = os.platform() === 'win32' ? uri.fsPath.toLowerCase() : uri.fsPath; const hashedName = createHash('sha256').update(normalizedName).digest('hex'); diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 0ae442b19afb..7eeb259016fc 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -252,7 +252,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { // Sign up for dirty events model.changed(this.modelChanged.bind(this)); - this.previouslyNotTrusted = model.isTrusted; + this.previouslyNotTrusted = !model.isTrusted; } // tslint:disable-next-line: no-any diff --git a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts index f7834234477f..6d44adda8a91 100644 --- a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts +++ b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts @@ -4,7 +4,7 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { Uri } from 'vscode'; +import { commands, Uri } from 'vscode'; import { IExtensionSingleActivationService } from '../../activation/types'; import { IApplicationShell, ICommandManager } from '../../common/application/types'; import { ContextKey } from '../../common/contextKey'; @@ -57,18 +57,25 @@ export class TrustCommandHandler implements IExtensionSingleActivationService { DataScience.doNotTrustNotebook(), DataScience.trustAllNotebooks() ); - if (selection !== DataScience.trustNotebook() || model.isTrusted) { - return; + + switch (selection) { + case DataScience.trustAllNotebooks(): + commands.executeCommand('workbench.action.openSettings', 'python.dataScience.alwaysTrustNotebooks'); + break; + case DataScience.trustNotebook(): + // 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); + break; + default: + break; } - // 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/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index 39f97b3f7090..77fe2ad356db 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -281,6 +281,9 @@ export class NativeCell extends React.Component { // tslint:disable-next-line: cyclomatic-complexity max-func-body-length private keyDownInput = (cellId: string, e: IKeyboardEvent) => { + if (!this.isNotebookTrusted() && !isCellNavigationKeyboardEvent(e)) { + return; + } const isFocusedWhenNotSuggesting = this.isFocused() && e.editorInfo && !e.editorInfo.isSuggesting; switch (e.code) { case 'ArrowUp': @@ -886,3 +889,15 @@ export class NativeCell extends React.Component { export function getConnectedNativeCell() { return connect(null, actionCreators)(NativeCell); } + +function isCellNavigationKeyboardEvent(e: IKeyboardEvent) { + return ( + e.code === 'Enter' || + e.code === 'NumpadEnter' || + e.code === 'ArrowUp' || + e.code === 'k' || + e.code === 'ArrowDown' || + e.code === 'j' || + e.code === 'Escape' + ); +} From ecc5c8e047a92c80f4ec11953357f9993bdd6fd6 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 14 Jul 2020 08:51:50 -0700 Subject: [PATCH 08/17] Hide editor icons when editor is not a notebook (#12934) (#12935) --- package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 36674cbae8dd..879ab436c941 100644 --- a/package.json +++ b/package.json @@ -869,19 +869,19 @@ "command": "python.datascience.notebookeditor.restartkernel", "title": "%python.command.python.datascience.restartkernel.title%", "group": "navigation", - "when": "notebookViewType == jupyter-notebook" + "when": "resourceLangId == jupyter && notebookViewType == 'jupyter-notebook'" }, { "command": "python.datascience.notebookeditor.trust", "title": "%DataScience.trustNotebookCommandTitle%", "group": "navigation@1", - "when": "notebookViewType == jupyter-notebook && !python.datascience.isnotebooktrusted && python.datascience.trustfeatureenabled" + "when": "resourceLangId == jupyter && notebookViewType == 'jupyter-notebook' && !python.datascience.isnotebooktrusted && python.datascience.trustfeatureenabled" }, { "command": "python.datascience.export", "title": "%DataScience.notebookExportAs%", "group": "navigation", - "when": "notebookViewType == jupyter-notebook" + "when": "resourceLangId == jupyter && notebookViewType == 'jupyter-notebook'" } ], "explorer/context": [ From 0ef5ed69d9b65e31efa1f6e276b78fb73429045a Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 14 Jul 2020 13:43:39 -0700 Subject: [PATCH 09/17] Check for hideFromUser before activating current terminal (#12942) (#12956) * Check for hideFromUser before activating current terminal * Add tests * Tweak logic --- news/2 Fixes/11122.md | 1 + src/client/providers/terminalProvider.ts | 8 +++--- src/test/providers/terminal.unit.test.ts | 33 ++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 news/2 Fixes/11122.md diff --git a/news/2 Fixes/11122.md b/news/2 Fixes/11122.md new file mode 100644 index 000000000000..7304c9aac45a --- /dev/null +++ b/news/2 Fixes/11122.md @@ -0,0 +1 @@ +Check for hideFromUser before activating current terminal. diff --git a/src/client/providers/terminalProvider.ts b/src/client/providers/terminalProvider.ts index 628bad8fa022..15f1241d88ce 100644 --- a/src/client/providers/terminalProvider.ts +++ b/src/client/providers/terminalProvider.ts @@ -24,13 +24,15 @@ export class TerminalProvider implements Disposable { const configuration = this.serviceContainer.get(IConfigurationService); const pythonSettings = configuration.getSettings(this.activeResourceService.getActiveResource()); - if (pythonSettings.terminal.activateEnvInCurrentTerminal) { - if (currentTerminal) { + if (currentTerminal && pythonSettings.terminal.activateEnvInCurrentTerminal) { + const hideFromUser = + 'hideFromUser' in currentTerminal.creationOptions && currentTerminal.creationOptions.hideFromUser; + if (!hideFromUser) { const terminalActivator = this.serviceContainer.get(ITerminalActivator); await terminalActivator.activateEnvironmentInTerminal(currentTerminal, { preserveFocus: true }); } sendTelemetryEvent(EventName.ACTIVATE_ENV_IN_CURRENT_TERMINAL, undefined, { - isTerminalVisible: currentTerminal ? true : false + isTerminalVisible: !hideFromUser }); } } diff --git a/src/test/providers/terminal.unit.test.ts b/src/test/providers/terminal.unit.test.ts index 3e852b63dc2d..aa1fc837cfd1 100644 --- a/src/test/providers/terminal.unit.test.ts +++ b/src/test/providers/terminal.unit.test.ts @@ -122,6 +122,11 @@ suite('Terminal Provider', () => { .returns(() => activeResourceService.object); terminal = TypeMoq.Mock.ofType(); + terminal + .setup((c) => c.creationOptions) + .returns(() => { + return { hideFromUser: false }; + }); }); test('If terminal.activateCurrentTerminal setting is set, provided terminal should be activated', async () => { @@ -168,6 +173,34 @@ suite('Terminal Provider', () => { configService.verifyAll(); }); + test('If terminal.activateCurrentTerminal setting is set, but hideFromUser is true, provided terminal should not be activated', async () => { + terminalSettings.setup((t) => t.activateEnvInCurrentTerminal).returns(() => true); + configService + .setup((c) => c.getSettings(resource)) + .returns(() => pythonSettings.object) + .verifiable(TypeMoq.Times.once()); + activeResourceService + .setup((a) => a.getActiveResource()) + .returns(() => resource) + .verifiable(TypeMoq.Times.once()); + + terminal + .setup((c) => c.creationOptions) + .returns(() => { + return { hideFromUser: true }; + }); + + terminalProvider = new TerminalProvider(serviceContainer.object); + await terminalProvider.initialize(terminal.object); + + terminalActivator.verify( + (a) => a.activateEnvironmentInTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny()), + TypeMoq.Times.never() + ); + activeResourceService.verifyAll(); + configService.verifyAll(); + }); + test('terminal.activateCurrentTerminal setting is set but provided terminal is undefined', async () => { terminalSettings.setup((t) => t.activateEnvInCurrentTerminal).returns(() => true); configService From cc412b149e7e1bb755aa67e3b0973b7fc2d13c73 Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Tue, 14 Jul 2020 17:11:52 -0700 Subject: [PATCH 10/17] Port final trust fixes for release (#12965) * Only allow Enter / NumpadEnter w/o ctrl/shift/alt (#12939) * Send telemetry for notebook trust prompt selections (#12964) * Fixes for persisting trust (#12950) --- src/client/datascience/constants.ts | 6 +++- .../interactive-ipynb/nativeEditorStorage.ts | 35 +++++++++++-------- .../interactive-ipynb/trustCommandHandler.ts | 9 ++++- .../datascience/notebookStorage/baseModel.ts | 8 ++--- src/client/telemetry/index.ts | 10 ++++++ .../native-editor/nativeCell.tsx | 3 +- 6 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index 5a371aa92ff5..03b38aaec1bf 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -368,7 +368,11 @@ export enum Telemetry { RunByLineStart = 'DATASCIENCE.RUN_BY_LINE', RunByLineStep = 'DATASCIENCE.RUN_BY_LINE_STEP', RunByLineStop = 'DATASCIENCE.RUN_BY_LINE_STOP', - RunByLineVariableHover = 'DATASCIENCE.RUN_BY_LINE_VARIABLE_HOVER' + RunByLineVariableHover = 'DATASCIENCE.RUN_BY_LINE_VARIABLE_HOVER', + TrustAllNotebooks = 'DATASCIENCE.TRUST_ALL_NOTEBOOKS', + TrustNotebook = 'DATASCIENCE.TRUST_NOTEBOOK', + DoNotTrustNotebook = 'DATASCIENCE.DO_NOT_TRUST_NOTEBOOK', + NotebookTrustPromptShown = 'DATASCIENCE.NOTEBOOK_TRUST_PROMPT_SHOWN' } export enum NativeKeyboardCommandTelemetry { diff --git a/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts b/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts index 10f2e29eb8ed..d9600bf8f483 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts @@ -282,10 +282,10 @@ export class NativeEditorStorage implements INotebookStorage { const dirtyContents = skipDirtyContents ? undefined : await this.getStoredContents(file, backupId); if (dirtyContents) { // This means we're dirty. Indicate dirty and load from this content - return this.loadContents(file, dirtyContents, true, contents, forVSCodeNotebook); + return this.loadContents(file, dirtyContents, true, forVSCodeNotebook); } else { // Load without setting dirty - return this.loadContents(file, contents, undefined, undefined, forVSCodeNotebook); + return this.loadContents(file, contents, undefined, forVSCodeNotebook); } } catch (ex) { // May not exist at this time. Should always have a single cell though @@ -308,7 +308,6 @@ export class NativeEditorStorage implements INotebookStorage { file: Uri, contents: string | undefined, isInitiallyDirty = false, - trueContents?: string, forVSCodeNotebook?: boolean ) { // tslint:disable-next-line: no-any @@ -348,18 +347,9 @@ export class NativeEditorStorage implements INotebookStorage { } const pythonNumber = json ? await this.extractPythonMainVersion(json) : 3; - /* As an optimization, we don't call trustNotebook for hot exit, since our hot exit backup code gets called by VS - Code whenever the notebook model changes. This means it's called very often, perhaps even as often as autosave. - Instead, when loading a file that is dirty, we check if the actual file contents on disk are trusted. If so, we treat - the dirty contents as trusted as well. */ - const contentsToCheck = isInitiallyDirty && trueContents !== undefined ? trueContents : contents; - const isTrusted = - contents === undefined || isUntitledFile(file) - ? true // If no contents or untitled, this is a newly created file, so it should be trusted - : await this.trustService.isNotebookTrusted(file, contentsToCheck!); - return this.factory.createModel( + const model = this.factory.createModel( { - trusted: isTrusted, + trusted: true, file, cells: remapped, notebookJson: json, @@ -369,6 +359,23 @@ export class NativeEditorStorage implements INotebookStorage { }, forVSCodeNotebook ); + + // If no contents or untitled, this is a newly created file + // If dirty, that means it's been edited before in our extension + if (contents !== undefined && !isUntitledFile(file) && !isInitiallyDirty) { + const isNotebookTrusted = await this.trustService.isNotebookTrusted(file, model.getContent()); + if (isNotebookTrusted !== model.isTrusted) { + model.update({ + source: 'user', + kind: 'updateTrust', + oldDirty: model.isDirty, + newDirty: model.isDirty, + isNotebookTrusted + }); + } + } + + return model; } private getStaticStorageKey(file: Uri): string { diff --git a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts index 6d44adda8a91..b9a002749cbb 100644 --- a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts +++ b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts @@ -13,7 +13,8 @@ import '../../common/extensions'; import { IDisposableRegistry, IExperimentService } from '../../common/types'; import { swallowExceptions } from '../../common/utils/decorators'; import { DataScience } from '../../common/utils/localize'; -import { Commands } from '../constants'; +import { sendTelemetryEvent } from '../../telemetry'; +import { Commands, Telemetry } from '../constants'; import { INotebookStorageProvider } from '../interactive-ipynb/notebookStorageProvider'; import { INotebookEditorProvider, ITrustService } from '../types'; @@ -57,10 +58,12 @@ export class TrustCommandHandler implements IExtensionSingleActivationService { DataScience.doNotTrustNotebook(), DataScience.trustAllNotebooks() ); + sendTelemetryEvent(Telemetry.NotebookTrustPromptShown); switch (selection) { case DataScience.trustAllNotebooks(): commands.executeCommand('workbench.action.openSettings', 'python.dataScience.alwaysTrustNotebooks'); + sendTelemetryEvent(Telemetry.TrustAllNotebooks); break; case DataScience.trustNotebook(): // Update model trust @@ -73,6 +76,10 @@ export class TrustCommandHandler implements IExtensionSingleActivationService { }); const contents = model.getContent(); await this.trustService.trustNotebook(model.file, contents); + sendTelemetryEvent(Telemetry.TrustNotebook); + break; + case DataScience.doNotTrustNotebook(): + sendTelemetryEvent(Telemetry.DoNotTrustNotebook); break; default: break; diff --git a/src/client/datascience/notebookStorage/baseModel.ts b/src/client/datascience/notebookStorage/baseModel.ts index 89fbaf7c5bad..452534c162e5 100644 --- a/src/client/datascience/notebookStorage/baseModel.ts +++ b/src/client/datascience/notebookStorage/baseModel.ts @@ -196,12 +196,8 @@ export abstract class BaseNotebookModel implements INotebookModel { this.ensureNotebookJson(); // Reuse our original json except for the cells. - const json = { - cells: this.cells.map((c) => pruneCell(c.data)), - metadata: this.notebookJson.metadata, - nbformat: this.notebookJson.nbformat, - nbformat_minor: this.notebookJson.nbformat_minor - }; + const json = { ...this.notebookJson }; + json.cells = this.cells.map((c) => pruneCell(c.data)); return JSON.stringify(json, null, this.indentAmount); } } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 0f76ead0ec48..ff26bcf385b2 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -2192,10 +2192,20 @@ export interface IEventNamePropertyMapping { [Telemetry.StartPageOpenFileBrowser]: never | undefined; [Telemetry.StartPageOpenFolder]: never | undefined; [Telemetry.StartPageOpenWorkspace]: never | undefined; + + // Run by line events [Telemetry.RunByLineStart]: never | undefined; [Telemetry.RunByLineStep]: never | undefined; [Telemetry.RunByLineStop]: never | undefined; [Telemetry.RunByLineVariableHover]: never | undefined; + + // Trusted notebooks events + [Telemetry.NotebookTrustPromptShown]: never | undefined; + [Telemetry.TrustNotebook]: never | undefined; + [Telemetry.TrustAllNotebooks]: never | undefined; + [Telemetry.DoNotTrustNotebook]: never | undefined; + + // Native notebooks events [VSCodeNativeTelemetry.AddCell]: never | undefined; [VSCodeNativeTelemetry.DeleteCell]: never | undefined; [VSCodeNativeTelemetry.MoveCell]: never | undefined; diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index 77fe2ad356db..19ddede77e01 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -892,8 +892,7 @@ export function getConnectedNativeCell() { function isCellNavigationKeyboardEvent(e: IKeyboardEvent) { return ( - e.code === 'Enter' || - e.code === 'NumpadEnter' || + ((e.code === 'Enter' || e.code === 'NumpadEnter') && !e.shiftKey && !e.ctrlKey && !e.altKey) || e.code === 'ArrowUp' || e.code === 'k' || e.code === 'ArrowDown' || From 40d5dcd6e2c4bc3fee14a3c46c2e1856ccfd2ebb Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 15 Jul 2020 10:51:08 -0700 Subject: [PATCH 11/17] Display survey for native notebooks on/after 1st August (#12961) (#12975) Co-authored-by: Joyce Er Co-authored-by: Joyce Er --- src/client/datascience/notebook/survey.ts | 5 +++++ src/test/datascience/notebook/survey.unit.test.ts | 1 + 2 files changed, 6 insertions(+) diff --git a/src/client/datascience/notebook/survey.ts b/src/client/datascience/notebook/survey.ts index 8a631c97ab4e..aabac9de00b6 100644 --- a/src/client/datascience/notebook/survey.ts +++ b/src/client/datascience/notebook/survey.ts @@ -81,6 +81,11 @@ export class NotebookSurveyBanner { if (!this.enabled || this.disabledInCurrentSession) { return false; } + const currentDate = new Date(); + if (currentDate.getMonth() < 7 && currentDate.getFullYear() <= 2020) { + return false; + } + const data = this.persistentState.createGlobalPersistentState(storageKey, {}); const totalActionsInPreviousSessions = diff --git a/src/test/datascience/notebook/survey.unit.test.ts b/src/test/datascience/notebook/survey.unit.test.ts index 5a7889548286..634882127978 100644 --- a/src/test/datascience/notebook/survey.unit.test.ts +++ b/src/test/datascience/notebook/survey.unit.test.ts @@ -57,6 +57,7 @@ suite('Data Science - NativeNotebook Survey', () => { shell = mock(); browser = mock(); clock = fakeTimers.install(); + clock.setSystemTime(new Date(2020, 7, 1)); // Survey will work only after 1st August. }); async function loadAndActivateExtension() { const surveyBanner = new NotebookSurveyBanner(instance(shell), instance(stateFactory), instance(browser)); From 70c32982694d5a49e634b87402d6fb9ba41cfa9b Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 15 Jul 2020 14:28:32 -0700 Subject: [PATCH 12/17] Contains cherry picks, version updates, change log updates (#12983) * Update version and change log * Improve detection when LS is fully loaded for IntelliCode (#12853) * Fix path * Actually fix settings * Add news * Add test * Format * Suppress 'jediEnabled' removal * Drop survey first launch threshold * Wait for client ready * Handle async dispose * Fix the date Co-authored-by: Mikhail Arkhipov --- CHANGELOG.md | 12 ++++++------ news/2 Fixes/11122.md | 1 - package-lock.json | 2 +- package.json | 2 +- .../activation/languageServer/languageServerProxy.ts | 6 +++++- src/client/activation/node/languageServerProxy.ts | 5 ++++- 6 files changed, 17 insertions(+), 11 deletions(-) delete mode 100644 news/2 Fixes/11122.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 048f460119d3..edf614a601c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 2020.7.0-rc (8 July 2020) +## 2020.7.0 (15 July 2020) ### Enhancements @@ -9,7 +9,8 @@ ([#9679](https://github.com/Microsoft/vscode-python/issues/9679)) 1. Added "argsExpansion" to debugpy launch.json schema. ([#11678](https://github.com/Microsoft/vscode-python/issues/11678)) -1. The extension will now automatically load if a `pyproject.toml` file is present in the workspace root directory. (@BrandonLWhite) +1. The extension will now automatically load if a `pyproject.toml` file is present in the workspace root directory. + (thanks [Brandon White](https://github.com/BrandonLWhite)) ([#12056](https://github.com/Microsoft/vscode-python/issues/12056)) 1. Add ability to check and update whether a notebook is trusted. ([#12146](https://github.com/Microsoft/vscode-python/issues/12146)) @@ -29,14 +30,11 @@ ([#12611](https://github.com/Microsoft/vscode-python/issues/12611)) 1. Include the JUPYTER_PATH environment variable when searching the disk for kernels. ([#12694](https://github.com/Microsoft/vscode-python/issues/12694)) -1. Added exporting to python, HTML and PDF from the interative window. +1. Added exporting to python, HTML and PDF from the interactive window. ([#12732](https://github.com/Microsoft/vscode-python/issues/12732)) 1. Show a prompt asking user to upgrade Code runner to new version to keep using it when in Deprecate PythonPath experiment. ([#12764](https://github.com/Microsoft/vscode-python/issues/12764)) 1. 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`. ([#10496](https://github.com/Microsoft/vscode-python/issues/10496)) ### Fixes @@ -45,6 +43,8 @@ ([#10579](https://github.com/Microsoft/vscode-python/issues/10579)) 1. Provided a method for external partners to participate in jupyter server URI picking/authentication. ([#10993](https://github.com/Microsoft/vscode-python/issues/10993)) +1. Check for hideFromUser before activating current terminal. + ([#11122](https://github.com/Microsoft/vscode-python/issues/11122)) 1. In Markdown cells, turn HTML links to markdown links so that nteract renders them. ([#11254](https://github.com/Microsoft/vscode-python/issues/11254)) 1. Prevent incorrect ipywidget display (double plots) due to synchronization issues. diff --git a/news/2 Fixes/11122.md b/news/2 Fixes/11122.md deleted file mode 100644 index 7304c9aac45a..000000000000 --- a/news/2 Fixes/11122.md +++ /dev/null @@ -1 +0,0 @@ -Check for hideFromUser before activating current terminal. diff --git a/package-lock.json b/package-lock.json index 00dca13130b6..5bcc48fd301d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "python", - "version": "2020.7.0-rc", + "version": "2020.7.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 879ab436c941..51814c4edd55 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "python", "displayName": "Python", "description": "Linting, Debugging (multi-threaded, remote), Intellisense, Jupyter Notebooks, code formatting, refactoring, unit tests, snippets, and more.", - "version": "2020.7.0-rc", + "version": "2020.7.0", "featureFlags": { "usingNewInterpreterStorage": true }, diff --git a/src/client/activation/languageServer/languageServerProxy.ts b/src/client/activation/languageServer/languageServerProxy.ts index c0ae13b03557..1d7c0cf8d33a 100644 --- a/src/client/activation/languageServer/languageServerProxy.ts +++ b/src/client/activation/languageServer/languageServerProxy.ts @@ -102,9 +102,13 @@ export class DotNetLanguageServerProxy implements ILanguageServerProxy { } @captureTelemetry(EventName.PYTHON_LANGUAGE_SERVER_READY, undefined, true) protected async serverReady(): Promise { - while (this.languageClient && !this.languageClient!.initializeResult) { + // languageClient can be disposed in awaits. + while (this.languageClient && !this.languageClient.initializeResult) { await sleep(100); } + if (this.languageClient) { + await this.languageClient.onReady(); + } this.startupCompleted.resolve(); } @swallowExceptions('Activating Unit Tests Manager for Language Server') diff --git a/src/client/activation/node/languageServerProxy.ts b/src/client/activation/node/languageServerProxy.ts index 0e02adb97eae..4d5bfc9429cd 100644 --- a/src/client/activation/node/languageServerProxy.ts +++ b/src/client/activation/node/languageServerProxy.ts @@ -146,9 +146,12 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy { NodeLanguageServerProxy.versionTelemetryProps ) protected async serverReady(): Promise { - while (this.languageClient && !this.languageClient!.initializeResult) { + while (this.languageClient && !this.languageClient.initializeResult) { await sleep(100); } + if (this.languageClient) { + await this.languageClient.onReady(); + } this.startupCompleted.resolve(); } From a6e46fcaa5fc77997c22661d8dfe5afff949919c Mon Sep 17 00:00:00 2001 From: David Kutugata Date: Wed, 15 Jul 2020 15:19:14 -0700 Subject: [PATCH 13/17] hide the gather button while a cell is executing (#12984) --- src/datascience-ui/history-react/interactiveCell.tsx | 1 + src/datascience-ui/native-editor/nativeCell.tsx | 1 + 2 files changed, 2 insertions(+) diff --git a/src/datascience-ui/history-react/interactiveCell.tsx b/src/datascience-ui/history-react/interactiveCell.tsx index b9e38b64fa1f..558258ebb099 100644 --- a/src/datascience-ui/history-react/interactiveCell.tsx +++ b/src/datascience-ui/history-react/interactiveCell.tsx @@ -189,6 +189,7 @@ export class InteractiveCell extends React.Component { onClick={gatherCode} hidden={ this.props.cellVM.cell.state === CellState.error || + this.props.cellVM.cell.state === CellState.executing || this.props.cellVM.cell.data.cell_type === 'markdown' || !this.props.settings.gatherIsInstalled } diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index 19ddede77e01..cda8de9462b0 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -598,6 +598,7 @@ export class NativeCell extends React.Component { this.props.cellVM.cell.data.execution_count === null || this.props.cellVM.hasBeenRun === null || this.props.cellVM.hasBeenRun === false || + this.props.cellVM.cell.state === CellState.executing || this.isError() || this.isMarkdownCell() || !this.props.gatherIsInstalled; From 227a9c57661651abf5fbd4814641749c67f5eb7c Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 16 Jul 2020 11:07:52 -0700 Subject: [PATCH 14/17] Update date (#13002) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index edf614a601c8..f06f56bae852 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 2020.7.0 (15 July 2020) +## 2020.7.0 (16 July 2020) ### Enhancements From 82fd6f6a3570ec15ca7ef073949a66bb7963b892 Mon Sep 17 00:00:00 2001 From: David Kutugata Date: Fri, 17 Jul 2020 19:59:36 -0700 Subject: [PATCH 15/17] remove release notes from the start page (#13032) --- StartPageReleaseNotes.md | 3 --- package.nls.json | 2 +- src/client/common/startPage/startPage.ts | 12 ++--------- src/client/common/startPage/types.ts | 11 +++++------ src/client/common/utils/localize.ts | 2 +- src/datascience-ui/startPage/startPage.css | 6 ++++++ src/datascience-ui/startPage/startPage.tsx | 23 ++++++---------------- 7 files changed, 21 insertions(+), 38 deletions(-) delete mode 100644 StartPageReleaseNotes.md diff --git a/StartPageReleaseNotes.md b/StartPageReleaseNotes.md deleted file mode 100644 index 5fb96d33988a..000000000000 --- a/StartPageReleaseNotes.md +++ /dev/null @@ -1,3 +0,0 @@ -Added a start page for the extension. Launched in experimental mode such that it opens to new users or when there is a new release. It can be disabled with the setting 'Python: Show Start Page' and it can be opened at any time with the command 'Python: Open Start Page'. -Removed `python.jediEnabled` setting in favor of `python.languageServer`. Instead of `"python.jediEnabled": true` please use `"python.languageServer": "Jedi"`. -Made the variable explorer in the Notebook editor resizable. diff --git a/package.nls.json b/package.nls.json index 2c7b4c709c71..7a1428eeab9b 100644 --- a/package.nls.json +++ b/package.nls.json @@ -528,7 +528,7 @@ "StartPage.pythonFileDescription": "- Create a
new file
with a .py extension", "StartPage.openInteractiveWindow": "Use the Interactive Window to develop Python Scripts", "StartPage.interactiveWindowDesc": "- You can create cells on a Python file by typing \"#%%\"
- Use \"
Shift + Enter
\" to run a cell, the output will be shown in the interactive window", - "StartPage.releaseNotes": "Take a look at our Release Notes to learn more about the latest features", + "StartPage.releaseNotes": "Take a look at our Release Notes to learn more about the latest features.", "StartPage.tutorialAndDoc": "Explore more features in our Tutorials or check Documentation for tips and troubleshooting.", "StartPage.dontShowAgain": "Don't show this page again", "StartPage.helloWorld": "Hello world", diff --git a/src/client/common/startPage/startPage.ts b/src/client/common/startPage/startPage.ts index 6ebb7fe62165..8a6ad587ed4b 100644 --- a/src/client/common/startPage/startPage.ts +++ b/src/client/common/startPage/startPage.ts @@ -120,11 +120,9 @@ export class StartPage extends WebViewHost implements IStartP case StartPageMessages.Started: this.webviewDidLoad = true; break; - case StartPageMessages.RequestReleaseNotesAndShowAgainSetting: + case StartPageMessages.RequestShowAgainSetting: const settings = this.configuration.getSettings(); - const filteredNotes = await this.handleReleaseNotesRequest(); - await this.postMessage(StartPageMessages.SendReleaseNotes, { - notes: filteredNotes, + await this.postMessage(StartPageMessages.SendSetting, { showAgainSetting: settings.showStartPage }); break; @@ -245,12 +243,6 @@ export class StartPage extends WebViewHost implements IStartP return shouldShowStartPage; } - // This gets the release notes from StartPageReleaseNotes.md - private async handleReleaseNotesRequest(): Promise { - const releaseNotes = await this.file.readFile(path.join(EXTENSION_ROOT_DIR, 'StartPageReleaseNotes.md')); - return releaseNotes.splitLines(); - } - private async activateBackground(): Promise { const enabled = await this.expService.inExperiment(EnableStartPage.experiment); const settings = this.configuration.getSettings(); diff --git a/src/client/common/startPage/types.ts b/src/client/common/startPage/types.ts index 0e724d7bff2d..a147085fe426 100644 --- a/src/client/common/startPage/types.ts +++ b/src/client/common/startPage/types.ts @@ -8,16 +8,15 @@ export interface IStartPage { extensionVersionChanged(): Promise; } -export interface IReleaseNotesPackage { - notes: string[]; +export interface ISettingPackage { showAgainSetting: boolean; } export namespace StartPageMessages { export const Started = SharedMessages.Started; export const UpdateSettings = SharedMessages.UpdateSettings; - export const RequestReleaseNotesAndShowAgainSetting = 'RequestReleaseNotesAndShowAgainSetting'; - export const SendReleaseNotes = 'SendReleaseNotes'; + export const RequestShowAgainSetting = 'RequestShowAgainSetting'; + export const SendSetting = 'SendSetting'; export const OpenBlankNotebook = 'OpenBlankNotebook'; export const OpenBlankPythonFile = 'OpenBlankPythonFile'; export const OpenInteractiveWindow = 'OpenInteractiveWindow'; @@ -30,8 +29,8 @@ export namespace StartPageMessages { } export class IStartPageMapping { - public [StartPageMessages.RequestReleaseNotesAndShowAgainSetting]: IReleaseNotesPackage; - public [StartPageMessages.SendReleaseNotes]: IReleaseNotesPackage; + public [StartPageMessages.RequestShowAgainSetting]: ISettingPackage; + public [StartPageMessages.SendSetting]: ISettingPackage; public [StartPageMessages.Started]: never | undefined; public [StartPageMessages.UpdateSettings]: boolean; public [StartPageMessages.OpenBlankNotebook]: never | undefined; diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index e5ae37dce774..27642897ae80 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -1062,7 +1062,7 @@ export namespace StartPage { export const releaseNotes = localize( 'StartPage.releaseNotes', - 'Take a look at our Release Notes to learn more about the latest features' + 'Take a look at our Release Notes to learn more about the latest features.' ); export const tutorialAndDoc = localize( 'StartPage.tutorialAndDoc', diff --git a/src/datascience-ui/startPage/startPage.css b/src/datascience-ui/startPage/startPage.css index de64fbe8538a..035610dcebf8 100644 --- a/src/datascience-ui/startPage/startPage.css +++ b/src/datascience-ui/startPage/startPage.css @@ -53,6 +53,12 @@ white-space: nowrap; } +.releaseNotesRow { + display: block; + min-height: 50px; + white-space: nowrap; +} + .link { display: inline; color: var(--vscode-debugIcon-continueForeground); diff --git a/src/datascience-ui/startPage/startPage.tsx b/src/datascience-ui/startPage/startPage.tsx index 84f1ad076062..3794c22919fe 100644 --- a/src/datascience-ui/startPage/startPage.tsx +++ b/src/datascience-ui/startPage/startPage.tsx @@ -4,7 +4,7 @@ import * as React from 'react'; import '../../client/common/extensions'; -import { IReleaseNotesPackage, IStartPageMapping, StartPageMessages } from '../../client/common/startPage/types'; +import { ISettingPackage, IStartPageMapping, StartPageMessages } from '../../client/common/startPage/types'; import { Image, ImageName } from '../react-common/image'; import { getLocString } from '../react-common/locReactSide'; import { IMessageHandler, PostOffice } from '../react-common/postOffice'; @@ -19,8 +19,7 @@ export interface IStartPageProps { // Front end of the Python extension start page. // In general it consists of its render method and methods that send and receive messages. export class StartPage extends React.Component implements IMessageHandler { - private releaseNotes: IReleaseNotesPackage = { - notes: [], + private releaseNotes: ISettingPackage = { showAgainSetting: false }; private postOffice: PostOffice = new PostOffice(); @@ -30,7 +29,7 @@ export class StartPage extends React.Component implements IMess } public componentDidMount() { - this.postOffice.sendMessage(StartPageMessages.RequestReleaseNotesAndShowAgainSetting); + this.postOffice.sendMessage(StartPageMessages.RequestShowAgainSetting); } // tslint:disable: no-any @@ -129,9 +128,8 @@ export class StartPage extends React.Component implements IMess {this.renderInteractiveWindowDescription()} -
+
{this.renderReleaseNotesLink()} - {this.renderReleaseNotes()} {this.renderTutorialAndDoc()}
@@ -151,8 +149,7 @@ export class StartPage extends React.Component implements IMess // tslint:disable-next-line: no-any public handleMessage = (msg: string, payload?: any) => { - if (msg === StartPageMessages.SendReleaseNotes) { - this.releaseNotes.notes = payload.notes; + if (msg === StartPageMessages.SendSetting) { this.releaseNotes.showAgainSetting = payload.showAgainSetting; this.setState({}); } @@ -240,21 +237,13 @@ export class StartPage extends React.Component implements IMess dangerouslySetInnerHTML={{ __html: getLocString( 'StartPage.releaseNotes', - 'Take a look at our Release Notes to learn more about the latest features' + 'Take a look at our Release Notes to learn more about the latest features.' ).format('https://aka.ms/AA8dxtb') }} /> ); } - private renderReleaseNotes(): JSX.Element { - const notes: JSX.Element[] = []; - this.releaseNotes.notes.forEach((rel, index) => { - notes.push(
  • {rel}
    • ); - }); - return
      {notes}
    ; - } - private renderTutorialAndDoc(): JSX.Element { // tslint:disable: react-no-dangerous-html return ( From d272d916d59229616f00f124ee3072230c2ae01f Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 22 Jul 2020 14:24:45 -0700 Subject: [PATCH 16/17] Cherry pick, version change and change log update (#13079) * Ensure languageServer value is valid, send event during activate (#13064) * Update change log and version * Activate banner prompt for Pylance (#12817) * Fix path * Actually fix settings * Add news * Add test * Format * Suppress 'jediEnabled' removal * Drop survey first launch threshold * Remove LS experiments * Frequency + tests * Fix test * Update message to match spec * Open workspace for extension rather than changing setting * Fix localization string * Show banners asynchronously * Add experiments * Formatting * Typo * Put back verifyAll * Remove obsolete experiments, add Pylance * Suppress experiment if Pylance is installed * PR feedback Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com> * Update change log as per comments Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Co-authored-by: Mikhail Arkhipov --- CHANGELOG.md | 7 + experiments.json | 12 - package-lock.json | 2 +- package.json | 6 +- package.nls.json | 3 + src/client/activation/activationService.ts | 19 +- .../activation/languageServer/activator.ts | 18 +- .../node/languageServerFolderService.ts | 5 +- src/client/activation/none/activator.ts | 17 +- src/client/activation/serviceRegistry.ts | 4 +- .../application/applicationEnvironment.ts | 3 + src/client/common/application/types.ts | 4 + src/client/common/configSettings.ts | 7 +- src/client/common/constants.ts | 1 + src/client/common/experiments/groups.ts | 8 +- src/client/common/types.ts | 2 +- src/client/common/utils/localize.ts | 7 + .../proposeLanguageServerBanner.ts | 119 +++----- src/client/providers/jediProxy.ts | 18 +- .../activation/activationService.unit.test.ts | 281 ++--------------- .../languageServer/activator.unit.test.ts | 8 +- .../languageServerFolderService.unit.test.ts | 8 +- .../activation/serviceRegistry.unit.test.ts | 4 +- .../configSettings.unit.test.ts | 25 ++ .../datascience/dataScienceIocContainer.ts | 8 + ...roposeNewLanguageServerBanner.unit.test.ts | 289 ++++++++++++------ 26 files changed, 379 insertions(+), 506 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f06f56bae852..471dec73356f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 2020.7.1 (22 July 2020) + +1. Fix language server setting when provided an invalid value, send config event more consistently. + ([#13064](https://github.com/Microsoft/vscode-python/pull/13064)) +1. Add banner for pylance, and remove old LS experiment. + ([#12817](https://github.com/microsoft/vscode-python/pull/12817)) + ## 2020.7.0 (16 July 2020) ### Enhancements diff --git a/experiments.json b/experiments.json index 992b023270b0..02d843e969bc 100644 --- a/experiments.json +++ b/experiments.json @@ -65,18 +65,6 @@ "min": 0, "max": 0 }, - { - "name": "LS - enabled", - "salt": "LS", - "min": 0, - "max": 4 - }, - { - "name": "LS - control", - "salt": "LS", - "min": 20, - "max": 24 - }, { "name": "UseTerminalToGetActivatedEnvVars - experiment", "salt": "UseTerminalToGetActivatedEnvVars", diff --git a/package-lock.json b/package-lock.json index 5bcc48fd301d..fcd4b734e2e0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "python", - "version": "2020.7.0", + "version": "2020.7.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 51814c4edd55..2197d57833a6 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "python", "displayName": "Python", "description": "Linting, Debugging (multi-threaded, remote), Intellisense, Jupyter Notebooks, code formatting, refactoring, unit tests, snippets, and more.", - "version": "2020.7.0", + "version": "2020.7.1", "featureFlags": { "usingNewInterpreterStorage": true }, @@ -1822,7 +1822,6 @@ "default": [], "items": { "enum": [ - "LS - enabled", "AlwaysDisplayTestExplorer - experiment", "ShowExtensionSurveyPrompt - enabled", "Reload - experiment", @@ -1836,6 +1835,7 @@ "DeprecatePythonPath - experiment", "RunByLine - experiment", "EnableTrustedNotebooks", + "tryPylance", "All" ] }, @@ -1847,7 +1847,6 @@ "default": [], "items": { "enum": [ - "LS - enabled", "AlwaysDisplayTestExplorer - experiment", "ShowExtensionSurveyPrompt - enabled", "Reload - experiment", @@ -1861,6 +1860,7 @@ "DeprecatePythonPath - experiment", "RunByLine - experiment", "EnableTrustedNotebooks", + "tryPylance", "All" ] }, diff --git a/package.nls.json b/package.nls.json index 7a1428eeab9b..8103c7b7e653 100644 --- a/package.nls.json +++ b/package.nls.json @@ -107,6 +107,9 @@ "python.snippet.launch.django.label": "Python: Django", "python.snippet.launch.flask.label": "Python: Flask", "python.snippet.launch.pyramid.label": "Python: Pyramid Application", + "LanguageService.proposePylanceMessage": "Try out a new faster, feature-rich language server for Python by Microsoft, Pylance! Install the extension now.", + "LanguageService.tryItNow": "Try it now", + "LanguageService.remindMeLater": "Remind me later", "LanguageService.bannerLabelYes": "Yes, take survey now", "LanguageService.bannerLabelNo": "No, thanks", "LanguageService.lsFailedToStart": "We encountered an issue starting the Language Server. Reverting to the alternative, Jedi. Check the Python output panel for details.", diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index f825a5235dc5..872c38c81b27 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -9,12 +9,10 @@ import { LSNotSupportedDiagnosticServiceId } from '../application/diagnostics/ch import { IDiagnosticsService } from '../application/diagnostics/types'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types'; import { STANDARD_OUTPUT_CHANNEL } from '../common/constants'; -import { LSControl, LSEnabled } from '../common/experiments/groups'; import { traceError } from '../common/logger'; import { IConfigurationService, IDisposableRegistry, - IExperimentsManager, IOutputChannel, IPersistentStateFactory, IPythonSettings, @@ -58,8 +56,7 @@ export class LanguageServerExtensionActivationService constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(IPersistentStateFactory) private stateFactory: IPersistentStateFactory, - @inject(IExperimentsManager) private readonly abExperiments: IExperimentsManager + @inject(IPersistentStateFactory) private stateFactory: IPersistentStateFactory ) { this.workspaceService = this.serviceContainer.get(IWorkspaceService); this.interpreterService = this.serviceContainer.get(IInterpreterService); @@ -175,20 +172,8 @@ export class LanguageServerExtensionActivationService * @returns `true` if user is using jedi, `false` if user is using language server */ public useJedi(): boolean { - // Check if `languageServer` setting is missing (default configuration). - if (this.isJediUsingDefaultConfiguration(this.resource)) { - // If user is assigned to an experiment (i.e. use LS), return false. - if (this.abExperiments.inExperiment(LSEnabled)) { - return false; - } - // Send telemetry if user is in control group - this.abExperiments.sendTelemetryIfInExperiment(LSControl); - return true; // Do use Jedi as it is default. - } - // Configuration is non-default, so `languageServer` should be present. const configurationService = this.serviceContainer.get(IConfigurationService); const lstType = configurationService.getSettings(this.resource).languageServer; - this.sendTelemetryForChosenLanguageServer(lstType).ignoreErrors(); return lstType === LanguageServerType.Jedi; } @@ -253,6 +238,8 @@ export class LanguageServerExtensionActivationService break; } + this.sendTelemetryForChosenLanguageServer(serverType).ignoreErrors(); + await this.logStartup(serverType); let server = this.serviceContainer.get(ILanguageServerActivator, serverType); try { diff --git a/src/client/activation/languageServer/activator.ts b/src/client/activation/languageServer/activator.ts index 42194f36adbe..cc4bb9be9678 100644 --- a/src/client/activation/languageServer/activator.ts +++ b/src/client/activation/languageServer/activator.ts @@ -1,13 +1,15 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { inject, injectable } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import * as path from 'path'; import { IWorkspaceService } from '../../common/application/types'; +import { isTestExecution } from '../../common/constants'; import { traceDecorators } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; -import { IConfigurationService, Resource } from '../../common/types'; +import { BANNER_NAME_PROPOSE_LS, IConfigurationService, IPythonExtensionBanner, Resource } from '../../common/types'; +import { PythonInterpreter } from '../../pythonEnvironments/info'; import { LanguageServerActivatorBase } from '../common/activatorBase'; import { ILanguageServerDownloader, ILanguageServerFolderService, ILanguageServerManager } from '../types'; @@ -27,11 +29,21 @@ export class DotNetLanguageServerActivator extends LanguageServerActivatorBase { @inject(IFileSystem) fs: IFileSystem, @inject(ILanguageServerDownloader) lsDownloader: ILanguageServerDownloader, @inject(ILanguageServerFolderService) languageServerFolderService: ILanguageServerFolderService, - @inject(IConfigurationService) configurationService: IConfigurationService + @inject(IConfigurationService) configurationService: IConfigurationService, + @inject(IPythonExtensionBanner) + @named(BANNER_NAME_PROPOSE_LS) + private proposePylancePopup: IPythonExtensionBanner ) { super(manager, workspace, fs, lsDownloader, languageServerFolderService, configurationService); } + public async start(resource: Resource, interpreter?: PythonInterpreter): Promise { + if (!isTestExecution()) { + this.proposePylancePopup.showBanner().ignoreErrors(); + } + return super.start(resource, interpreter); + } + @traceDecorators.error('Failed to ensure language server is available') public async ensureLanguageServerIsAvailable(resource: Resource): Promise { const languageServerFolderPath = await this.ensureLanguageServerFileIsAvailable(resource, 'mscorlib.dll'); diff --git a/src/client/activation/node/languageServerFolderService.ts b/src/client/activation/node/languageServerFolderService.ts index 44922a052970..0ccb89b28fc3 100644 --- a/src/client/activation/node/languageServerFolderService.ts +++ b/src/client/activation/node/languageServerFolderService.ts @@ -8,14 +8,13 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { SemVer } from 'semver'; import { IWorkspaceService } from '../../common/application/types'; +import { PYLANCE_EXTENSION_ID } from '../../common/constants'; import { NugetPackage } from '../../common/nuget/types'; import { IConfigurationService, IExtensions, Resource } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { LanguageServerFolderService } from '../common/languageServerFolderService'; import { FolderVersionPair, ILanguageServerFolderService, NodeLanguageServerFolder } from '../types'; -export const PylanceExtensionName = 'ms-python.vscode-pylance'; - class FallbackNodeLanguageServerFolderService extends LanguageServerFolderService { constructor(serviceContainer: IServiceContainer) { super(serviceContainer, NodeLanguageServerFolder); @@ -101,7 +100,7 @@ export class NodeLanguageServerFolderService implements ILanguageServerFolderSer return undefined; } - const extension = this.extensions.getExtension(PylanceExtensionName); + const extension = this.extensions.getExtension(PYLANCE_EXTENSION_ID); if (!extension) { return undefined; } diff --git a/src/client/activation/none/activator.ts b/src/client/activation/none/activator.ts index 8d808ae87ce0..5266e8ab39c1 100644 --- a/src/client/activation/none/activator.ts +++ b/src/client/activation/none/activator.ts @@ -1,6 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { injectable } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import { CancellationToken, CodeLens, @@ -20,7 +20,8 @@ import { TextDocument, WorkspaceEdit } from 'vscode'; -import { Resource } from '../../common/types'; +import { isTestExecution } from '../../common/constants'; +import { BANNER_NAME_PROPOSE_LS, IPythonExtensionBanner, Resource } from '../../common/types'; import { PythonInterpreter } from '../../pythonEnvironments/info'; import { ILanguageServerActivator } from '../types'; @@ -33,8 +34,16 @@ import { ILanguageServerActivator } from '../types'; */ @injectable() export class NoLanguageServerExtensionActivator implements ILanguageServerActivator { - // tslint:disable-next-line: no-empty - public async start(_resource: Resource, _interpreter?: PythonInterpreter): Promise {} + constructor( + @inject(IPythonExtensionBanner) + @named(BANNER_NAME_PROPOSE_LS) + private proposePylancePopup: IPythonExtensionBanner + ) {} + public async start(_resource: Resource, _interpreter?: PythonInterpreter): Promise { + if (!isTestExecution()) { + this.proposePylancePopup.showBanner().ignoreErrors(); + } + } // tslint:disable-next-line: no-empty public dispose(): void {} // tslint:disable-next-line: no-empty diff --git a/src/client/activation/serviceRegistry.ts b/src/client/activation/serviceRegistry.ts index f5bec7c65f35..0f1fa5497f67 100644 --- a/src/client/activation/serviceRegistry.ts +++ b/src/client/activation/serviceRegistry.ts @@ -12,7 +12,7 @@ import { import { DataScienceSurveyBanner } from '../datascience/dataScienceSurveyBanner'; import { InteractiveShiftEnterBanner } from '../datascience/shiftEnterBanner'; import { IServiceManager } from '../ioc/types'; -import { ProposeLanguageServerBanner } from '../languageServices/proposeLanguageServerBanner'; +import { ProposePylanceBanner } from '../languageServices/proposeLanguageServerBanner'; import { AATesting } from './aaTesting'; import { ExtensionActivationManager } from './activationManager'; import { LanguageServerExtensionActivationService } from './activationService'; @@ -86,7 +86,7 @@ export function registerTypes(serviceManager: IServiceManager, languageServerTyp serviceManager.addSingleton( IPythonExtensionBanner, - ProposeLanguageServerBanner, + ProposePylanceBanner, BANNER_NAME_PROPOSE_LS ); serviceManager.addSingleton( diff --git a/src/client/common/application/applicationEnvironment.ts b/src/client/common/application/applicationEnvironment.ts index a16082e0b848..3d01d85f4b35 100644 --- a/src/client/common/application/applicationEnvironment.ts +++ b/src/client/common/application/applicationEnvironment.ts @@ -88,4 +88,7 @@ export class ApplicationEnvironment implements IApplicationEnvironment { const version = parse(this.packageJson.version); return !version || version.prerelease.length > 0 ? 'insiders' : 'stable'; } + public get uriScheme(): string { + return vscode.env.uriScheme; + } } diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index f1d7f38900c6..2459ca1aded7 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -1024,6 +1024,10 @@ export interface IApplicationEnvironment { * The version of the editor. */ readonly vscodeVersion: string; + /** + * The custom uri scheme the editor registers to in the operating system. + */ + readonly uriScheme: string; } export const IWebPanelMessageListener = Symbol('IWebPanelMessageListener'); diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 57c865fccfa2..d0066f58b378 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -231,11 +231,12 @@ export class PythonSettings implements IPythonSettings { pythonSettings.get('autoUpdateLanguageServer', true) )!; - let ls = pythonSettings.get('languageServer'); - if (!ls) { + let ls = pythonSettings.get('languageServer') ?? LanguageServerType.Jedi; + ls = systemVariables.resolveAny(ls); + if (!Object.values(LanguageServerType).includes(ls)) { ls = LanguageServerType.Jedi; } - this.languageServer = systemVariables.resolveAny(ls)!; + this.languageServer = ls; // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion this.jediPath = systemVariables.resolveAny(pythonSettings.get('jediPath'))!; diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index fb4ccf0e9ef9..6096285fa170 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -15,6 +15,7 @@ export const PYTHON_ALLFILES = [{ language: PYTHON_LANGUAGE }]; export const PVSC_EXTENSION_ID = 'ms-python.python'; export const CODE_RUNNER_EXTENSION_ID = 'formulahendry.code-runner'; +export const PYLANCE_EXTENSION_ID = 'ms-python.vscode-pylance'; export const AppinsightsKey = 'AIF-d9b70cd4-b9f9-4d70-929b-a071c400b217'; export namespace Commands { diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index 3757a03d54a0..6daf346c23f8 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -1,6 +1,3 @@ -export const LSControl = 'LS - control'; -export const LSEnabled = 'LS - enabled'; - // Experiment to check whether to always display the test explorer. export enum AlwaysDisplayTestExplorerGroups { control = 'AlwaysDisplayTestExplorer - control', @@ -99,3 +96,8 @@ export enum RemoveKernelToolbarInInteractiveWindow { export enum EnableTrustedNotebooks { experiment = 'EnableTrustedNotebooks' } + +// Experiment to offer switch to Pylance language server +export enum TryPylance { + experiment = 'tryPylance' +} diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 99557821e85c..d7fab4ddc24e 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -525,7 +525,7 @@ export interface IPythonExtensionBanner { readonly enabled: boolean; showBanner(): Promise; } -export const BANNER_NAME_PROPOSE_LS: string = 'ProposeLS'; +export const BANNER_NAME_PROPOSE_LS: string = 'ProposePylance'; export const BANNER_NAME_DS_SURVEY: string = 'DSSurveyBanner'; export const BANNER_NAME_INTERACTIVE_SHIFTENTER: string = 'InteractiveShiftEnterBanner'; diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 27642897ae80..324fc2e2f914 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -101,6 +101,13 @@ export namespace AttachProcess { } export namespace LanguageService { + export const proposePylanceMessage = localize( + 'LanguageService.proposePylanceMessage', + 'Try out a new faster, feature-rich language server for Python by Microsoft, Pylance! Install the extension now.' + ); + export const tryItNow = localize('LanguageService.tryItNow', 'Try it now'); + export const remindMeLater = localize('LanguageService.remindMeLater', 'Remind me later'); + export const bannerLabelYes = localize('LanguageService.bannerLabelYes', 'Yes, take survey now'); export const bannerLabelNo = localize('LanguageService.bannerLabelNo', 'No, thanks'); export const lsFailedToStart = localize( diff --git a/src/client/languageServices/proposeLanguageServerBanner.ts b/src/client/languageServices/proposeLanguageServerBanner.ts index 55645f90f220..dfc792c48e47 100644 --- a/src/client/languageServices/proposeLanguageServerBanner.ts +++ b/src/client/languageServices/proposeLanguageServerBanner.ts @@ -4,22 +4,27 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { ConfigurationTarget } from 'vscode'; import { LanguageServerType } from '../activation/types'; -import { IApplicationShell } from '../common/application/types'; +import { IApplicationEnvironment, IApplicationShell } from '../common/application/types'; +import { PYLANCE_EXTENSION_ID } from '../common/constants'; +import { TryPylance } from '../common/experiments/groups'; import '../common/extensions'; -import { IConfigurationService, IPersistentStateFactory, IPythonExtensionBanner } from '../common/types'; -import { getRandomBetween } from '../common/utils/random'; +import { + IConfigurationService, + IExperimentService, + IExtensions, + IPersistentStateFactory, + IPythonExtensionBanner +} from '../common/types'; +import { LanguageService } from '../common/utils/localize'; -// persistent state names, exported to make use of in testing -export enum ProposeLSStateKeys { - ShowBanner = 'ProposeLSBanner' +export function getPylanceExtensionUri(appEnv: IApplicationEnvironment): string { + return `${appEnv.uriScheme}:extension/${PYLANCE_EXTENSION_ID}`; } -enum ProposeLSLabelIndex { - Yes, - No, - Later +// persistent state names, exported to make use of in testing +export enum ProposeLSStateKeys { + ShowBanner = 'TryPylanceBanner' } /* @@ -30,46 +35,24 @@ and will show as soon as it is instructed to do so, if a random sample function enables the popup for this user. */ @injectable() -export class ProposeLanguageServerBanner implements IPythonExtensionBanner { - private initialized?: boolean; +export class ProposePylanceBanner implements IPythonExtensionBanner { private disabledInCurrentSession: boolean = false; - private sampleSizePerHundred: number; - private bannerMessage: string = - 'Try out Preview of our new Python Language Server to get richer and faster IntelliSense completions, and syntax errors as you type.'; - private bannerLabels: string[] = ['Try it now', 'No thanks', 'Remind me Later']; constructor( @inject(IApplicationShell) private appShell: IApplicationShell, + @inject(IApplicationEnvironment) private appEnv: IApplicationEnvironment, @inject(IPersistentStateFactory) private persistentState: IPersistentStateFactory, @inject(IConfigurationService) private configuration: IConfigurationService, - sampleSizePerOneHundredUsers: number = 10 - ) { - this.sampleSizePerHundred = sampleSizePerOneHundredUsers; - this.initialize(); - } - - public initialize() { - if (this.initialized) { - return; - } - this.initialized = true; - - // Don't even bother adding handlers if banner has been turned off. - if (!this.enabled) { - return; - } + @inject(IExperimentService) private experiments: IExperimentService, + @inject(IExtensions) readonly extensions: IExtensions + ) {} - // we only want 10% of folks that use Jedi to see this survey. - const randomSample: number = getRandomBetween(0, 100); - if (randomSample >= this.sampleSizePerHundred) { - this.disable().ignoreErrors(); - return; - } - } public get enabled(): boolean { - // Banner is deactivated. - return false; - // return this.persistentState.createGlobalPersistentState(ProposeLSStateKeys.ShowBanner, true).value; + const lsType = this.configuration.getSettings().languageServer ?? LanguageServerType.Jedi; + if (lsType === LanguageServerType.Jedi || lsType === LanguageServerType.Node) { + return false; + } + return this.persistentState.createGlobalPersistentState(ProposeLSStateKeys.ShowBanner, true).value; } public async showBanner(): Promise { @@ -82,30 +65,31 @@ export class ProposeLanguageServerBanner implements IPythonExtensionBanner { return; } - const response = await this.appShell.showInformationMessage(this.bannerMessage, ...this.bannerLabels); - switch (response) { - case this.bannerLabels[ProposeLSLabelIndex.Yes]: { - await this.enableLanguageServer(); - await this.disable(); - break; - } - case this.bannerLabels[ProposeLSLabelIndex.No]: { - await this.disable(); - break; - } - case this.bannerLabels[ProposeLSLabelIndex.Later]: { - this.disabledInCurrentSession = true; - break; - } - default: { - // Disable for the current session. - this.disabledInCurrentSession = true; - } + const response = await this.appShell.showInformationMessage( + LanguageService.proposePylanceMessage(), + LanguageService.tryItNow(), + LanguageService.bannerLabelNo(), + LanguageService.remindMeLater() + ); + + if (response === LanguageService.tryItNow()) { + this.appShell.openUrl(getPylanceExtensionUri(this.appEnv)); + await this.disable(); + } else if (response === LanguageService.bannerLabelNo()) { + await this.disable(); + } else { + this.disabledInCurrentSession = true; } } public async shouldShowBanner(): Promise { - return Promise.resolve(this.enabled && !this.disabledInCurrentSession); + // Do not prompt if Pylance is already installed. + if (this.extensions.getExtension(PYLANCE_EXTENSION_ID)) { + return false; + } + // Only prompt for users in experiment. + const inExperiment = await this.experiments.inExperiment(TryPylance.experiment); + return inExperiment && this.enabled && !this.disabledInCurrentSession; } public async disable(): Promise { @@ -113,13 +97,4 @@ export class ProposeLanguageServerBanner implements IPythonExtensionBanner { .createGlobalPersistentState(ProposeLSStateKeys.ShowBanner, false) .updateValue(false); } - - public async enableLanguageServer(): Promise { - await this.configuration.updateSetting( - 'languageServer', - LanguageServerType.Microsoft, - undefined, - ConfigurationTarget.Global - ); - } } diff --git a/src/client/providers/jediProxy.ts b/src/client/providers/jediProxy.ts index c72a700f03f7..add675bb2e76 100644 --- a/src/client/providers/jediProxy.ts +++ b/src/client/providers/jediProxy.ts @@ -7,19 +7,13 @@ import * as path from 'path'; // @ts-ignore import * as pidusage from 'pidusage'; import { CancellationToken, CancellationTokenSource, CompletionItemKind, Disposable, SymbolKind, Uri } from 'vscode'; -import { isTestExecution } from '../common/constants'; import '../common/extensions'; import { IS_WINDOWS } from '../common/platform/constants'; import { IFileSystem } from '../common/platform/types'; import * as internalPython from '../common/process/internal/python'; import * as internalScripts from '../common/process/internal/scripts'; import { IPythonExecutionFactory } from '../common/process/types'; -import { - BANNER_NAME_PROPOSE_LS, - IConfigurationService, - IPythonExtensionBanner, - IPythonSettings -} from '../common/types'; +import { IConfigurationService, IPythonSettings } from '../common/types'; import { createDeferred, Deferred } from '../common/utils/async'; import { swallowExceptions } from '../common/utils/decorators'; import { StopWatch } from '../common/utils/stopWatch'; @@ -157,7 +151,6 @@ export class JediProxy implements Disposable { private pidUsageFailures = { timer: new StopWatch(), counter: 0 }; private lastCmdIdProcessed?: number; private lastCmdIdProcessedForPidUsage?: number; - private proposeNewLanguageServerPopup: IPythonExtensionBanner; private readonly disposables: Disposable[] = []; private timer?: NodeJS.Timer | number; @@ -174,12 +167,6 @@ export class JediProxy implements Disposable { this.startLanguageServer() .then(() => this.initialized.resolve()) .ignoreErrors(); - - this.proposeNewLanguageServerPopup = serviceContainer.get( - IPythonExtensionBanner, - BANNER_NAME_PROPOSE_LS - ); - this.checkJediMemoryFootprint().ignoreErrors(); } @@ -332,9 +319,6 @@ export class JediProxy implements Disposable { private async startLanguageServer(): Promise { const newAutoComletePaths = await this.buildAutoCompletePaths(); this.additionalAutoCompletePaths = newAutoComletePaths; - if (!isTestExecution()) { - await this.proposeNewLanguageServerPopup.showBanner(); - } return this.restartLanguageServer(); } private restartLanguageServer(): Promise { diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index ae0c34aa5a6d..fea8949e050e 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -16,13 +16,11 @@ import { import { LSNotSupportedDiagnosticServiceId } from '../../client/application/diagnostics/checks/lsNotSupported'; import { IDiagnostic, IDiagnosticsService } from '../../client/application/diagnostics/types'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../client/common/application/types'; -import { LSControl, LSEnabled } from '../../client/common/experiments/groups'; import { IPlatformService } from '../../client/common/platform/types'; import { IConfigurationService, IDisposable, IDisposableRegistry, - IExperimentsManager, IOutputChannel, IPersistentState, IPersistentStateFactory, @@ -53,7 +51,6 @@ suite('Language Server Activation - ActivationService', () => { let lsNotSupportedDiagnosticService: TypeMoq.IMock; let stateFactory: TypeMoq.IMock; let state: TypeMoq.IMock>; - let experiments: TypeMoq.IMock; let workspaceConfig: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; let interpreterChangedHandler!: Function; @@ -67,7 +64,6 @@ suite('Language Server Activation - ActivationService', () => { state = TypeMoq.Mock.ofType>(); const configService = TypeMoq.Mock.ofType(); pythonSettings = TypeMoq.Mock.ofType(); - experiments = TypeMoq.Mock.ofType(); const langFolderServiceMock = TypeMoq.Mock.ofType(); const folderVer: FolderVersionPair = { path: '', @@ -179,16 +175,10 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => activator.object) .verifiable(TypeMoq.Times.once()); - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); - await activationService.activate(undefined); activator.verifyAll(); serviceContainer.verifyAll(); - experiments.verifyAll(); } async function testReloadMessage(settingName: string): Promise { @@ -205,8 +195,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); workspaceService.verifyAll(); @@ -244,8 +233,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await testActivation(activationService, activator, true); @@ -255,8 +243,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await testActivation(activationService, activator, false); @@ -267,8 +254,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await testActivation(activationService, activator); @@ -278,8 +264,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await testActivation(activationService, activator); @@ -294,8 +279,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await testActivation(activationService, activator, false, LanguageServerType.None); }); @@ -316,8 +300,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); workspaceService.verifyAll(); @@ -392,8 +375,7 @@ suite('Language Server Activation - ActivationService', () => { pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); const ls1 = await activationService.get(folder1.uri, interpreter1); const ls2 = await activationService.get(folder1.uri, interpreter2); @@ -463,8 +445,7 @@ suite('Language Server Activation - ActivationService', () => { pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await activationService.activate(folder1.uri); await interpreterChangedHandler(); @@ -486,8 +467,7 @@ suite('Language Server Activation - ActivationService', () => { const activatorJedi = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); const diagnostics: IDiagnostic[] = []; lsNotSupportedDiagnosticService @@ -562,17 +542,12 @@ suite('Language Server Activation - ActivationService', () => { .setup((w) => w.getWorkspaceFolderIdentifier(resource, '')) .returns(() => resource!.fsPath) .verifiable(TypeMoq.Times.atLeastOnce()); - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); await activationService.activate(resource); activator.verifyAll(); serviceContainer.verifyAll(); workspaceService.verifyAll(); - experiments.verifyAll(); } test('Activator is disposed if activated workspace is removed and LS is "Microsoft"', async () => { pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); @@ -584,8 +559,7 @@ suite('Language Server Activation - ActivationService', () => { .verifiable(TypeMoq.Times.once()); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); workspaceService.verifyAll(); expect(workspaceFoldersChangedHandler).not.to.be.equal(undefined, 'Handler not set'); @@ -624,8 +598,7 @@ suite('Language Server Activation - ActivationService', () => { const activator1 = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; const folder2 = { name: 'two', uri: Uri.parse('two'), index: 2 }; @@ -642,15 +615,10 @@ suite('Language Server Activation - ActivationService', () => { .setup((a) => a.start(folder1.uri, undefined)) .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); await activationService.activate(folder1.uri); activator1.verifyAll(); activator1.verify((a) => a.activate(), TypeMoq.Times.once()); serviceContainer.verifyAll(); - experiments.verifyAll(); const activator2 = TypeMoq.Mock.ofType(); serviceContainer @@ -667,16 +635,11 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.never()); activator2.setup((a) => a.activate()).verifiable(TypeMoq.Times.never()); - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); await activationService.activate(folder2.uri); serviceContainer.verifyAll(); activator1.verifyAll(); activator1.verify((a) => a.activate(), TypeMoq.Times.exactly(2)); activator2.verifyAll(); - experiments.verifyAll(); }); } } @@ -693,7 +656,6 @@ suite('Language Server Activation - ActivationService', () => { let lsNotSupportedDiagnosticService: TypeMoq.IMock; let stateFactory: TypeMoq.IMock; let state: TypeMoq.IMock>; - let experiments: TypeMoq.IMock; let workspaceConfig: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; setup(() => { @@ -706,7 +668,6 @@ suite('Language Server Activation - ActivationService', () => { state = TypeMoq.Mock.ofType>(); const configService = TypeMoq.Mock.ofType(); pythonSettings = TypeMoq.Mock.ofType(); - experiments = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); const e = new EventEmitter(); interpreterService.setup((i) => i.onDidChangeInterpreter).returns(() => e.event); @@ -785,8 +746,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Jedi); @@ -805,8 +765,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Microsoft); @@ -825,8 +784,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Jedi); @@ -845,8 +803,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Jedi); @@ -854,204 +811,6 @@ suite('Language Server Activation - ActivationService', () => { }); }); - suite('Test useJedi()', () => { - let serviceContainer: TypeMoq.IMock; - let pythonSettings: TypeMoq.IMock; - let appShell: TypeMoq.IMock; - let cmdManager: TypeMoq.IMock; - let workspaceService: TypeMoq.IMock; - let platformService: TypeMoq.IMock; - let lsNotSupportedDiagnosticService: TypeMoq.IMock; - let stateFactory: TypeMoq.IMock; - let state: TypeMoq.IMock>; - let experiments: TypeMoq.IMock; - let workspaceConfig: TypeMoq.IMock; - let interpreterService: TypeMoq.IMock; - setup(() => { - serviceContainer = TypeMoq.Mock.ofType(); - appShell = TypeMoq.Mock.ofType(); - workspaceService = TypeMoq.Mock.ofType(); - cmdManager = TypeMoq.Mock.ofType(); - platformService = TypeMoq.Mock.ofType(); - stateFactory = TypeMoq.Mock.ofType(); - state = TypeMoq.Mock.ofType>(); - const configService = TypeMoq.Mock.ofType(); - pythonSettings = TypeMoq.Mock.ofType(); - experiments = TypeMoq.Mock.ofType(); - interpreterService = TypeMoq.Mock.ofType(); - const e = new EventEmitter(); - interpreterService.setup((i) => i.onDidChangeInterpreter).returns(() => e.event); - const langFolderServiceMock = TypeMoq.Mock.ofType(); - const folderVer: FolderVersionPair = { - path: '', - version: new SemVer('1.2.3') - }; - lsNotSupportedDiagnosticService = TypeMoq.Mock.ofType(); - workspaceService.setup((w) => w.hasWorkspaceFolders).returns(() => false); - workspaceService.setup((w) => w.workspaceFolders).returns(() => []); - configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); - langFolderServiceMock - .setup((l) => l.getCurrentLanguageServerDirectory()) - .returns(() => Promise.resolve(folderVer)); - stateFactory - .setup((f) => - f.createGlobalPersistentState( - TypeMoq.It.isValue('SWITCH_LS'), - TypeMoq.It.isAny(), - TypeMoq.It.isAny() - ) - ) - .returns(() => state.object); - state.setup((s) => s.value).returns(() => undefined); - state.setup((s) => s.updateValue(TypeMoq.It.isAny())).returns(() => Promise.resolve()); - workspaceConfig = TypeMoq.Mock.ofType(); - workspaceService - .setup((ws) => ws.getConfiguration('python', TypeMoq.It.isAny())) - .returns(() => workspaceConfig.object); - const output = TypeMoq.Mock.ofType(); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IOutputChannel), TypeMoq.It.isAny())) - .returns(() => output.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IWorkspaceService))) - .returns(() => workspaceService.object); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IApplicationShell))).returns(() => appShell.object); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => []); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IConfigurationService))) - .returns(() => configService.object); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(ICommandManager))).returns(() => cmdManager.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) - .returns(() => platformService.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterService))) - .returns(() => interpreterService.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerFolderService))) - .returns(() => langFolderServiceMock.object); - serviceContainer - .setup((s) => - s.get( - TypeMoq.It.isValue(IDiagnosticsService), - TypeMoq.It.isValue(LSNotSupportedDiagnosticServiceId) - ) - ) - .returns(() => lsNotSupportedDiagnosticService.object); - }); - - test('If default value (Jedi) of language server is being used, and LSEnabled experiment is enabled, then return false', async () => { - const settings = {}; - experiments - .setup((ex) => ex.inExperiment(LSEnabled)) - .returns(() => true) - .verifiable(TypeMoq.Times.once()); - experiments - .setup((ex) => ex.sendTelemetryIfInExperiment(TypeMoq.It.isAny())) - .returns(() => undefined) - .verifiable(TypeMoq.Times.never()); - workspaceConfig - .setup((c) => c.inspect('languageServer')) - .returns(() => settings as any) - .verifiable(TypeMoq.Times.once()); - - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - const result = activationService.useJedi(); - expect(result).to.equal(false, 'LS should be enabled'); - - workspaceService.verifyAll(); - workspaceConfig.verifyAll(); - experiments.verifyAll(); - }); - - test('If default value of languageServer (Jedi) is being used, and LSEnabled experiment is disabled, then send telemetry if user is in Experiment LSControl and useJedi() should be true)', async () => { - const settings = {}; - experiments - .setup((ex) => ex.inExperiment(LSEnabled)) - .returns(() => false) - .verifiable(TypeMoq.Times.once()); - experiments - .setup((ex) => ex.sendTelemetryIfInExperiment(LSControl)) - .returns(() => undefined) - .verifiable(TypeMoq.Times.once()); - workspaceConfig - .setup((c) => c.inspect('languageServer')) - .returns(() => settings as any) - .verifiable(TypeMoq.Times.once()); - - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - const result = activationService.useJedi(); - expect(result).to.equal(true, 'Return value should be true'); - - pythonSettings.verifyAll(); - experiments.verifyAll(); - workspaceService.verifyAll(); - workspaceConfig.verifyAll(); - }); - - suite( - 'If default value of languageServer (Jedi) is not being used, then no experiments are used, and python settings value is returned', - async () => { - [ - { - testName: 'Returns false when python settings value is Microsoft', - pythonSettingsValue: LanguageServerType.Microsoft, - expectedResult: false - }, - { - testName: 'Returns true when python settings value is Jedi', - pythonSettingsValue: LanguageServerType.Jedi, - expectedResult: true - } - ].forEach((testParams) => { - test(testParams.testName, async () => { - const settings = { workspaceFolderValue: true }; - experiments - .setup((ex) => ex.inExperiment(LSEnabled)) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); - experiments - .setup((ex) => ex.sendTelemetryIfInExperiment(LSControl)) - .returns(() => undefined) - .verifiable(TypeMoq.Times.never()); - workspaceConfig - .setup((c) => c.inspect('languageServer')) - .returns(() => settings as any) - .verifiable(TypeMoq.Times.once()); - pythonSettings - .setup((p) => p.languageServer) - .returns(() => testParams.pythonSettingsValue) - .verifiable(TypeMoq.Times.once()); - - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - const result = activationService.useJedi(); - expect(result).to.equal( - testParams.expectedResult, - `Return value should be ${testParams.expectedResult}` - ); - - pythonSettings.verifyAll(); - experiments.verifyAll(); - workspaceService.verifyAll(); - workspaceConfig.verifyAll(); - }); - }); - } - ); - }); - suite('Function isJediUsingDefaultConfiguration()', () => { let serviceContainer: TypeMoq.IMock; let pythonSettings: TypeMoq.IMock; @@ -1062,7 +821,6 @@ suite('Language Server Activation - ActivationService', () => { let lsNotSupportedDiagnosticService: TypeMoq.IMock; let stateFactory: TypeMoq.IMock; let state: TypeMoq.IMock>; - let experiments: TypeMoq.IMock; let workspaceConfig: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; setup(() => { @@ -1075,7 +833,6 @@ suite('Language Server Activation - ActivationService', () => { state = TypeMoq.Mock.ofType>(); const configService = TypeMoq.Mock.ofType(); pythonSettings = TypeMoq.Mock.ofType(); - experiments = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); const e = new EventEmitter(); interpreterService.setup((i) => i.onDidChangeInterpreter).returns(() => e.event); @@ -1166,8 +923,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); const result = activationService.isJediUsingDefaultConfiguration(Uri.parse('a')); expect(result).to.equal(expectedResult); @@ -1187,8 +943,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); const result = activationService.isJediUsingDefaultConfiguration(Uri.parse('a')); expect(result).to.equal(false, 'Return value should be false'); diff --git a/src/test/activation/languageServer/activator.unit.test.ts b/src/test/activation/languageServer/activator.unit.test.ts index 87fa327568a7..1b393fa95488 100644 --- a/src/test/activation/languageServer/activator.unit.test.ts +++ b/src/test/activation/languageServer/activator.unit.test.ts @@ -21,9 +21,10 @@ import { PythonSettings } from '../../../client/common/configSettings'; import { ConfigurationService } from '../../../client/common/configuration/service'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../client/common/platform/types'; -import { IConfigurationService, IPythonSettings } from '../../../client/common/types'; +import { IConfigurationService, IPythonExtensionBanner, IPythonSettings } from '../../../client/common/types'; import { createDeferred } from '../../../client/common/utils/async'; import { EXTENSION_ROOT_DIR } from '../../../client/constants'; +import { ProposePylanceBanner } from '../../../client/languageServices/proposeLanguageServerBanner'; import { sleep } from '../../core'; // tslint:disable:max-func-body-length @@ -37,6 +38,7 @@ suite('Language Server - Activator', () => { let lsFolderService: ILanguageServerFolderService; let configuration: IConfigurationService; let settings: IPythonSettings; + let banner: IPythonExtensionBanner; setup(() => { manager = mock(DotNetLanguageServerManager); workspaceService = mock(WorkspaceService); @@ -45,6 +47,7 @@ suite('Language Server - Activator', () => { lsFolderService = mock(DotNetLanguageServerFolderService); configuration = mock(ConfigurationService); settings = mock(PythonSettings); + banner = mock(ProposePylanceBanner); when(configuration.getSettings(anything())).thenReturn(instance(settings)); activator = new DotNetLanguageServerActivator( instance(manager), @@ -52,7 +55,8 @@ suite('Language Server - Activator', () => { instance(fs), instance(lsDownloader), instance(lsFolderService), - instance(configuration) + instance(configuration), + instance(banner) ); }); test('Manager must be started without any workspace', async () => { diff --git a/src/test/activation/node/languageServerFolderService.unit.test.ts b/src/test/activation/node/languageServerFolderService.unit.test.ts index 115bdecc7eda..b944dde8ffb6 100644 --- a/src/test/activation/node/languageServerFolderService.unit.test.ts +++ b/src/test/activation/node/languageServerFolderService.unit.test.ts @@ -9,10 +9,10 @@ import { Extension, Uri, WorkspaceConfiguration } from 'vscode'; import { ILanguageServerFolder, ILSExtensionApi, - NodeLanguageServerFolderService, - PylanceExtensionName + NodeLanguageServerFolderService } from '../../../client/activation/node/languageServerFolderService'; import { IWorkspaceService } from '../../../client/common/application/types'; +import { PYLANCE_EXTENSION_ID } from '../../../client/common/constants'; import { IConfigurationService, IExtensions, IPythonSettings } from '../../../client/common/types'; import { IServiceContainer } from '../../../client/ioc/types'; @@ -115,7 +115,7 @@ suite('Node Language Server Folder Service', () => { test('lsExtension not installed', async () => { pythonSettings.setup((p) => p.downloadLanguageServer).returns(() => true); workspaceConfiguration.setup((wc) => wc.get('packageName')).returns(() => undefined); - extensions.setup((e) => e.getExtension(PylanceExtensionName)).returns(() => undefined); + extensions.setup((e) => e.getExtension(PYLANCE_EXTENSION_ID)).returns(() => undefined); const folderService = new TestService( serviceContainer.object, @@ -148,7 +148,7 @@ suite('Node Language Server Folder Service', () => { extension.setup((e) => e.exports).returns(() => extensionApi); pythonSettings.setup((p) => p.downloadLanguageServer).returns(() => true); workspaceConfiguration.setup((wc) => wc.get('packageName')).returns(() => undefined); - extensions.setup((e) => e.getExtension(PylanceExtensionName)).returns(() => extension.object); + extensions.setup((e) => e.getExtension(PYLANCE_EXTENSION_ID)).returns(() => extension.object); folderService = new TestService( serviceContainer.object, configService.object, diff --git a/src/test/activation/serviceRegistry.unit.test.ts b/src/test/activation/serviceRegistry.unit.test.ts index 18e2bc853ba3..51219665453e 100644 --- a/src/test/activation/serviceRegistry.unit.test.ts +++ b/src/test/activation/serviceRegistry.unit.test.ts @@ -57,7 +57,7 @@ import { DataScienceSurveyBanner } from '../../client/datascience/dataScienceSur import { InteractiveShiftEnterBanner } from '../../client/datascience/shiftEnterBanner'; import { ServiceManager } from '../../client/ioc/serviceManager'; import { IServiceManager } from '../../client/ioc/types'; -import { ProposeLanguageServerBanner } from '../../client/languageServices/proposeLanguageServerBanner'; +import { ProposePylanceBanner } from '../../client/languageServices/proposeLanguageServerBanner'; // tslint:disable:max-func-body-length @@ -101,7 +101,7 @@ suite('Unit Tests - Language Server Activation Service Registry', () => { verify( serviceManager.addSingleton( IPythonExtensionBanner, - ProposeLanguageServerBanner, + ProposePylanceBanner, BANNER_NAME_PROPOSE_LS ) ).once(); diff --git a/src/test/common/configSettings/configSettings.unit.test.ts b/src/test/common/configSettings/configSettings.unit.test.ts index a07d1082fb8b..919459206484 100644 --- a/src/test/common/configSettings/configSettings.unit.test.ts +++ b/src/test/common/configSettings/configSettings.unit.test.ts @@ -193,6 +193,31 @@ suite('Python Settings', async () => { config.verifyAll(); }); + function testLanguageServer(languageServer: LanguageServerType, expectedValue: LanguageServerType) { + test(languageServer, () => { + expected.pythonPath = 'python3'; + expected.languageServer = languageServer; + initializeConfig(expected); + config + .setup((c) => c.get('languageServer')) + .returns(() => expected.languageServer) + .verifiable(TypeMoq.Times.once()); + + settings.update(config.object); + + expect(settings.languageServer).to.be.equal(expectedValue); + config.verifyAll(); + }); + } + + suite('languageServer settings', async () => { + Object.values(LanguageServerType).forEach(async (languageServer) => { + testLanguageServer(languageServer, languageServer); + }); + + testLanguageServer('invalid' as LanguageServerType, LanguageServerType.Jedi); + }); + function testExperiments(enabled: boolean) { expected.pythonPath = 'python3'; // tslint:disable-next-line:no-any diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index b0ac5a5220e6..974befd219b0 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -151,6 +151,7 @@ import { TerminalActivationProviders } from '../../client/common/terminal/types'; import { + BANNER_NAME_PROPOSE_LS, GLOBAL_MEMENTO, IAsyncDisposableRegistry, IBrowserService, @@ -170,6 +171,7 @@ import { IOutputChannel, IPathUtils, IPersistentStateFactory, + IPythonExtensionBanner, IPythonSettings, IsWindows, ProductType, @@ -365,6 +367,7 @@ import { InterpreterVersionService } from '../../client/interpreter/interpreterV import { registerInterpreterTypes } from '../../client/interpreter/serviceRegistry'; import { VirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs'; import { IVirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs/types'; +import { ProposePylanceBanner } from '../../client/languageServices/proposeLanguageServerBanner'; import { CacheableLocatorPromiseCache } from '../../client/pythonEnvironments/discovery/locators/services/cacheableLocatorService'; import { InterpreterType, PythonInterpreter } from '../../client/pythonEnvironments/info'; import { registerForIOC } from '../../client/pythonEnvironments/legacyIOC'; @@ -761,6 +764,11 @@ export class DataScienceIocContainer extends UnitTestIocContainer { LanguageServerType.Microsoft ); this.serviceManager.add(ILanguageServerManager, DotNetLanguageServerManager); + this.serviceManager.add( + IPythonExtensionBanner, + ProposePylanceBanner, + BANNER_NAME_PROPOSE_LS + ); } else if (languageServerType === LanguageServerType.Node) { this.serviceManager.add( ILanguageServerActivator, diff --git a/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts b/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts index 42e34a2106a0..db4911814901 100644 --- a/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts +++ b/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts @@ -3,103 +3,202 @@ 'use strict'; -// tslint:disable:no-any max-func-body-length +import { expect } from 'chai'; +import * as typemoq from 'typemoq'; +import { Extension } from 'vscode'; +import { LanguageServerType } from '../../../client/activation/types'; +import { IApplicationEnvironment, IApplicationShell } from '../../../client/common/application/types'; +import { PYLANCE_EXTENSION_ID } from '../../../client/common/constants'; +import { TryPylance } from '../../../client/common/experiments/groups'; +import { + IConfigurationService, + IExperimentService, + IExtensions, + IPersistentState, + IPersistentStateFactory, + IPythonSettings +} from '../../../client/common/types'; +import { LanguageService } from '../../../client/common/utils/localize'; +import { + getPylanceExtensionUri, + ProposeLSStateKeys, + ProposePylanceBanner +} from '../../../client/languageServices/proposeLanguageServerBanner'; -// import { expect } from 'chai'; -// import * as typemoq from 'typemoq'; -// import { IApplicationShell } from '../../../client/common/application/types'; -// import { IConfigurationService, IPersistentState, IPersistentStateFactory } from '../../../client/common/types'; -// import { -// ProposeLanguageServerBanner, -// ProposeLSStateKeys -// } from '../../../client/languageServices/proposeLanguageServerBanner'; +interface IExperimentLsCombination { + inExperiment: boolean; + lsType: LanguageServerType; + shouldShowBanner: boolean; +} +const testData: IExperimentLsCombination[] = [ + { inExperiment: true, lsType: LanguageServerType.None, shouldShowBanner: true }, + { inExperiment: true, lsType: LanguageServerType.Microsoft, shouldShowBanner: true }, + { inExperiment: true, lsType: LanguageServerType.Node, shouldShowBanner: false }, + { inExperiment: true, lsType: LanguageServerType.Jedi, shouldShowBanner: false }, + { inExperiment: false, lsType: LanguageServerType.None, shouldShowBanner: false }, + { inExperiment: false, lsType: LanguageServerType.Microsoft, shouldShowBanner: false }, + { inExperiment: false, lsType: LanguageServerType.Node, shouldShowBanner: false }, + { inExperiment: false, lsType: LanguageServerType.Jedi, shouldShowBanner: false } +]; -// suite('Propose New Language Server Banner', () => { -// let config: typemoq.IMock; -// let appShell: typemoq.IMock; -// const message = -// 'Try out Preview of our new Python Language Server to get richer and faster IntelliSense completions, and syntax errors as you type.'; -// const yes = 'Try it now'; -// const no = 'No thanks'; -// const later = 'Remind me Later'; +suite('Propose Pylance Banner', () => { + let config: typemoq.IMock; + let appShell: typemoq.IMock; + let appEnv: typemoq.IMock; + let settings: typemoq.IMock; -// setup(() => { -// config = typemoq.Mock.ofType(); -// appShell = typemoq.Mock.ofType(); -// }); -// test('Is debugger enabled upon creation?', () => { -// const enabledValue: boolean = true; -// const testBanner: ProposeLanguageServerBanner = preparePopup(enabledValue, 100, appShell.object, config.object); -// expect(testBanner.enabled).to.be.equal(true, 'Sampling 100/100 should always enable the banner.'); -// }); -// test('Do not show banner when it is disabled', () => { -// appShell -// .setup((a) => -// a.showInformationMessage( -// typemoq.It.isValue(message), -// typemoq.It.isValue(yes), -// typemoq.It.isValue(no), -// typemoq.It.isValue(later) -// ) -// ) -// .verifiable(typemoq.Times.never()); -// const enabled: boolean = true; -// const testBanner: ProposeLanguageServerBanner = preparePopup(enabled, 0, appShell.object, config.object); -// testBanner.showBanner().ignoreErrors(); -// }); -// test('shouldShowBanner must return false when Banner is implicitly disabled by sampling', () => { -// const enabled: boolean = true; -// const testBanner: ProposeLanguageServerBanner = preparePopup(enabled, 0, appShell.object, config.object); -// expect(testBanner.enabled).to.be.equal(false, 'We implicitly disabled the banner, it should never show.'); -// }); -// test('shouldShowBanner must return false when Banner is explicitly disabled', async () => { -// const enabled: boolean = true; -// const testBanner: ProposeLanguageServerBanner = preparePopup(enabled, 100, appShell.object, config.object); + const message = LanguageService.proposePylanceMessage(); + const yes = LanguageService.tryItNow(); + const no = LanguageService.bannerLabelNo(); + const later = LanguageService.remindMeLater(); -// expect(await testBanner.shouldShowBanner()).to.be.equal( -// true, -// '100% sample size should always make the banner enabled.' -// ); -// await testBanner.disable(); -// expect(await testBanner.shouldShowBanner()).to.be.equal( -// false, -// 'Explicitly disabled banner shouldShowBanner != false.' -// ); -// }); -// }); + setup(() => { + config = typemoq.Mock.ofType(); + settings = typemoq.Mock.ofType(); + config.setup((x) => x.getSettings(typemoq.It.isAny())).returns(() => settings.object); + appShell = typemoq.Mock.ofType(); + appEnv = typemoq.Mock.ofType(); + appEnv.setup((x) => x.uriScheme).returns(() => 'scheme'); + }); + testData.forEach((t) => { + test(`${t.inExperiment ? 'In' : 'Not in'} experiment and "python.languageServer": "${t.lsType}" should ${ + t.shouldShowBanner ? 'show' : 'not show' + } banner`, async () => { + settings.setup((x) => x.languageServer).returns(() => t.lsType); + const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, t.inExperiment, false); + const actual = await testBanner.shouldShowBanner(); + expect(actual).to.be.equal(t.shouldShowBanner, `shouldShowBanner() returned ${actual}`); + }); + }); + testData.forEach((t) => { + test(`When Pylance is installed, banner should not be shown when "python.languageServer": "${t.lsType}"`, async () => { + settings.setup((x) => x.languageServer).returns(() => t.lsType); + const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, t.inExperiment, true); + const actual = await testBanner.shouldShowBanner(); + expect(actual).to.be.equal(false, `shouldShowBanner() returned ${actual}`); + }); + }); + test('Do not show banner when it is disabled', async () => { + appShell + .setup((a) => + a.showInformationMessage( + typemoq.It.isValue(message), + typemoq.It.isValue(yes), + typemoq.It.isValue(no), + typemoq.It.isValue(later) + ) + ) + .verifiable(typemoq.Times.never()); + const testBanner = preparePopup(false, appShell.object, appEnv.object, config.object, true, false); + await testBanner.showBanner(); + appShell.verifyAll(); + }); + test('Clicking No should disable the banner', async () => { + appShell + .setup((a) => + a.showInformationMessage( + typemoq.It.isValue(message), + typemoq.It.isValue(yes), + typemoq.It.isValue(no), + typemoq.It.isValue(later) + ) + ) + .returns(async () => no) + .verifiable(typemoq.Times.once()); + appShell.setup((a) => a.openUrl(getPylanceExtensionUri(appEnv.object))).verifiable(typemoq.Times.never()); -// function preparePopup( -// enabledValue: boolean, -// sampleValue: number, -// appShell: IApplicationShell, -// config: IConfigurationService -// ): ProposeLanguageServerBanner { -// const myfactory: typemoq.IMock = typemoq.Mock.ofType(); -// const val: typemoq.IMock> = typemoq.Mock.ofType>(); -// val.setup((a) => a.updateValue(typemoq.It.isValue(true))).returns(() => { -// enabledValue = true; -// return Promise.resolve(); -// }); -// val.setup((a) => a.updateValue(typemoq.It.isValue(false))).returns(() => { -// enabledValue = false; -// return Promise.resolve(); -// }); -// val.setup((a) => a.value).returns(() => { -// return enabledValue; -// }); -// myfactory -// .setup((a) => -// a.createGlobalPersistentState(typemoq.It.isValue(ProposeLSStateKeys.ShowBanner), typemoq.It.isValue(true)) -// ) -// .returns(() => { -// return val.object; -// }); -// myfactory -// .setup((a) => -// a.createGlobalPersistentState(typemoq.It.isValue(ProposeLSStateKeys.ShowBanner), typemoq.It.isValue(false)) -// ) -// .returns(() => { -// return val.object; -// }); -// return new ProposeLanguageServerBanner(appShell, myfactory.object, config, sampleValue); -// } + const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, true, false); + await testBanner.showBanner(); + expect(testBanner.enabled).to.be.equal(false, 'Banner should be permanently disabled when user clicked No'); + appShell.verifyAll(); + }); + test('Clicking Later should disable banner in session', async () => { + appShell + .setup((a) => + a.showInformationMessage( + typemoq.It.isValue(message), + typemoq.It.isValue(yes), + typemoq.It.isValue(no), + typemoq.It.isValue(later) + ) + ) + .returns(async () => later) + .verifiable(typemoq.Times.once()); + appShell.setup((a) => a.openUrl(getPylanceExtensionUri(appEnv.object))).verifiable(typemoq.Times.never()); + + const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, true, false); + await testBanner.showBanner(); + expect(testBanner.enabled).to.be.equal( + true, + 'Banner should not be permanently disabled when user clicked Later' + ); + appShell.verifyAll(); + }); + test('Clicking Yes opens the extension marketplace entry', async () => { + appShell + .setup((a) => + a.showInformationMessage( + typemoq.It.isValue(message), + typemoq.It.isValue(yes), + typemoq.It.isValue(no), + typemoq.It.isValue(later) + ) + ) + .returns(async () => yes) + .verifiable(typemoq.Times.once()); + appShell.setup((a) => a.openUrl(getPylanceExtensionUri(appEnv.object))).verifiable(typemoq.Times.once()); + + const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, true, false); + await testBanner.showBanner(); + expect(testBanner.enabled).to.be.equal(false, 'Banner should be permanently disabled after opening store URL'); + appShell.verifyAll(); + }); +}); + +function preparePopup( + enabledValue: boolean, + appShell: IApplicationShell, + appEnv: IApplicationEnvironment, + config: IConfigurationService, + inExperiment: boolean, + pylanceInstalled: boolean +): ProposePylanceBanner { + const myfactory = typemoq.Mock.ofType(); + const val = typemoq.Mock.ofType>(); + val.setup((a) => a.updateValue(typemoq.It.isValue(true))).returns(() => { + enabledValue = true; + return Promise.resolve(); + }); + val.setup((a) => a.updateValue(typemoq.It.isValue(false))).returns(() => { + enabledValue = false; + return Promise.resolve(); + }); + val.setup((a) => a.value).returns(() => { + return enabledValue; + }); + myfactory + .setup((a) => + a.createGlobalPersistentState(typemoq.It.isValue(ProposeLSStateKeys.ShowBanner), typemoq.It.isValue(true)) + ) + .returns(() => { + return val.object; + }); + myfactory + .setup((a) => + a.createGlobalPersistentState(typemoq.It.isValue(ProposeLSStateKeys.ShowBanner), typemoq.It.isValue(false)) + ) + .returns(() => { + return val.object; + }); + + const experiments = typemoq.Mock.ofType(); + experiments.setup((x) => x.inExperiment(TryPylance.experiment)).returns(() => Promise.resolve(inExperiment)); + + const extensions = typemoq.Mock.ofType(); + // tslint:disable-next-line: no-any + const extension = typemoq.Mock.ofType>(); + extensions + .setup((x) => x.getExtension(PYLANCE_EXTENSION_ID)) + .returns(() => (pylanceInstalled ? extension.object : undefined)); + return new ProposePylanceBanner(appShell, appEnv, myfactory.object, config, experiments.object, extensions.object); +} From 46dc0830124073f16b4280bc60bd475afc1ffaff Mon Sep 17 00:00:00 2001 From: David Kutugata Date: Wed, 22 Jul 2020 17:17:38 -0700 Subject: [PATCH 17/17] Port fix the gather survey (#13086) (#13105) * Fix the gather survey (#13086) * fix the gather survey added 'gather stats' telemetry mention the gather comments to update the python ext * oops * fix tests and address comments * update gather stats when resetting the kernel * set globalstate vars to 0 when we open vs code * fix gather stats telemetry * fix tests * fix tests for real --- package.json | 11 ++++++++++ package.nls.json | 12 ++++++++--- src/client/common/application/commands.ts | 1 + src/client/common/utils/localize.ts | 4 ++-- .../datascience/commands/commandRegistry.ts | 9 +++++++-- src/client/datascience/constants.ts | 2 ++ .../datascience/gather/gatherListener.ts | 20 +++++++++++++++++++ .../interactive-common/linkProvider.ts | 4 ++-- src/client/telemetry/index.ts | 6 ++++++ 9 files changed, 60 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index 2197d57833a6..e60bfb589563 100644 --- a/package.json +++ b/package.json @@ -765,6 +765,11 @@ "command": "python.datascience.gatherquality", "title": "DataScience.gatherQuality", "category": "Python" + }, + { + "command": "python.datascience.latestExtension", + "title": "DataScience.latestExtension", + "category": "Python" } ], "menus": { @@ -1276,6 +1281,12 @@ "category": "Python", "when": "false" }, + { + "command": "python.datascience.latestExtension", + "title": "%DataScience.latestExtension%", + "category": "Python", + "when": "false" + }, { "command": "python.datascience.export", "title": "%DataScience.notebookExportAs%", diff --git a/package.nls.json b/package.nls.json index 8103c7b7e653..2bef35ccb426 100644 --- a/package.nls.json +++ b/package.nls.json @@ -475,8 +475,8 @@ "DataScience.findJupyterCommandProgressCheckInterpreter": "Checking {0}.", "DataScience.findJupyterCommandProgressSearchCurrentPath": "Searching current path.", "DataScience.gatherError": "Gather internal error", - "DataScience.gatheredScriptDescription": "# This file was generated by the Gather Extension.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n", - "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|   |This notebook was generated by the Gather Extension. The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://command:python.datascience.gatherquality?yes) [No](https://command:python.datascience.gatherquality?no)", + "DataScience.gatheredScriptDescription": "# This file was generated by the Gather Extension.\n# It requires version 2020.7.94776 (or newer) of the Python Extension.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n", + "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|   |This notebook was generated by the Gather Extension. It requires version 2020.7.94776 (or newer) of the Python Extension, please update [here](https://command:python.datascience.latestExtension). The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://command:python.datascience.gatherquality?yes) [No](https://command:python.datascience.gatherquality?no)", "DataScience.savePngTitle": "Save Image", "DataScience.jupyterSelectURIQuickPickTitle": "Pick how to connect to Jupyter", "DataScience.jupyterSelectURIQuickPickPlaceholder": "Choose an option", @@ -515,6 +515,7 @@ "DataScience.jupyterSelectURIQuickPickTitleRemoteOnly": "Pick an already running jupyter server", "DataScience.jupyterSelectURIRemoteDetail": "Specify the URI of an existing server", "DataScience.gatherQuality": "Did gather work as desired?", + "DataScience.latestExtension": "Download the latest version of the Python Extension", "DataScience.loadClassFailedWithNoInternet": "Error loading {0}:{1}. Internet connection required for loading 3rd party widgets.", "DataScience.useCDNForWidgets": "Widgets require us to download supporting files from a 3rd party website. Click [here](https://aka.ms/PVSCIPyWidgets) for more information.", "DataScience.loadThirdPartyWidgetScriptsPostEnabled": "Please restart the Kernel when changing the setting 'python.dataScience.widgetScriptSources'.", @@ -557,5 +558,10 @@ "DataScienceRendererExtension.downloadingMessage": "Downloading Notebook Renderers Extension...", "DataScienceRendererExtension.downloadCompletedOutputMessage": "Notebook Renderers extension download complete.", "DataScience.uriProviderDescriptionFormat": "{0} (From {1} extension)", - "DataScience.unknownPackage": "unknown" + "DataScience.unknownPackage": "unknown", + "DataScience.interactiveWindowTitleFormat": "Python Interactive - {0}", + "DataScience.interactiveWindowModeBannerTitle": "Do you want to open a new Python Interactive window for this file? [More Information](command:workbench.action.openSettings?%5B%22python.dataScience.interactiveWindowMode%22%5D).", + "DataScience.interactiveWindowModeBannerSwitchYes": "Yes", + "DataScience.interactiveWindowModeBannerSwitchAlways": "Always", + "DataScience.interactiveWindowModeBannerSwitchNo": "No" } diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 9e35df989c56..3a5985e972aa 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -182,6 +182,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu [DSCommands.SaveAsNotebookNonCustomEditor]: [Uri, Uri]; [DSCommands.OpenNotebookNonCustomEditor]: [Uri]; [DSCommands.GatherQuality]: [string]; + [DSCommands.LatestExtension]: [string]; [DSCommands.EnableLoadingWidgetsFrom3rdPartySource]: [undefined | never]; [DSCommands.TrustNotebook]: [undefined | never | Uri]; } diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 324fc2e2f914..ba07eddce146 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -870,11 +870,11 @@ export namespace DataScience { export const gatherError = localize('DataScience.gatherError', 'Gather internal error'); export const gatheredScriptDescription = localize( 'DataScience.gatheredScriptDescription', - '# This file was generated by the Gather Extension.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n' + '# This file was generated by the Gather Extension.\n# It requires version 2020.7.94776 (or newer) of the Python Extension.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n' ); export const gatheredNotebookDescriptionInMarkdown = localize( 'DataScience.gatheredNotebookDescriptionInMarkdown', - '# Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|   |This notebook was generated by the Gather Extension. The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://command:python.datascience.gatherquality?yes) [No](https://command:python.datascience.gatherquality?no)' + '# Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|   |This notebook was generated by the Gather Extension. It requires version 2020.7.94776 (or newer) of the Python Extension, please update [here](https://command:python.datascience.latestExtension). The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://command:python.datascience.gatherquality?yes) [No](https://command:python.datascience.gatherquality?no)' ); export const savePngTitle = localize('DataScience.savePngTitle', 'Save Image'); export const fallbackToUseActiveInterpeterAsKernel = localize( diff --git a/src/client/datascience/commands/commandRegistry.ts b/src/client/datascience/commands/commandRegistry.ts index d474d5175ec4..5bf2ded5f2f9 100644 --- a/src/client/datascience/commands/commandRegistry.ts +++ b/src/client/datascience/commands/commandRegistry.ts @@ -78,6 +78,7 @@ export class CommandRegistry implements IDisposable { this.registerCommand(Commands.CreateNewNotebook, this.createNewNotebook); this.registerCommand(Commands.ViewJupyterOutput, this.viewJupyterOutput); this.registerCommand(Commands.GatherQuality, this.reportGatherQuality); + this.registerCommand(Commands.LatestExtension, this.openPythonExtensionPage); this.registerCommand( Commands.EnableLoadingWidgetsFrom3rdPartySource, this.enableLoadingWidgetScriptsFromThirdParty @@ -388,7 +389,11 @@ export class CommandRegistry implements IDisposable { } private reportGatherQuality(val: string) { - sendTelemetryEvent(Telemetry.GatherQualityReport, undefined, { result: val === 'no' ? 'no' : 'yes' }); - env.openExternal(Uri.parse(`https://aka.ms/gathersurvey?succeed=${val}`)); + sendTelemetryEvent(Telemetry.GatherQualityReport, undefined, { result: val[0] === 'no' ? 'no' : 'yes' }); + env.openExternal(Uri.parse(`https://aka.ms/gathersurvey?succeed=${val[0]}`)); + } + + private openPythonExtensionPage() { + env.openExternal(Uri.parse(`https://marketplace.visualstudio.com/items?itemName=ms-python.python`)); } } diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index 03b38aaec1bf..43b550f1db4c 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -90,6 +90,7 @@ 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 LatestExtension = 'python.datascience.latestExtension'; export const TrustNotebook = 'python.datascience.notebookeditor.trust'; export const EnableLoadingWidgetsFrom3rdPartySource = 'python.datascience.enableLoadingWidgetScriptsFromThirdPartySource'; @@ -320,6 +321,7 @@ export enum Telemetry { KernelInvalid = 'DS_INTERNAL.INVALID_KERNEL_USED', GatherIsInstalled = 'DS_INTERNAL.GATHER_IS_INSTALLED', GatherCompleted = 'DATASCIENCE.GATHER_COMPLETED', + GatherStats = 'DS_INTERNAL.GATHER_STATS', GatheredNotebookSaved = 'DATASCIENCE.GATHERED_NOTEBOOK_SAVED', GatherQualityReport = 'DS_INTERNAL.GATHER_QUALITY_REPORT', ZMQSupported = 'DS_INTERNAL.ZMQ_NATIVE_BINARIES_LOADING', diff --git a/src/client/datascience/gather/gatherListener.ts b/src/client/datascience/gather/gatherListener.ts index 470e7ef477a3..95b6215b5e31 100644 --- a/src/client/datascience/gather/gatherListener.ts +++ b/src/client/datascience/gather/gatherListener.ts @@ -42,6 +42,8 @@ export class GatherListener implements IInteractiveWindowListener { private notebookUri: Uri | undefined; private gatherProvider: IGatherProvider | undefined; private gatherTimer: StopWatch | undefined; + private linesSubmitted: number = 0; + private cellsSubmitted: number = 0; constructor( @inject(IApplicationShell) private applicationShell: IApplicationShell, @@ -78,11 +80,22 @@ export class GatherListener implements IInteractiveWindowListener { break; case InteractiveWindowMessages.RestartKernel: + this.linesSubmitted = 0; + this.cellsSubmitted = 0; if (this.gatherProvider) { this.gatherProvider.resetLog(); } break; + case InteractiveWindowMessages.FinishCell: + const cell = payload as ICell; + if (cell && cell.data && cell.data.source) { + const lineCount: number = cell.data.source.length as number; + this.linesSubmitted += lineCount; + this.cellsSubmitted += 1; + } + break; + default: break; } @@ -155,6 +168,13 @@ export class GatherListener implements IInteractiveWindowListener { await this.showNotebook(slicedProgram, cell); sendTelemetryEvent(Telemetry.GatherCompleted, this.gatherTimer?.elapsedTime, { result: 'notebook' }); } + + sendTelemetryEvent(Telemetry.GatherStats, undefined, { + linesSubmitted: this.linesSubmitted, + cellsSubmitted: this.cellsSubmitted, + linesGathered: slicedProgram.splitLines().length, + cellsGathered: generateCellsFromString(slicedProgram).length + }); } }; diff --git a/src/client/datascience/interactive-common/linkProvider.ts b/src/client/datascience/interactive-common/linkProvider.ts index ea6461b66114..b8096ad616a6 100644 --- a/src/client/datascience/interactive-common/linkProvider.ts +++ b/src/client/datascience/interactive-common/linkProvider.ts @@ -19,6 +19,7 @@ const LineQueryRegex = /line=(\d+)/; // in a markdown cell using the syntax: https://command:[my.vscode.command]. const linkCommandWhitelist = [ 'python.datascience.gatherquality', + 'python.datascience.latestExtension', 'python.datascience.enableLoadingWidgetScriptsFromThirdPartySource' ]; @@ -52,8 +53,7 @@ export class LinkProvider implements IInteractiveWindowListener { this.openFile(href); } else if (href.startsWith('https://command:')) { const temp: string = href.split(':')[2]; - const params: string[] = - temp.includes('/?') && temp.includes(',') ? temp.split('/?')[1].split(',') : []; + const params: string[] = temp.includes('/?') ? temp.split('/?')[1].split(',') : []; let command = temp.split('/?')[0]; if (command.endsWith('/')) { command = command.substring(0, command.length - 1); diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index ff26bcf385b2..6be34d0f427e 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -2041,6 +2041,12 @@ export interface IEventNamePropertyMapping { */ result: 'err' | 'script' | 'notebook' | 'unavailable'; }; + [Telemetry.GatherStats]: { + linesSubmitted: number; + cellsSubmitted: number; + linesGathered: number; + cellsGathered: number; + }; /** * Telemetry event sent when a gathered notebook has been saved by the user. */