Skip to content

Commit

Permalink
Improve getting started experience when starting on a fresh macOS (#2…
Browse files Browse the repository at this point in the history
…0789)

Closes #20635

- Suggest to install from `python.org` if brew is not available
- Do not suggest irrelevant prompts
  • Loading branch information
Kartik Raj authored Mar 2, 2023
1 parent 16c0437 commit ee8e80e
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 22 deletions.
6 changes: 5 additions & 1 deletion src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,13 @@ export class InstallPythonViaTerminal implements IExtensionSingleActivationServi

public async _installPythonOnUnix(os: OSType.Linux | OSType.OSX): Promise<void> {
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();
Expand All @@ -69,31 +73,36 @@ 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];
}
}
return [];
}
}

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>;
when(cmdManager.registerCommand(Commands.InstallPythonOnMac, anything())).thenCall((_, cb) => {
installCommandHandler = cb;
return TypeMoq.Mock.ofType<IDisposable>().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();

Expand All @@ -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<void>;
when(cmdManager.registerCommand(Commands.InstallPythonOnMac, anything())).thenCall((_, cb) => {
installCommandHandler = cb;
return TypeMoq.Mock.ofType<IDisposable>().object;
});
rewiremock('which').with((_cmd: string) => {
throw new Error('Command not found');
});

await installPythonCommand.activate();

await installCommandHandler!();

expect(message).to.be.equal(Interpreters.installPythonTerminalMacMessage);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down

0 comments on commit ee8e80e

Please sign in to comment.