Skip to content

Commit d44386e

Browse files
author
Mikhail Arkhipov
authored
804 + Improve 'no installers available' messaging (#823)
* Fix pylint search * Handle quote escapes in strings * Escapes in strings * CR feedback * Missing pip * Test * Tests * Tests * Mac python path * Tests * Tests * Test * "Go To Python object" doesn't work * Proper detection and type population in virtual env * Test fixes * Discover pylintrc better + tests * Undo change * CR feedback * Set interprereter before checking install
1 parent 3fe37a2 commit d44386e

23 files changed

+442
-126
lines changed

‎src/client/common/installer/channelManager.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { inject, injectable } from 'inversify';
5+
import { QuickPickItem, Uri } from 'vscode';
6+
import { IInterpreterService, InterpreterType } from '../../interpreter/contracts';
7+
import { IServiceContainer } from '../../ioc/types';
8+
import { IApplicationShell } from '../application/types';
9+
import { IPlatformService } from '../platform/types';
10+
import { Product } from '../types';
11+
import { ProductNames } from './productNames';
12+
import { IInstallationChannelManager, IModuleInstaller } from './types';
13+
14+
@injectable()
15+
export class InstallationChannelManager implements IInstallationChannelManager {
16+
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { }
17+
18+
public async getInstallationChannel(product: Product, resource?: Uri): Promise<IModuleInstaller | undefined> {
19+
const channels = await this.getInstallationChannels(resource);
20+
if (channels.length === 1) {
21+
return channels[0];
22+
}
23+
24+
const productName = ProductNames.get(product)!;
25+
const appShell = this.serviceContainer.get<IApplicationShell>(IApplicationShell);
26+
if (channels.length === 0) {
27+
await this.showNoInstallersMessage(resource);
28+
return;
29+
}
30+
31+
const placeHolder = `Select an option to install ${productName}`;
32+
const options = channels.map(installer => {
33+
return {
34+
label: `Install using ${installer.displayName}`,
35+
description: '',
36+
installer
37+
} as QuickPickItem & { installer: IModuleInstaller };
38+
});
39+
const selection = await appShell.showQuickPick(options, { matchOnDescription: true, matchOnDetail: true, placeHolder });
40+
return selection ? selection.installer : undefined;
41+
}
42+
43+
public async getInstallationChannels(resource?: Uri): Promise<IModuleInstaller[]> {
44+
const installers = this.serviceContainer.getAll<IModuleInstaller>(IModuleInstaller);
45+
const supportedInstallers: IModuleInstaller[] = [];
46+
for (const mi of installers) {
47+
if (await mi.isSupported(resource)) {
48+
supportedInstallers.push(mi);
49+
}
50+
}
51+
return supportedInstallers;
52+
}
53+
54+
public async showNoInstallersMessage(resource?: Uri): Promise<void> {
55+
const interpreters = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
56+
const interpreter = await interpreters.getActiveInterpreter(resource);
57+
if (!interpreter) {
58+
return; // Handled in the Python installation check.
59+
}
60+
61+
const appShell = this.serviceContainer.get<IApplicationShell>(IApplicationShell);
62+
const search = 'Search for help';
63+
let result: string | undefined;
64+
if (interpreter.type === InterpreterType.Conda) {
65+
result = await appShell.showErrorMessage('There is no Conda or Pip installer available in the selected environment.', search);
66+
} else {
67+
result = await appShell.showErrorMessage('There is no Pip installer available in the selected environment.', search);
68+
}
69+
if (result === search) {
70+
const platform = this.serviceContainer.get<IPlatformService>(IPlatformService);
71+
const osName = platform.isWindows
72+
? 'Windows'
73+
: (platform.isMac ? 'MacOS' : 'Linux');
74+
appShell.openUrl(`https://www.bing.com/search?q=Install Pip ${osName} ${(interpreter.type === InterpreterType.Conda) ? 'Conda' : ''}`);
75+
}
76+
}
77+
}

‎src/client/common/installer/condaInstaller.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import { inject, injectable } from 'inversify';
55
import { Uri } from 'vscode';
6-
import { ICondaService, IInterpreterService, InterpreterType } from '../../interpreter/contracts';
6+
import { ICondaService } from '../../interpreter/contracts';
77
import { IServiceContainer } from '../../ioc/types';
88
import { ExecutionInfo, IConfigurationService } from '../types';
99
import { ModuleInstaller } from './moduleInstaller';
@@ -15,7 +15,7 @@ export class CondaInstaller extends ModuleInstaller implements IModuleInstaller
1515
public get displayName() {
1616
return 'Conda';
1717
}
18-
constructor( @inject(IServiceContainer) serviceContainer: IServiceContainer) {
18+
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
1919
super(serviceContainer);
2020
}
2121
/**
@@ -27,16 +27,14 @@ export class CondaInstaller extends ModuleInstaller implements IModuleInstaller
2727
* @returns {Promise<boolean>} Whether conda is supported as a module installer or not.
2828
*/
2929
public async isSupported(resource?: Uri): Promise<boolean> {
30-
if (typeof this.isCondaAvailable === 'boolean') {
30+
if (this.isCondaAvailable !== undefined) {
3131
return this.isCondaAvailable!;
3232
}
3333
const condaLocator = this.serviceContainer.get<ICondaService>(ICondaService);
34-
const available = await condaLocator.isCondaAvailable();
35-
36-
if (!available) {
34+
this.isCondaAvailable = await condaLocator.isCondaAvailable();
35+
if (!this.isCondaAvailable) {
3736
return false;
3837
}
39-
4038
// Now we need to check if the current environment is a conda environment or not.
4139
return this.isCurrentEnvironmentACondaEnvironment(resource);
4240
}
@@ -48,11 +46,11 @@ export class CondaInstaller extends ModuleInstaller implements IModuleInstaller
4846
const info = await condaService.getCondaEnvironment(pythonPath);
4947
const args = ['install'];
5048

51-
if (info.name) {
49+
if (info && info.name) {
5250
// If we have the name of the conda environment, then use that.
5351
args.push('--name');
5452
args.push(info.name!);
55-
} else if (info.path) {
53+
} else if (info && info.path) {
5654
// Else provide the full path to the environment path.
5755
args.push('--prefix');
5856
args.push(info.path);

‎src/client/common/installer/installer.ts

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { IPlatformService } from '../platform/types';
1313
import { IProcessService, IPythonExecutionFactory } from '../process/types';
1414
import { ITerminalServiceFactory } from '../terminal/types';
1515
import { IInstaller, ILogger, InstallerResponse, IOutputChannel, ModuleNamePurpose, Product } from '../types';
16-
import { IModuleInstaller } from './types';
16+
import { IInstallationChannelManager, IModuleInstaller } from './types';
1717

1818
export { Product } from '../types';
1919

@@ -71,7 +71,7 @@ ProductTypes.set(Product.rope, ProductType.RefactoringLibrary);
7171

7272
@injectable()
7373
export class Installer implements IInstaller {
74-
constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer,
74+
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer,
7575
@inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private outputChannel: vscode.OutputChannel) {
7676
}
7777
// tslint:disable-next-line:no-empty
@@ -135,7 +135,9 @@ export class Installer implements IInstaller {
135135
if (product === Product.ctags) {
136136
return this.installCTags();
137137
}
138-
const installer = await this.getInstallationChannel(product, resource);
138+
139+
const channels = this.serviceContainer.get<IInstallationChannelManager>(IInstallationChannelManager);
140+
const installer = await channels.getInstallationChannel(product, resource);
139141
if (!installer) {
140142
return InstallerResponse.Ignore;
141143
}
@@ -191,32 +193,7 @@ export class Installer implements IInstaller {
191193
}
192194
return InstallerResponse.Ignore;
193195
}
194-
private async getInstallationChannel(product: Product, resource?: Uri): Promise<IModuleInstaller | undefined> {
195-
const productName = ProductNames.get(product)!;
196-
const channels = await this.getInstallationChannels(resource);
197-
if (channels.length === 0) {
198-
window.showInformationMessage(`No installers available to install ${productName}.`);
199-
return;
200-
}
201-
if (channels.length === 1) {
202-
return channels[0];
203-
}
204-
const placeHolder = `Select an option to install ${productName}`;
205-
const options = channels.map(installer => {
206-
return {
207-
label: `Install using ${installer.displayName}`,
208-
description: '',
209-
installer
210-
} as QuickPickItem & { installer: IModuleInstaller };
211-
});
212-
const selection = await window.showQuickPick(options, { matchOnDescription: true, matchOnDetail: true, placeHolder });
213-
return selection ? selection.installer : undefined;
214-
}
215-
private async getInstallationChannels(resource?: Uri): Promise<IModuleInstaller[]> {
216-
const installers = this.serviceContainer.getAll<IModuleInstaller>(IModuleInstaller);
217-
const supportedInstallers = await Promise.all(installers.map(async installer => installer.isSupported(resource).then(supported => supported ? installer : undefined)));
218-
return supportedInstallers.filter(installer => installer !== undefined).map(installer => installer!);
219-
}
196+
220197
// tslint:disable-next-line:no-any
221198
private updateSetting(setting: string, value: any, resource?: Uri) {
222199
if (resource && workspace.getWorkspaceFolder(resource)) {

‎src/client/common/installer/productInstaller.ts

Lines changed: 5 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -13,27 +13,13 @@ import { IPlatformService } from '../platform/types';
1313
import { IProcessService, IPythonExecutionFactory } from '../process/types';
1414
import { ITerminalServiceFactory } from '../terminal/types';
1515
import { IConfigurationService, IInstaller, ILogger, InstallerResponse, IOutputChannel, ModuleNamePurpose, Product } from '../types';
16-
import { IModuleInstaller } from './types';
16+
import { ProductNames } from './productNames';
17+
import { IInstallationChannelManager, IModuleInstaller } from './types';
1718

1819
export { Product } from '../types';
1920

2021
const CTagsInsllationScript = os.platform() === 'darwin' ? 'brew install ctags' : 'sudo apt-get install exuberant-ctags';
2122

22-
// tslint:disable-next-line:variable-name
23-
const ProductNames = new Map<Product, string>();
24-
ProductNames.set(Product.autopep8, 'autopep8');
25-
ProductNames.set(Product.flake8, 'flake8');
26-
ProductNames.set(Product.mypy, 'mypy');
27-
ProductNames.set(Product.nosetest, 'nosetest');
28-
ProductNames.set(Product.pep8, 'pep8');
29-
ProductNames.set(Product.pylama, 'pylama');
30-
ProductNames.set(Product.prospector, 'prospector');
31-
ProductNames.set(Product.pydocstyle, 'pydocstyle');
32-
ProductNames.set(Product.pylint, 'pylint');
33-
ProductNames.set(Product.pytest, 'pytest');
34-
ProductNames.set(Product.yapf, 'yapf');
35-
ProductNames.set(Product.rope, 'rope');
36-
3723
enum ProductType {
3824
Linter,
3925
Formatter,
@@ -59,7 +45,8 @@ abstract class BaseInstaller {
5945
return InstallerResponse.Installed;
6046
}
6147

62-
const installer = await this.getInstallationChannel(product, resource);
48+
const channels = this.serviceContainer.get<IInstallationChannelManager>(IInstallationChannelManager);
49+
const installer = await channels.getInstallationChannel(product, resource);
6350
if (!installer) {
6451
return InstallerResponse.Ignore;
6552
}
@@ -100,34 +87,6 @@ abstract class BaseInstaller {
10087
protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
10188
throw new Error('getExecutableNameFromSettings is not supported on this object');
10289
}
103-
104-
private async getInstallationChannel(product: Product, resource?: Uri): Promise<IModuleInstaller | undefined> {
105-
const productName = ProductNames.get(product)!;
106-
const channels = await this.getInstallationChannels(resource);
107-
if (channels.length === 0) {
108-
window.showInformationMessage(`No installers available to install ${productName}.`);
109-
return;
110-
}
111-
if (channels.length === 1) {
112-
return channels[0];
113-
}
114-
const placeHolder = `Select an option to install ${productName}`;
115-
const options = channels.map(installer => {
116-
return {
117-
label: `Install using ${installer.displayName}`,
118-
description: '',
119-
installer
120-
} as QuickPickItem & { installer: IModuleInstaller };
121-
});
122-
const selection = await window.showQuickPick(options, { matchOnDescription: true, matchOnDetail: true, placeHolder });
123-
return selection ? selection.installer : undefined;
124-
}
125-
126-
private async getInstallationChannels(resource?: Uri): Promise<IModuleInstaller[]> {
127-
const installers = this.serviceContainer.getAll<IModuleInstaller>(IModuleInstaller);
128-
const supportedInstallers = await Promise.all(installers.map(async installer => installer.isSupported(resource).then(supported => supported ? installer : undefined)));
129-
return supportedInstallers.filter(installer => installer !== undefined).map(installer => installer!);
130-
}
13190
}
13291

13392
class CTagsInstaller extends BaseInstaller {
@@ -253,7 +212,7 @@ class RefactoringLibraryInstaller extends BaseInstaller {
253212
export class ProductInstaller implements IInstaller {
254213
private ProductTypes = new Map<Product, ProductType>();
255214

256-
constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer,
215+
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer,
257216
@inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private outputChannel: vscode.OutputChannel) {
258217
this.ProductTypes.set(Product.flake8, ProductType.Linter);
259218
this.ProductTypes.set(Product.mypy, ProductType.Linter);

‎src/client/common/installer/productNames.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { Product } from '../types';
5+
6+
// tslint:disable-next-line:variable-name
7+
export const ProductNames = new Map<Product, string>();
8+
ProductNames.set(Product.autopep8, 'autopep8');
9+
ProductNames.set(Product.flake8, 'flake8');
10+
ProductNames.set(Product.mypy, 'mypy');
11+
ProductNames.set(Product.nosetest, 'nosetest');
12+
ProductNames.set(Product.pep8, 'pep8');
13+
ProductNames.set(Product.pylama, 'pylama');
14+
ProductNames.set(Product.prospector, 'prospector');
15+
ProductNames.set(Product.pydocstyle, 'pydocstyle');
16+
ProductNames.set(Product.pylint, 'pylint');
17+
ProductNames.set(Product.pytest, 'pytest');
18+
ProductNames.set(Product.yapf, 'yapf');
19+
ProductNames.set(Product.rope, 'rope');

‎src/client/common/installer/pythonInstallation.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
// Licensed under the MIT License.
33
'use strict';
44

5-
import { IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts';
5+
import { IInterpreterLocatorService, IInterpreterService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts';
6+
import { isMacDefaultPythonPath } from '../../interpreter/locators/helpers';
67
import { IServiceContainer } from '../../ioc/types';
78
import { IApplicationShell } from '../application/types';
89
import { IPlatformService } from '../platform/types';
@@ -15,7 +16,7 @@ export class PythonInstaller {
1516
constructor(private serviceContainer: IServiceContainer) {
1617
this.locator = serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE);
1718
this.shell = serviceContainer.get<IApplicationShell>(IApplicationShell);
18-
}
19+
}
1920

2021
public async checkPythonInstallation(settings: IPythonSettings): Promise<boolean> {
2122
if (settings.disableInstallationChecks === true) {
@@ -24,10 +25,12 @@ export class PythonInstaller {
2425
const interpreters = await this.locator.getInterpreters();
2526
if (interpreters.length > 0) {
2627
const platform = this.serviceContainer.get<IPlatformService>(IPlatformService);
27-
if (platform.isMac &&
28-
settings.pythonPath === 'python' &&
29-
interpreters[0].type === InterpreterType.Unknown) {
30-
await this.shell.showWarningMessage('Selected interpreter is macOS system Python which is not recommended. Please select different interpreter');
28+
if (platform.isMac && isMacDefaultPythonPath(settings.pythonPath)) {
29+
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
30+
const interpreter = await interpreterService.getActiveInterpreter();
31+
if (interpreter && interpreter.type === InterpreterType.Unknown) {
32+
await this.shell.showWarningMessage('Selected interpreter is macOS system Python which is not recommended. Please select different interpreter');
33+
}
3134
}
3235
return true;
3336
}

‎src/client/common/installer/serviceRegistry.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
'use strict';
44

55
import { IServiceManager } from '../../ioc/types';
6+
import { InstallationChannelManager } from './channelManager';
67
import { CondaInstaller } from './condaInstaller';
78
import { PipInstaller } from './pipInstaller';
8-
import { IModuleInstaller } from './types';
9+
import { IInstallationChannelManager, IModuleInstaller } from './types';
910

1011
export function registerTypes(serviceManager: IServiceManager) {
1112
serviceManager.addSingleton<IModuleInstaller>(IModuleInstaller, CondaInstaller);
1213
serviceManager.addSingleton<IModuleInstaller>(IModuleInstaller, PipInstaller);
14+
serviceManager.addSingleton<IInstallationChannelManager>(IInstallationChannelManager, InstallationChannelManager);
1315
}

‎src/client/common/installer/types.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
import { Uri } from 'vscode';
5+
import { Product } from '../types';
56

67
export const IModuleInstaller = Symbol('IModuleInstaller');
78
export interface IModuleInstaller {
@@ -14,3 +15,10 @@ export const IPythonInstallation = Symbol('IPythonInstallation');
1415
export interface IPythonInstallation {
1516
checkInstallation(): Promise<boolean>;
1617
}
18+
19+
export const IInstallationChannelManager = Symbol('IInstallationChannelManager');
20+
export interface IInstallationChannelManager {
21+
getInstallationChannel(product: Product, resource?: Uri): Promise<IModuleInstaller | undefined>;
22+
getInstallationChannels(resource?: Uri): Promise<IModuleInstaller[]>;
23+
showNoInstallersMessage(): void;
24+
}

‎src/client/extension.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,14 +103,14 @@ export async function activate(context: vscode.ExtensionContext) {
103103
sortImports.activate(context, standardOutputChannel, serviceContainer);
104104
const interpreterManager = serviceContainer.get<IInterpreterService>(IInterpreterService);
105105

106+
// This must be completed before we can continue.
107+
interpreterManager.initialize();
108+
await interpreterManager.autoSetInterpreter();
109+
106110
const pythonInstaller = new PythonInstaller(serviceContainer);
107111
pythonInstaller.checkPythonInstallation(PythonSettings.getInstance())
108112
.catch(ex => console.error('Python Extension: pythonInstaller.checkPythonInstallation', ex));
109113

110-
// This must be completed before we can continue.
111-
await interpreterManager.autoSetInterpreter();
112-
113-
interpreterManager.initialize();
114114
interpreterManager.refresh()
115115
.catch(ex => console.error('Python Extension: interpreterManager.refresh', ex));
116116

0 commit comments

Comments
 (0)