From ee8e80e0f808cab044cdc40bd99e9f2aa7d3b66a Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 2 Mar 2023 14:43:05 -0800 Subject: [PATCH] Improve getting started experience when starting on a fresh macOS (#20789) Closes https://github.com/microsoft/vscode-python/issues/20635 - Suggest to install from `python.org` if brew is not available - Do not suggest irrelevant prompts --- src/client/common/utils/localize.ts | 6 ++- .../installPython/installPythonViaTerminal.ts | 41 +++++++++++-------- .../locators/lowLevel/macDefaultLocator.ts | 2 +- .../installPythonViaTerminal.unit.test.ts | 28 ++++++++++++- .../lowLevel/macDefaultLocator.unit.test.ts | 2 - 5 files changed, 57 insertions(+), 22 deletions(-) diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 60cbfd295f88..5037494463f2 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -204,9 +204,13 @@ export namespace Interpreters { export const selectInterpreterTip = l10n.t( 'Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar', ); - export const installPythonTerminalMessage = l10n.t( + export const installPythonTerminalMessageLinux = l10n.t( '💡 Please try installing the python package using your package manager. Alternatively you can also download it from https://www.python.org/downloads', ); + + export const installPythonTerminalMacMessage = l10n.t( + '💡 Brew does not seem to be available. Please try to download Python from https://www.python.org/downloads. Alternatively, you can install the python package using some other package manager which is available.', + ); export const changePythonInterpreter = l10n.t('Change Python Interpreter'); export const selectedPythonInterpreter = l10n.t('Selected Python Interpreter'); } diff --git a/src/client/interpreter/configuration/interpreterSelector/commands/installPython/installPythonViaTerminal.ts b/src/client/interpreter/configuration/interpreterSelector/commands/installPython/installPythonViaTerminal.ts index 7587c997d6c5..9da7284a3bea 100644 --- a/src/client/interpreter/configuration/interpreterSelector/commands/installPython/installPythonViaTerminal.ts +++ b/src/client/interpreter/configuration/interpreterSelector/commands/installPython/installPythonViaTerminal.ts @@ -55,9 +55,13 @@ export class InstallPythonViaTerminal implements IExtensionSingleActivationServi public async _installPythonOnUnix(os: OSType.Linux | OSType.OSX): Promise { const commands = await this.getCommands(os); + const installMessage = + os === OSType.OSX + ? Interpreters.installPythonTerminalMacMessage + : Interpreters.installPythonTerminalMessageLinux; const terminal = this.terminalManager.createTerminal({ name: 'Python', - message: commands.length ? undefined : Interpreters.installPythonTerminalMessage, + message: commands.length ? undefined : installMessage, }); terminal.show(true); await waitForTerminalToStartup(); @@ -69,24 +73,17 @@ export class InstallPythonViaTerminal implements IExtensionSingleActivationServi private async getCommands(os: OSType.Linux | OSType.OSX) { if (os === OSType.OSX) { - return this.packageManagerCommands[PackageManagers.brew]; + return this.getCommandsForPackageManagers([PackageManagers.brew]); } - return this.getCommandsForLinux(); + if (os === OSType.Linux) { + return this.getCommandsForPackageManagers([PackageManagers.apt, PackageManagers.dnf]); + } + throw new Error('OS not supported'); } - private async getCommandsForLinux() { - for (const packageManager of [PackageManagers.apt, PackageManagers.dnf]) { - let isPackageAvailable = false; - try { - const which = require('which') as typeof whichTypes; - const resolvedPath = await which(packageManager); - traceVerbose(`Resolved path to ${packageManager} module:`, resolvedPath); - isPackageAvailable = resolvedPath.trim().length > 0; - } catch (ex) { - traceVerbose(`${packageManager} not found`, ex); - isPackageAvailable = false; - } - if (isPackageAvailable) { + private async getCommandsForPackageManagers(packageManagers: PackageManagers[]) { + for (const packageManager of packageManagers) { + if (await isPackageAvailable(packageManager)) { return this.packageManagerCommands[packageManager]; } } @@ -94,6 +91,18 @@ export class InstallPythonViaTerminal implements IExtensionSingleActivationServi } } +async function isPackageAvailable(packageManager: PackageManagers) { + try { + const which = require('which') as typeof whichTypes; + const resolvedPath = await which(packageManager); + traceVerbose(`Resolved path to ${packageManager} module:`, resolvedPath); + return resolvedPath.trim().length > 0; + } catch (ex) { + traceVerbose(`${packageManager} not found`, ex); + return false; + } +} + async function waitForTerminalToStartup() { // Sometimes the terminal takes some time to start up before it can start accepting input. await sleep(100); diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.ts index 9baf6d2affd3..abb8652b23a1 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.ts @@ -13,7 +13,7 @@ export function isMacDefaultPythonPath(pythonPath: string): boolean { return false; } - const defaultPaths = ['python', '/usr/bin/python']; + const defaultPaths = ['/usr/bin/python']; return defaultPaths.includes(pythonPath) || pythonPath.startsWith('/usr/bin/python2'); } diff --git a/src/test/configuration/interpreterSelector/commands/installPythonViaTerminal.unit.test.ts b/src/test/configuration/interpreterSelector/commands/installPythonViaTerminal.unit.test.ts index 91f73cb0a305..16014290c218 100644 --- a/src/test/configuration/interpreterSelector/commands/installPythonViaTerminal.unit.test.ts +++ b/src/test/configuration/interpreterSelector/commands/installPythonViaTerminal.unit.test.ts @@ -96,15 +96,22 @@ suite('Install Python via Terminal', () => { await installPythonCommand.activate(); await installCommandHandler!(); - expect(message).to.be.equal(Interpreters.installPythonTerminalMessage); + expect(message).to.be.equal(Interpreters.installPythonTerminalMessageLinux); }); - test('Sends expected commands on Mac when InstallPythonOnMac command is executed if no dnf is available', async () => { + test('Sends expected commands on Mac when InstallPythonOnMac command is executed if brew is available', async () => { let installCommandHandler: () => Promise; when(cmdManager.registerCommand(Commands.InstallPythonOnMac, anything())).thenCall((_, cb) => { installCommandHandler = cb; return TypeMoq.Mock.ofType().object; }); + rewiremock('which').with((cmd: string) => { + if (cmd === 'brew') { + return 'path/to/brew'; + } + throw new Error('Command not found'); + }); + await installPythonCommand.activate(); when(terminalService.sendText('brew install python3')).thenResolve(); @@ -113,4 +120,21 @@ suite('Install Python via Terminal', () => { verify(terminalService.sendText('brew install python3')).once(); expect(message).to.be.equal(undefined); }); + + test('Creates terminal with appropriate message when InstallPythonOnMac command is executed if brew is not available', async () => { + let installCommandHandler: () => Promise; + when(cmdManager.registerCommand(Commands.InstallPythonOnMac, anything())).thenCall((_, cb) => { + installCommandHandler = cb; + return TypeMoq.Mock.ofType().object; + }); + rewiremock('which').with((_cmd: string) => { + throw new Error('Command not found'); + }); + + await installPythonCommand.activate(); + + await installCommandHandler!(); + + expect(message).to.be.equal(Interpreters.installPythonTerminalMacMessage); + }); }); diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.unit.test.ts index 3847869f5a2b..56012e11e4c5 100644 --- a/src/test/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.unit.test.ts @@ -18,8 +18,6 @@ suite('isMacDefaultPythonPath', () => { }); const testCases: { path: string; os: osUtils.OSType; expected: boolean }[] = [ - { path: 'python', os: osUtils.OSType.OSX, expected: true }, - { path: 'python', os: osUtils.OSType.Windows, expected: false }, { path: '/usr/bin/python', os: osUtils.OSType.OSX, expected: true }, { path: '/usr/bin/python', os: osUtils.OSType.Linux, expected: false }, { path: '/usr/bin/python2', os: osUtils.OSType.OSX, expected: true },