From 009f5b370104cf659c9dfa13391e91397bd2e901 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 27 Jun 2022 16:43:08 -0700 Subject: [PATCH 1/5] Improve how to notify the user they don't have Python installed when installing the Python extension for the first time --- package.json | 28 ++-- ...windows.md => install-python-windows-8.md} | 29 +++-- src/client/common/application/commands.ts | 2 + src/client/common/application/contextKeys.ts | 1 + src/client/common/application/walkThroughs.ts | 9 ++ src/client/common/constants.ts | 2 + src/client/common/utils/localize.ts | 8 ++ .../commands/installPython.ts | 63 +++++++++ .../commands/setInterpreter.ts | 101 +++++++++----- src/client/interpreter/serviceRegistry.ts | 5 + .../commands/installPython.unit.test.ts | 123 ++++++++++++++++++ .../commands/setInterpreter.unit.test.ts | 68 +++++++++- .../interpreters/serviceRegistry.unit.test.ts | 2 + 13 files changed, 376 insertions(+), 65 deletions(-) rename resources/walkthrough/{install-python-windows.md => install-python-windows-8.md} (60%) create mode 100644 src/client/common/application/walkThroughs.ts create mode 100644 src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts create mode 100644 src/test/configuration/interpreterSelector/commands/installPython.unit.test.ts diff --git a/package.json b/package.json index 6c0577f72b6e..647efdddc07b 100644 --- a/package.json +++ b/package.json @@ -107,13 +107,23 @@ "when": "workspacePlatform != webworker", "steps": [ { - "id": "python.installPythonWin", + "id": "python.createPythonFile", + "title": "Create a Python file", + "description": "[Open](command:toSide:workbench.action.files.openFile) or [create](command:toSide:workbench.action.files.newUntitledFile) a Python file - make sure to save it as \".py\".\n[Create Python File](command:toSide:workbench.action.files.newUntitledFile)", + "media": { + "svg": "resources/walkthrough/open-folder.svg", + "altText": "Open a Python file or a folder with a Python project." + }, + "when": "" + }, + { + "id": "python.installPythonWin8", "title": "Install Python", - "description": "The Python Extension requires Python to be installed. Install Python from the [Microsoft Store](https://aka.ms/AAd9rms).\n\n[Install Python](https://aka.ms/AAd9rms)\n", + "description": "The Python Extension requires Python to be installed. Install Python [from python.org](https://www.python.org/downloads).\n\n[Install Python](https://www.python.org/downloads)\n", "media": { - "markdown": "resources/walkthrough/install-python-windows.md" + "markdown": "resources/walkthrough/install-python-windows-8.md" }, - "when": "workspacePlatform == windows" + "when": "workspacePlatform == windows && showInstallTile" }, { "id": "python.installPythonMac", @@ -135,16 +145,6 @@ "when": "workspacePlatform == linux", "command": "workbench.action.terminal.new" }, - { - "id": "python.createPythonFile", - "title": "Create a Python file", - "description": "[Open](command:toSide:workbench.action.files.openFile) or [create](command:toSide:python.createNewFile) a Python file - make sure to save it as \".py\".\n[Create Python File](command:toSide:python.createNewFile)", - "media": { - "svg": "resources/walkthrough/open-folder.svg", - "altText": "Open a Python file or a folder with a Python project." - }, - "when": "" - }, { "id": "python.selectInterpreter", "title": "Select a Python Interpreter", diff --git a/resources/walkthrough/install-python-windows.md b/resources/walkthrough/install-python-windows-8.md similarity index 60% rename from resources/walkthrough/install-python-windows.md rename to resources/walkthrough/install-python-windows-8.md index 8723e613e733..f25f2f7d024d 100644 --- a/resources/walkthrough/install-python-windows.md +++ b/resources/walkthrough/install-python-windows-8.md @@ -1,14 +1,15 @@ -## Install Python on Windows - -If you don't have Python installed on your Windows machine, you can install it from the [Microsoft Store](https://aka.ms/AAd9rms). - -To verify it's installed, create a new terminal (Ctrl + Shift + `) and try running the following command: - -``` -python --version -``` - -You should see something similar to the following: -``` -Python 3.9.5 -``` +## Install Python on Windows + +If you don't have Python installed on your Windows machine, you can install it [from python.org](https://www.python.org/downloads). + +To verify it's installed, create a new terminal (Ctrl + Shift + `) and try running the following command: + +``` +python --version +``` + +You should see something similar to the following: +``` +Python 3.9.5 +``` +For additional information about using Python on Windows, see [Using Python on Windows at Python.org](https://docs.python.org/3.10/using/windows.html). diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 7ada08002e1a..01fce2dbe6e4 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -16,6 +16,7 @@ export type CommandsWithoutArgs = keyof ICommandNameWithoutArgumentTypeMapping; * @interface ICommandNameWithoutArgumentTypeMapping */ interface ICommandNameWithoutArgumentTypeMapping { + [Commands.InstallPython]: []; [Commands.ClearWorkspaceInterpreter]: []; [Commands.Set_Interpreter]: []; [Commands.Set_ShebangInterpreter]: []; @@ -57,6 +58,7 @@ export type AllCommands = keyof ICommandNameArgumentTypeMapping; export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgumentTypeMapping { ['vscode.openWith']: [Uri, string]; ['workbench.action.quickOpen']: [string]; + ['workbench.action.openWalkthrough']: [string | { category: string; step: string }, boolean | undefined]; ['workbench.extensions.installExtension']: [ Uri | 'ms-python.python', ( diff --git a/src/client/common/application/contextKeys.ts b/src/client/common/application/contextKeys.ts index 98fca50bcfa8..9e6fecea28d3 100644 --- a/src/client/common/application/contextKeys.ts +++ b/src/client/common/application/contextKeys.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. export enum ExtensionContextKey { + showInstallTile = 'showInstallTile', HasFailedTests = 'hasFailedTests', RefreshingTests = 'refreshingTests', } diff --git a/src/client/common/application/walkThroughs.ts b/src/client/common/application/walkThroughs.ts new file mode 100644 index 000000000000..89e57ee74e47 --- /dev/null +++ b/src/client/common/application/walkThroughs.ts @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +export enum PythonWelcome { + name = 'pythonWelcome', + windowsInstallId = 'python.installPythonWin8', + linuxInstallId = 'python.installPythonLinux', + macOSInstallId = 'python.installPythonMac', +} diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index 8dcbf65cdda1..b2fb4b9a200d 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -57,6 +57,7 @@ export namespace Commands { export const LaunchTensorBoard = 'python.launchTensorBoard'; export const RefreshTensorBoard = 'python.refreshTensorBoard'; export const ReportIssue = 'python.reportIssue'; + export const InstallPython = 'python.installPython'; } // Look at https://microsoft.github.io/vscode-codicons/dist/codicon.html for other Octicon icon ids @@ -72,6 +73,7 @@ export namespace Octicons { export const Star = '$(star-full)'; export const Gear = '$(gear)'; export const Warning = '$(warning)'; + export const Error = '$(error)'; } export const DEFAULT_INTERPRETER_SETTING = 'python'; diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index ba283792eb41..80ca63cc431e 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -280,6 +280,14 @@ export namespace Interpreters { } export namespace InterpreterQuickPickList { + export const noPythonInstalled = localize( + 'InterpreterQuickPickList.noPythonInstalled', + 'Python is not installed, please download and install it', + ); + export const clickForInstructions = localize( + 'InterpreterQuickPickList.clickForInstructions', + 'Click for instructions...', + ); export const globalGroupName = localize('InterpreterQuickPickList.globalGroupName', 'Global'); export const workspaceGroupName = localize('InterpreterQuickPickList.workspaceGroupName', 'Workspace'); export const enterPath = { diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts b/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts new file mode 100644 index 000000000000..3b015138bda0 --- /dev/null +++ b/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts @@ -0,0 +1,63 @@ +// 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 { ExtensionContextKey } from '../../../../common/application/contextKeys'; +import { ICommandManager, IContextKeyManager } from '../../../../common/application/types'; +import { PythonWelcome } from '../../../../common/application/walkThroughs'; +import { Commands, PVSC_EXTENSION_ID } from '../../../../common/constants'; +import { IBrowserService, IDisposableRegistry } from '../../../../common/types'; +import { IPlatformService } from '../../../../common/platform/types'; + +@injectable() +export class InstallPythonCommand implements IExtensionSingleActivationService { + public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: false }; + + constructor( + @inject(ICommandManager) private readonly commandManager: ICommandManager, + @inject(IContextKeyManager) private readonly contextManager: IContextKeyManager, + @inject(IBrowserService) private readonly browserService: IBrowserService, + @inject(IPlatformService) private readonly platformService: IPlatformService, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + ) {} + + public async activate(): Promise { + this.disposables.push(this.commandManager.registerCommand(Commands.InstallPython, () => this._installPython())); + } + + public async _installPython(): Promise { + if (this.platformService.isWindows) { + const version = await this.platformService.getVersion(); + if (version.major !== 8) { + // OS is not Windows 8, ms-windows-store URIs are available: + // https://docs.microsoft.com/en-us/windows/uwp/launch-resume/launch-store-app + this.browserService.launch('ms-windows-store://pdp/?ProductId=9PJPW5LDXLZ5'); + return; + } + } + this.showInstallTile(); + } + + private showInstallTile() { + this.contextManager.setContext(ExtensionContextKey.showInstallTile, true); + let step: string; + if (this.platformService.isWindows) { + step = PythonWelcome.windowsInstallId; + } else if (this.platformService.isLinux) { + step = PythonWelcome.linuxInstallId; + } else { + step = PythonWelcome.macOSInstallId; + } + this.commandManager.executeCommand( + 'workbench.action.openWalkthrough', + { + category: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}`, + step: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}#${step}`, + }, + false, + ); + } +} diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts index 3d9cd76f9f77..d1be191ebfe2 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts @@ -74,6 +74,12 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { alwaysShow: true, }; + private readonly noPythonInstalled: ISpecialQuickPickItem = { + label: `${Octicons.Error} ${InterpreterQuickPickList.noPythonInstalled}`, + detail: InterpreterQuickPickList.clickForInstructions, + alwaysShow: true, + }; + constructor( @inject(IApplicationShell) applicationShell: IApplicationShell, @inject(IPathUtils) pathUtils: IPathUtils, @@ -164,11 +170,12 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { } else if (selection.label === this.manualEntrySuggestion.label) { sendTelemetryEvent(EventName.SELECT_INTERPRETER_ENTER_OR_FIND); return this._enterOrBrowseInterpreterPath(input, state, suggestions); + } else if (selection.label === this.noPythonInstalled.label) { + this.commandManager.executeCommand(Commands.InstallPython); } else { sendTelemetryEvent(EventName.SELECT_INTERPRETER_SELECTED, undefined, { action: 'selected' }); state.path = (selection as IInterpreterQuickPickItem).path; } - return undefined; } @@ -179,7 +186,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { suggestions.push(defaultInterpreterPathSuggestion); } const interpreterSuggestions = this.getSuggestions(resource); - this.setRecommendedItem(interpreterSuggestions, resource); + this.finalizeItems(interpreterSuggestions, resource); suggestions.push(...interpreterSuggestions); return suggestions; } @@ -214,7 +221,10 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { if (firstInterpreterSuggestion) { return firstInterpreterSuggestion; } - return suggestions[0]; + const noPythonInstalledItem = suggestions.find( + (i) => isSpecialQuickPickItem(i) && i.label === this.noPythonInstalled.label, + ); + return noPythonInstalledItem ?? suggestions[0]; } private getDefaultInterpreterPathSuggestion(resource: Resource): ISpecialQuickPickItem | undefined { @@ -286,6 +296,12 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { !areItemsGrouped, ); if (envIndex === -1) { + const noPyIndex = updatedItems.findIndex( + (item) => isSpecialQuickPickItem(item) && item.label === this.noPythonInstalled.label, + ); + if (noPyIndex !== -1) { + updatedItems.splice(noPyIndex, 1); + } if (areItemsGrouped) { addSeparatorIfApplicable( updatedItems, @@ -301,44 +317,57 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand { if (envIndex !== -1 && event.new === undefined) { updatedItems.splice(envIndex, 1); } - this.setRecommendedItem(updatedItems, resource); + this.finalizeItems(updatedItems, resource); return updatedItems; } - private setRecommendedItem(items: QuickPickType[], resource: Resource) { + private finalizeItems(items: QuickPickType[], resource: Resource) { const interpreterSuggestions = this.interpreterSelector.getSuggestions(resource, true); - if (!this.interpreterService.refreshPromise && interpreterSuggestions.length > 0) { - const suggestion = this.interpreterSelector.getRecommendedSuggestion( - interpreterSuggestions, - this.workspaceService.getWorkspaceFolder(resource)?.uri, - ); - if (!suggestion) { - return; - } - const areItemsGrouped = items.find((item) => isSeparatorItem(item) && item.label === EnvGroups.Recommended); - const recommended = cloneDeep(suggestion); - recommended.label = `${Octicons.Star} ${recommended.label}`; - recommended.description = areItemsGrouped - ? // No need to add a tag as "Recommended" group already exists. - recommended.description - : `${recommended.description ?? ''} - ${Common.recommended}`; - const index = items.findIndex( - (item) => isInterpreterQuickPickItem(item) && item.interpreter.id === recommended.interpreter.id, - ); - if (index !== -1) { - items[index] = recommended; - } - items.forEach((item, i) => { - if ( - isInterpreterQuickPickItem(item) && - item.interpreter.path === 'python' && - item.interpreter.envType === EnvironmentType.Conda - ) { - if (!items[i].label.includes(Octicons.Warning)) { - items[i].label = `${Octicons.Warning} ${items[i].label}`; + if (!this.interpreterService.refreshPromise) { + if (interpreterSuggestions.length) { + this.setRecommendedItem(interpreterSuggestions, items, resource); + // Add warning label to certain environments + items.forEach((item, i) => { + if ( + isInterpreterQuickPickItem(item) && + item.interpreter.path === 'python' && + item.interpreter.envType === EnvironmentType.Conda + ) { + if (!items[i].label.includes(Octicons.Warning)) { + items[i].label = `${Octicons.Warning} ${items[i].label}`; + } } - } - }); + }); + } else { + items.push(this.noPythonInstalled); + } + } + } + + private setRecommendedItem( + interpreterSuggestions: IInterpreterQuickPickItem[], + items: QuickPickType[], + resource: Resource, + ) { + const suggestion = this.interpreterSelector.getRecommendedSuggestion( + interpreterSuggestions, + this.workspaceService.getWorkspaceFolder(resource)?.uri, + ); + if (!suggestion) { + return; + } + const areItemsGrouped = items.find((item) => isSeparatorItem(item) && item.label === EnvGroups.Recommended); + const recommended = cloneDeep(suggestion); + recommended.label = `${Octicons.Star} ${recommended.label}`; + recommended.description = areItemsGrouped + ? // No need to add a tag as "Recommended" group already exists. + recommended.description + : `${recommended.description ?? ''} - ${Common.recommended}`; + const index = items.findIndex( + (item) => isInterpreterQuickPickItem(item) && item.interpreter.id === recommended.interpreter.id, + ); + if (index !== -1) { + items[index] = recommended; } } diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index bf586dd4e959..af39479601de 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -11,6 +11,7 @@ import { InterpreterAutoSelectionService } from './autoSelection/index'; import { InterpreterAutoSelectionProxyService } from './autoSelection/proxy'; import { IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService } from './autoSelection/types'; import { EnvironmentTypeComparer } from './configuration/environmentTypeComparer'; +import { InstallPythonCommand } from './configuration/interpreterSelector/commands/installPython'; import { ResetInterpreterCommand } from './configuration/interpreterSelector/commands/resetInterpreter'; import { SetInterpreterCommand } from './configuration/interpreterSelector/commands/setInterpreter'; import { SetShebangInterpreterCommand } from './configuration/interpreterSelector/commands/setShebangInterpreter'; @@ -40,6 +41,10 @@ import { VirtualEnvironmentPrompt } from './virtualEnvs/virtualEnvPrompt'; */ export function registerInterpreterTypes(serviceManager: IServiceManager): void { + serviceManager.addSingleton( + IExtensionSingleActivationService, + InstallPythonCommand, + ); serviceManager.addSingleton( IExtensionSingleActivationService, SetInterpreterCommand, diff --git a/src/test/configuration/interpreterSelector/commands/installPython.unit.test.ts b/src/test/configuration/interpreterSelector/commands/installPython.unit.test.ts new file mode 100644 index 000000000000..62178e44c0a0 --- /dev/null +++ b/src/test/configuration/interpreterSelector/commands/installPython.unit.test.ts @@ -0,0 +1,123 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { assert } from 'chai'; +import { SemVer } from 'semver'; +import * as sinon from 'sinon'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import * as TypeMoq from 'typemoq'; +import { ExtensionContextKey } from '../../../../client/common/application/contextKeys'; +import { ICommandManager, IContextKeyManager } from '../../../../client/common/application/types'; +import { PythonWelcome } from '../../../../client/common/application/walkThroughs'; +import { Commands, PVSC_EXTENSION_ID } from '../../../../client/common/constants'; +import { IPlatformService } from '../../../../client/common/platform/types'; +import { IBrowserService, IDisposable } from '../../../../client/common/types'; +import { InstallPythonCommand } from '../../../../client/interpreter/configuration/interpreterSelector/commands/installPython'; + +suite('Install Python Command', () => { + let cmdManager: ICommandManager; + let browserService: IBrowserService; + let contextKeyManager: IContextKeyManager; + let platformService: IPlatformService; + let installPythonCommand: InstallPythonCommand; + let walkthroughID: + | { + category: string; + step: string; + } + | undefined; + setup(() => { + walkthroughID = undefined; + cmdManager = mock(); + when(cmdManager.executeCommand('workbench.action.openWalkthrough', anything(), false)).thenCall((_, w) => { + walkthroughID = w; + }); + browserService = mock(); + when(browserService.launch(anything())).thenReturn(undefined); + contextKeyManager = mock(); + when(contextKeyManager.setContext(ExtensionContextKey.showInstallTile, true)).thenResolve(); + platformService = mock(); + installPythonCommand = new InstallPythonCommand( + instance(cmdManager), + instance(contextKeyManager), + instance(browserService), + instance(platformService), + [], + ); + }); + + teardown(() => { + sinon.restore(); + }); + + test('Ensure command is registered with the correct callback handler', async () => { + let installCommandHandler!: () => Promise; + when(cmdManager.registerCommand(Commands.InstallPython, anything())).thenCall((_, cb) => { + installCommandHandler = cb; + return TypeMoq.Mock.ofType().object; + }); + + await installPythonCommand.activate(); + + verify(cmdManager.registerCommand(Commands.InstallPython, anything())).once(); + + const installPython = sinon.stub(InstallPythonCommand.prototype, '_installPython'); + await installCommandHandler(); + assert(installPython.calledOnce); + }); + + test('Opens Linux Install tile on Linux', async () => { + when(platformService.isWindows).thenReturn(false); + when(platformService.isLinux).thenReturn(true); + when(platformService.isMac).thenReturn(false); + const expectedWalkthroughID = { + category: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}`, + step: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}#${PythonWelcome.linuxInstallId}`, + }; + await installPythonCommand._installPython(); + verify(contextKeyManager.setContext(ExtensionContextKey.showInstallTile, true)).once(); + verify(browserService.launch(anything())).never(); + assert.deepEqual(walkthroughID, expectedWalkthroughID); + }); + + test('Opens Mac Install tile on MacOS', async () => { + when(platformService.isWindows).thenReturn(false); + when(platformService.isLinux).thenReturn(false); + when(platformService.isMac).thenReturn(true); + const expectedWalkthroughID = { + category: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}`, + step: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}#${PythonWelcome.macOSInstallId}`, + }; + await installPythonCommand._installPython(); + verify(contextKeyManager.setContext(ExtensionContextKey.showInstallTile, true)).once(); + verify(browserService.launch(anything())).never(); + assert.deepEqual(walkthroughID, expectedWalkthroughID); + }); + + test('Opens Windows Install tile on Windows 8', async () => { + when(platformService.isWindows).thenReturn(true); + when(platformService.isLinux).thenReturn(false); + when(platformService.isMac).thenReturn(false); + when(platformService.getVersion()).thenResolve(new SemVer('8.2.0')); + const expectedWalkthroughID = { + category: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}`, + step: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}#${PythonWelcome.windowsInstallId}`, + }; + await installPythonCommand._installPython(); + verify(contextKeyManager.setContext(ExtensionContextKey.showInstallTile, true)).once(); + verify(browserService.launch(anything())).never(); + assert.deepEqual(walkthroughID, expectedWalkthroughID); + }); + + test('Opens windows store app on Windows otherwise', async () => { + when(platformService.isWindows).thenReturn(true); + when(platformService.isLinux).thenReturn(false); + when(platformService.isMac).thenReturn(false); + when(platformService.getVersion()).thenResolve(new SemVer('10.0.0')); + await installPythonCommand._installPython(); + verify(browserService.launch(anything())).once(); + verify(contextKeyManager.setContext(ExtensionContextKey.showInstallTile, true)).never(); + }); +}); diff --git a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts index bd08255324f9..d51e81365015 100644 --- a/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts @@ -42,7 +42,7 @@ import { EnvironmentType, PythonEnvironment } from '../../../../client/pythonEnv import { EventName } from '../../../../client/telemetry/constants'; import * as Telemetry from '../../../../client/telemetry'; import { MockWorkspaceConfiguration } from '../../../mocks/mockWorkspaceConfig'; -import { Octicons } from '../../../../client/common/constants'; +import { Commands, Octicons } from '../../../../client/common/constants'; import { IInterpreterService, PythonEnvironmentsChangedEvent } from '../../../../client/interpreter/contracts'; import { createDeferred, sleep } from '../../../../client/common/utils/async'; import { SystemVariables } from '../../../../client/common/variables/systemVariables'; @@ -129,6 +129,12 @@ suite('Set Interpreter Command', () => { alwaysShow: true, }; + const noPythonInstalled = { + label: `${Octicons.Error} ${InterpreterQuickPickList.noPythonInstalled}`, + detail: InterpreterQuickPickList.clickForInstructions, + alwaysShow: true, + }; + const refreshedItem: IInterpreterQuickPickItem = { description: interpreterPath, detail: '', @@ -256,6 +262,66 @@ suite('Set Interpreter Command', () => { assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal'); }); + test('Picker should be displayed with expected items if no interpreters are available', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + const suggestions = [ + expectedEnterInterpreterPathSuggestion, + defaultInterpreterPathSuggestion, + noPythonInstalled, + ]; + const expectedParameters: IQuickPickParameters = { + placeholder: `Selected Interpreter: ${currentPythonPath}`, + items: suggestions, // Verify suggestions + activeItem: noPythonInstalled, // Verify active item + matchOnDetail: true, + matchOnDescription: true, + title: InterpreterQuickPickList.browsePath.openButtonLabel, + sortByLabel: true, + keepScrollPosition: true, + }; + let actualParameters: IQuickPickParameters | undefined; + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + .callback((options) => { + actualParameters = options; + }) + .returns(() => Promise.resolve((undefined as unknown) as QuickPickItem)); + interpreterSelector.reset(); + interpreterSelector + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => []); + + await setInterpreterCommand._pickInterpreter(multiStepInput.object, state); + + expect(actualParameters).to.not.equal(undefined, 'Parameters not set'); + const refreshButtonCallback = actualParameters!.customButtonSetup?.callback; + expect(refreshButtonCallback).to.not.equal(undefined, 'Callback not set'); + delete actualParameters!.customButtonSetup; + delete actualParameters!.onChangeItem; + assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal'); + }); + + test('Picker should install python if corresponding item is selected', async () => { + const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; + const multiStepInput = TypeMoq.Mock.ofType>(); + multiStepInput + .setup((i) => i.showQuickPick(TypeMoq.It.isAny())) + .returns(() => Promise.resolve((noPythonInstalled as unknown) as QuickPickItem)); + interpreterSelector.reset(); + interpreterSelector + .setup((i) => i.getSuggestions(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => []); + commandManager + .setup((c) => c.executeCommand(Commands.InstallPython)) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.once()); + + await setInterpreterCommand._pickInterpreter(multiStepInput.object, state); + + commandManager.verifyAll(); + }); + test('Items displayed should be grouped if no refresh is going on', async () => { const state: InterpreterStateArgs = { path: 'some path', workspace: undefined }; const multiStepInput = TypeMoq.Mock.ofType>(); diff --git a/src/test/interpreters/serviceRegistry.unit.test.ts b/src/test/interpreters/serviceRegistry.unit.test.ts index 40664764fd4c..44e14c6c09e3 100644 --- a/src/test/interpreters/serviceRegistry.unit.test.ts +++ b/src/test/interpreters/serviceRegistry.unit.test.ts @@ -14,6 +14,7 @@ import { IInterpreterAutoSelectionProxyService, } from '../../client/interpreter/autoSelection/types'; import { EnvironmentTypeComparer } from '../../client/interpreter/configuration/environmentTypeComparer'; +import { InstallPythonCommand } from '../../client/interpreter/configuration/interpreterSelector/commands/installPython'; import { ResetInterpreterCommand } from '../../client/interpreter/configuration/interpreterSelector/commands/resetInterpreter'; import { SetInterpreterCommand } from '../../client/interpreter/configuration/interpreterSelector/commands/setInterpreter'; import { SetShebangInterpreterCommand } from '../../client/interpreter/configuration/interpreterSelector/commands/setShebangInterpreter'; @@ -48,6 +49,7 @@ suite('Interpreters - Service Registry', () => { registerTypes(instance(serviceManager)); [ + [IExtensionSingleActivationService, InstallPythonCommand], [IExtensionSingleActivationService, SetInterpreterCommand], [IExtensionSingleActivationService, ResetInterpreterCommand], [IExtensionSingleActivationService, SetShebangInterpreterCommand], From b4a7269b75db52739825124541b23dbc94813f28 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 28 Jun 2022 09:17:32 -0700 Subject: [PATCH 2/5] Code reviews --- .../configuration/interpreterSelector/commands/installPython.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts b/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts index 3b015138bda0..9c065957bf43 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts @@ -31,7 +31,7 @@ export class InstallPythonCommand implements IExtensionSingleActivationService { public async _installPython(): Promise { if (this.platformService.isWindows) { const version = await this.platformService.getVersion(); - if (version.major !== 8) { + if (version.major > 8) { // OS is not Windows 8, ms-windows-store URIs are available: // https://docs.microsoft.com/en-us/windows/uwp/launch-resume/launch-store-app this.browserService.launch('ms-windows-store://pdp/?ProductId=9PJPW5LDXLZ5'); From d687933f4003b384c088273d1174927c9af5f422 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 1 Jul 2022 11:24:13 -0700 Subject: [PATCH 3/5] Ensure create a python file automatically selects the language for the file as python --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 647efdddc07b..9366df312cfa 100644 --- a/package.json +++ b/package.json @@ -109,7 +109,7 @@ { "id": "python.createPythonFile", "title": "Create a Python file", - "description": "[Open](command:toSide:workbench.action.files.openFile) or [create](command:toSide:workbench.action.files.newUntitledFile) a Python file - make sure to save it as \".py\".\n[Create Python File](command:toSide:workbench.action.files.newUntitledFile)", + "description": "[Open](command:toSide:workbench.action.files.openFile) or [create](command:toSide:workbench.action.files.newUntitledFile?%7B%22languageId%22%3A%22python%22%7D) a Python file - make sure to save it as \".py\".\n[Create Python File](command:toSide:workbench.action.files.newUntitledFile?%7B%22languageId%22%3A%22python%22%7D)", "media": { "svg": "resources/walkthrough/open-folder.svg", "altText": "Open a Python file or a folder with a Python project." From bd9e3c34412b3a7a40e32908c4aa93c6c2506174 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 1 Jul 2022 13:57:38 -0700 Subject: [PATCH 4/5] Hide tiles by default for Mac and Linux --- package.json | 6 +++--- src/client/common/application/contextKeys.ts | 2 +- .../interpreterSelector/commands/installPython.ts | 6 +++--- .../commands/installPython.unit.test.ts | 10 +++++----- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index 9366df312cfa..9cba30c3ba91 100644 --- a/package.json +++ b/package.json @@ -123,7 +123,7 @@ "media": { "markdown": "resources/walkthrough/install-python-windows-8.md" }, - "when": "workspacePlatform == windows && showInstallTile" + "when": "workspacePlatform == windows && showInstallPythonTile" }, { "id": "python.installPythonMac", @@ -132,7 +132,7 @@ "media": { "markdown": "resources/walkthrough/install-python-macos.md" }, - "when": "workspacePlatform == mac", + "when": "workspacePlatform == mac && showInstallPythonTile", "command": "workbench.action.terminal.new" }, { @@ -142,7 +142,7 @@ "media": { "markdown": "resources/walkthrough/install-python-linux.md" }, - "when": "workspacePlatform == linux", + "when": "workspacePlatform == linux && showInstallPythonTile", "command": "workbench.action.terminal.new" }, { diff --git a/src/client/common/application/contextKeys.ts b/src/client/common/application/contextKeys.ts index 9e6fecea28d3..2f791ae66846 100644 --- a/src/client/common/application/contextKeys.ts +++ b/src/client/common/application/contextKeys.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. export enum ExtensionContextKey { - showInstallTile = 'showInstallTile', + showInstallPythonTile = 'showInstallPythonTile', HasFailedTests = 'hasFailedTests', RefreshingTests = 'refreshingTests', } diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts b/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts index 9c065957bf43..ba837afeb4bc 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/installPython.ts @@ -38,11 +38,11 @@ export class InstallPythonCommand implements IExtensionSingleActivationService { return; } } - this.showInstallTile(); + this.showInstallPythonTile(); } - private showInstallTile() { - this.contextManager.setContext(ExtensionContextKey.showInstallTile, true); + private showInstallPythonTile() { + this.contextManager.setContext(ExtensionContextKey.showInstallPythonTile, true); let step: string; if (this.platformService.isWindows) { step = PythonWelcome.windowsInstallId; diff --git a/src/test/configuration/interpreterSelector/commands/installPython.unit.test.ts b/src/test/configuration/interpreterSelector/commands/installPython.unit.test.ts index 62178e44c0a0..6efc94ef28b2 100644 --- a/src/test/configuration/interpreterSelector/commands/installPython.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/installPython.unit.test.ts @@ -37,7 +37,7 @@ suite('Install Python Command', () => { browserService = mock(); when(browserService.launch(anything())).thenReturn(undefined); contextKeyManager = mock(); - when(contextKeyManager.setContext(ExtensionContextKey.showInstallTile, true)).thenResolve(); + when(contextKeyManager.setContext(ExtensionContextKey.showInstallPythonTile, true)).thenResolve(); platformService = mock(); installPythonCommand = new InstallPythonCommand( instance(cmdManager), @@ -77,7 +77,7 @@ suite('Install Python Command', () => { step: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}#${PythonWelcome.linuxInstallId}`, }; await installPythonCommand._installPython(); - verify(contextKeyManager.setContext(ExtensionContextKey.showInstallTile, true)).once(); + verify(contextKeyManager.setContext(ExtensionContextKey.showInstallPythonTile, true)).once(); verify(browserService.launch(anything())).never(); assert.deepEqual(walkthroughID, expectedWalkthroughID); }); @@ -91,7 +91,7 @@ suite('Install Python Command', () => { step: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}#${PythonWelcome.macOSInstallId}`, }; await installPythonCommand._installPython(); - verify(contextKeyManager.setContext(ExtensionContextKey.showInstallTile, true)).once(); + verify(contextKeyManager.setContext(ExtensionContextKey.showInstallPythonTile, true)).once(); verify(browserService.launch(anything())).never(); assert.deepEqual(walkthroughID, expectedWalkthroughID); }); @@ -106,7 +106,7 @@ suite('Install Python Command', () => { step: `${PVSC_EXTENSION_ID}#${PythonWelcome.name}#${PythonWelcome.windowsInstallId}`, }; await installPythonCommand._installPython(); - verify(contextKeyManager.setContext(ExtensionContextKey.showInstallTile, true)).once(); + verify(contextKeyManager.setContext(ExtensionContextKey.showInstallPythonTile, true)).once(); verify(browserService.launch(anything())).never(); assert.deepEqual(walkthroughID, expectedWalkthroughID); }); @@ -118,6 +118,6 @@ suite('Install Python Command', () => { when(platformService.getVersion()).thenResolve(new SemVer('10.0.0')); await installPythonCommand._installPython(); verify(browserService.launch(anything())).once(); - verify(contextKeyManager.setContext(ExtensionContextKey.showInstallTile, true)).never(); + verify(contextKeyManager.setContext(ExtensionContextKey.showInstallPythonTile, true)).never(); }); }); From 3b3e2e598f19efe01a0ce36e3ca2081b75a7a086 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 5 Jul 2022 10:59:03 -0700 Subject: [PATCH 5/5] Remove built-in new python file command --- package.json | 14 ------ package.nls.json | 1 - src/client/common/application/commands.ts | 1 - .../application/commands/createFileCommand.ts | 29 ------------ src/client/common/constants.ts | 1 - src/client/common/serviceRegistry.ts | 5 --- src/client/telemetry/constants.ts | 1 - src/client/telemetry/index.ts | 7 --- .../createNewFileCommand.unit.test.ts | 44 ------------------- 9 files changed, 103 deletions(-) delete mode 100644 src/client/common/application/commands/createFileCommand.ts delete mode 100644 src/test/common/application/commands/createNewFileCommand.unit.test.ts diff --git a/package.json b/package.json index 9cba30c3ba91..cd7b72f8af7a 100644 --- a/package.json +++ b/package.json @@ -64,7 +64,6 @@ "onDebugInitialConfigurations", "onLanguage:python", "onDebugResolve:python", - "onCommand:python.createNewFile", "onCommand:python.execInTerminal", "onCommand:python.debugInTerminal", "onCommand:python.sortImports", @@ -248,12 +247,6 @@ } ], "commands": [ - { - "title": "%python.command.python.createNewFile.title%", - "shortTitle": "%python.menu.createNewFile.title%", - "category": "Python", - "command": "python.createNewFile" - }, { "category": "Python", "command": "python.analysis.restartLanguageServer", @@ -1710,13 +1703,6 @@ "when": "resourceLangId == python && !virtualWorkspace && shellExecutionSupported" } ], - "file/newFile": [ - { - "command": "python.createNewFile", - "group": "file", - "when": "!virtualWorkspace" - } - ], "view/title": [ { "command": "testing.reRunFailTests", diff --git a/package.nls.json b/package.nls.json index 552f7cb18f2b..ac2d2176ee0e 100644 --- a/package.nls.json +++ b/package.nls.json @@ -2,7 +2,6 @@ "python.command.python.sortImports.title": "Sort Imports", "python.command.python.startREPL.title": "Start REPL", "python.command.python.createTerminal.title": "Create Terminal", - "python.command.python.createNewFile.title": "New Python File", "python.command.python.execInTerminal.title": "Run Python File in Terminal", "python.command.python.debugInTerminal.title": "Debug Python File", "python.command.python.execInTerminalIcon.title": "Run Python File", diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 01fce2dbe6e4..58830bd8799e 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -41,7 +41,6 @@ interface ICommandNameWithoutArgumentTypeMapping { [Commands.PickLocalProcess]: []; [Commands.ClearStorage]: []; [Commands.ReportIssue]: []; - [Commands.CreateNewFile]: []; [Commands.RefreshTensorBoard]: []; [LSCommands.RestartLS]: []; } diff --git a/src/client/common/application/commands/createFileCommand.ts b/src/client/common/application/commands/createFileCommand.ts deleted file mode 100644 index b8c96f141f89..000000000000 --- a/src/client/common/application/commands/createFileCommand.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { injectable, inject } from 'inversify'; -import { IExtensionSingleActivationService } from '../../../activation/types'; -import { Commands } from '../../constants'; -import { IApplicationShell, ICommandManager, IWorkspaceService } from '../types'; -import { sendTelemetryEvent } from '../../../telemetry'; -import { EventName } from '../../../telemetry/constants'; -import { IDisposableRegistry } from '../../types'; - -@injectable() -export class CreatePythonFileCommandHandler implements IExtensionSingleActivationService { - public readonly supportedWorkspaceTypes = { untrustedWorkspace: true, virtualWorkspace: true }; - - constructor( - @inject(ICommandManager) private readonly commandManager: ICommandManager, - @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, - @inject(IApplicationShell) private readonly appShell: IApplicationShell, - @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, - ) {} - - public async activate(): Promise { - this.disposables.push(this.commandManager.registerCommand(Commands.CreateNewFile, this.createPythonFile, this)); - } - - public async createPythonFile(): Promise { - const newFile = await this.workspaceService.openTextDocument({ language: 'python' }); - this.appShell.showTextDocument(newFile); - sendTelemetryEvent(EventName.CREATE_NEW_FILE_COMMAND); - } -} diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index b2fb4b9a200d..1c1064b77921 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -45,7 +45,6 @@ export namespace Commands { export const ViewOutput = 'python.viewOutput'; export const Start_REPL = 'python.startREPL'; export const Create_Terminal = 'python.createTerminal'; - export const CreateNewFile = 'python.createNewFile'; export const Set_Linter = 'python.setLinter'; export const Enable_Linter = 'python.enableLinting'; export const Run_Linter = 'python.runLinting'; diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index 27d65366af29..58ad92d58fe0 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -30,7 +30,6 @@ import { ClipboardService } from './application/clipboard'; import { CommandManager } from './application/commandManager'; import { ReloadVSCodeCommandHandler } from './application/commands/reloadCommand'; import { ReportIssueCommandHandler } from './application/commands/reportIssueCommand'; -import { CreatePythonFileCommandHandler } from './application/commands/createFileCommand'; import { DebugService } from './application/debugService'; import { DebugSessionTelemetry } from './application/debugSessionTelemetry'; import { DocumentManager } from './application/documentManager'; @@ -177,10 +176,6 @@ export function registerTypes(serviceManager: IServiceManager): void { IExtensionSingleActivationService, ReportIssueCommandHandler, ); - serviceManager.addSingleton( - IExtensionSingleActivationService, - CreatePythonFileCommandHandler, - ); serviceManager.addSingleton( IExtensionSingleActivationService, DebugSessionTelemetry, diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 59f4ae5b419c..2ab6c8a8a3ba 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -77,7 +77,6 @@ export enum EventName { SELECT_LINTER = 'LINTING.SELECT', USE_REPORT_ISSUE_COMMAND = 'USE_REPORT_ISSUE_COMMAND', - CREATE_NEW_FILE_COMMAND = 'CREATE_NEW_FILE_COMMAND', LINTER_NOT_INSTALLED_PROMPT = 'LINTER_NOT_INSTALLED_PROMPT', HASHED_PACKAGE_NAME = 'HASHED_PACKAGE_NAME', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 9d0cec7f569c..dafaca8cfefe 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1352,13 +1352,6 @@ export interface IEventNamePropertyMapping { "use_report_issue_command" : { "owner": "paulacamargo25" } */ [EventName.USE_REPORT_ISSUE_COMMAND]: unknown; - /** - * Telemetry event sent when the New Python File command is executed. - */ - /* __GDPR__ - "create_new_file_command" : { "owner": "luabud" } - */ - [EventName.CREATE_NEW_FILE_COMMAND]: unknown; /** * Telemetry event sent when the installed versions of Python, Jupyter, and Pylance are all capable * of supporting the LSP notebooks experiment. This does not indicate that the experiment is enabled. diff --git a/src/test/common/application/commands/createNewFileCommand.unit.test.ts b/src/test/common/application/commands/createNewFileCommand.unit.test.ts deleted file mode 100644 index 1372b7662dcf..000000000000 --- a/src/test/common/application/commands/createNewFileCommand.unit.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; -import { TextDocument } from 'vscode'; -import { Commands } from '../../../../client/common/constants'; -import { CommandManager } from '../../../../client/common/application/commandManager'; -import { CreatePythonFileCommandHandler } from '../../../../client/common/application/commands/createFileCommand'; -import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../client/common/application/types'; -import { WorkspaceService } from '../../../../client/common/application/workspace'; -import { ApplicationShell } from '../../../../client/common/application/applicationShell'; - -suite('Create New Python File Commmand', () => { - let createNewFileCommandHandler: CreatePythonFileCommandHandler; - let cmdManager: ICommandManager; - let workspaceService: IWorkspaceService; - let appShell: IApplicationShell; - - setup(async () => { - cmdManager = mock(CommandManager); - workspaceService = mock(WorkspaceService); - appShell = mock(ApplicationShell); - - createNewFileCommandHandler = new CreatePythonFileCommandHandler( - instance(cmdManager), - instance(workspaceService), - instance(appShell), - [], - ); - when(workspaceService.openTextDocument(deepEqual({ language: 'python' }))).thenReturn( - Promise.resolve(({} as unknown) as TextDocument), - ); - await createNewFileCommandHandler.activate(); - }); - - test('Create Python file command is registered', async () => { - verify(cmdManager.registerCommand(Commands.CreateNewFile, anything(), anything())).once(); - }); - test('Create a Python file if command is executed', async () => { - await createNewFileCommandHandler.createPythonFile(); - verify(workspaceService.openTextDocument(deepEqual({ language: 'python' }))).once(); - verify(appShell.showTextDocument(anything())).once(); - }); -});