Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure resource is passed into isInstalled method #1895

Merged
merged 2 commits into from
Jun 7, 2018
Merged

Ensure resource is passed into isInstalled method #1895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ac9796
Ensure resource is passed into isInstalled method
DonJayamanne Jun 7, 2018
79d3da8
:memo: add news entry
DonJayamanne Jun 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/3 Code Health/1893.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure workspace information is passed into installer when determining whether a product/tool is installed.
48 changes: 24 additions & 24 deletions src/client/common/installer/productInstaller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { inject, injectable, named } from 'inversify';
import * as os from 'os';
import * as path from 'path';
import * as vscode from 'vscode';
import { OutputChannel, Uri } from 'vscode';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

import '../../common/extensions';
import { IFormatterHelper } from '../../formatters/types';
import { IServiceContainer } from '../../ioc/types';
Expand Down Expand Up @@ -35,13 +35,13 @@ export abstract class BaseInstaller {
protected configService: IConfigurationService;
private readonly workspaceService: IWorkspaceService;

constructor(protected serviceContainer: IServiceContainer, protected outputChannel: vscode.OutputChannel) {
constructor(protected serviceContainer: IServiceContainer, protected outputChannel: OutputChannel) {
this.appShell = serviceContainer.get<IApplicationShell>(IApplicationShell);
this.configService = serviceContainer.get<IConfigurationService>(IConfigurationService);
this.workspaceService = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
}

public promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
public promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse> {
// If this method gets called twice, while previous promise has not been resolved, then return that same promise.
// E.g. previous promise is not resolved as a message has been displayed to the user, so no point displaying
// another message.
Expand All @@ -58,7 +58,7 @@ export abstract class BaseInstaller {
return promise;
}

public async install(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
public async install(product: Product, resource?: Uri): Promise<InstallerResponse> {
if (product === Product.unittest) {
return InstallerResponse.Installed;
}
Expand All @@ -74,11 +74,11 @@ export abstract class BaseInstaller {
await installer.installModule(moduleName, resource)
.catch(logger.logError.bind(logger, `Error in installing the module '${moduleName}'`));

return this.isInstalled(product)
return this.isInstalled(product, resource)
.then(isInstalled => isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore);
}

public async isInstalled(product: Product, resource?: vscode.Uri): Promise<boolean | undefined> {
public async isInstalled(product: Product, resource?: Uri): Promise<boolean | undefined> {
if (product === Product.unittest) {
return true;
}
Expand All @@ -102,18 +102,18 @@ export abstract class BaseInstaller {
.catch(() => false);
}
}
protected abstract promptToInstallImplementation(product: Product, resource?: vscode.Uri): Promise<InstallerResponse>;
protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
protected abstract promptToInstallImplementation(product: Product, resource?: Uri): Promise<InstallerResponse>;
protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
throw new Error('getExecutableNameFromSettings is not supported on this object');
}
}

export class CTagsInstaller extends BaseInstaller {
constructor(serviceContainer: IServiceContainer, outputChannel: vscode.OutputChannel) {
constructor(serviceContainer: IServiceContainer, outputChannel: OutputChannel) {
super(serviceContainer, outputChannel);
}

public async install(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
public async install(product: Product, resource?: Uri): Promise<InstallerResponse> {
if (this.serviceContainer.get<IPlatformService>(IPlatformService).isWindows) {
this.outputChannel.appendLine('Install Universal Ctags Win32 to enable support for Workspace Symbols');
this.outputChannel.appendLine('Download the CTags binary from the Universal CTags site.');
Expand All @@ -129,19 +129,19 @@ export class CTagsInstaller extends BaseInstaller {
}
return InstallerResponse.Ignore;
}
protected async promptToInstallImplementation(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
protected async promptToInstallImplementation(product: Product, resource?: Uri): Promise<InstallerResponse> {
const item = await this.appShell.showErrorMessage('Install CTags to enable Python workspace symbols?', 'Yes', 'No');
return item === 'Yes' ? this.install(product, resource) : InstallerResponse.Ignore;
}

protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
const settings = this.configService.getSettings(resource);
return settings.workspaceSymbols.ctagsPath;
}
}

export class FormatterInstaller extends BaseInstaller {
protected async promptToInstallImplementation(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
protected async promptToInstallImplementation(product: Product, resource?: Uri): Promise<InstallerResponse> {
// Hard-coded on purpose because the UI won't necessarily work having
// another formatter.
const formatters = [Product.autopep8, Product.black, Product.yapf];
Expand All @@ -168,7 +168,7 @@ export class FormatterInstaller extends BaseInstaller {
return InstallerResponse.Ignore;
}

protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
const settings = this.configService.getSettings(resource);
const formatHelper = this.serviceContainer.get<IFormatterHelper>(IFormatterHelper);
const settingsPropNames = formatHelper.getSettingsPropertyNames(product);
Expand All @@ -178,7 +178,7 @@ export class FormatterInstaller extends BaseInstaller {

// tslint:disable-next-line:max-classes-per-file
export class LinterInstaller extends BaseInstaller {
protected async promptToInstallImplementation(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
protected async promptToInstallImplementation(product: Product, resource?: Uri): Promise<InstallerResponse> {
const productName = ProductNames.get(product)!;
const install = 'Install';
const disableAllLinting = 'Disable linting';
Expand All @@ -199,21 +199,21 @@ export class LinterInstaller extends BaseInstaller {
}
return InstallerResponse.Ignore;
}
protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
const linterManager = this.serviceContainer.get<ILinterManager>(ILinterManager);
return linterManager.getLinterInfo(product).pathName(resource);
}
}

// tslint:disable-next-line:max-classes-per-file
export class TestFrameworkInstaller extends BaseInstaller {
protected async promptToInstallImplementation(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
protected async promptToInstallImplementation(product: Product, resource?: Uri): Promise<InstallerResponse> {
const productName = ProductNames.get(product)!;
const item = await this.appShell.showErrorMessage(`Test framework ${productName} is not installed. Install?`, 'Yes', 'No');
return item === 'Yes' ? this.install(product, resource) : InstallerResponse.Ignore;
}

protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
const testHelper = this.serviceContainer.get<ITestsHelper>(ITestsHelper);
const settingsPropNames = testHelper.getSettingsPropertyNames(product);
if (!settingsPropNames.pathName) {
Expand All @@ -227,12 +227,12 @@ export class TestFrameworkInstaller extends BaseInstaller {

// tslint:disable-next-line:max-classes-per-file
export class RefactoringLibraryInstaller extends BaseInstaller {
protected async promptToInstallImplementation(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
protected async promptToInstallImplementation(product: Product, resource?: Uri): Promise<InstallerResponse> {
const productName = ProductNames.get(product)!;
const item = await this.appShell.showErrorMessage(`Refactoring library ${productName} is not installed. Install?`, 'Yes', 'No');
return item === 'Yes' ? this.install(product, resource) : InstallerResponse.Ignore;
}
protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
return translateProductToModule(product, ModuleNamePurpose.run);
}
}
Expand All @@ -243,7 +243,7 @@ export class ProductInstaller implements IInstaller {
private ProductTypes = new Map<Product, ProductType>();

constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer,
@inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private outputChannel: vscode.OutputChannel) {
@inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private outputChannel: OutputChannel) {
this.ProductTypes.set(Product.flake8, ProductType.Linter);
this.ProductTypes.set(Product.mypy, ProductType.Linter);
this.ProductTypes.set(Product.pep8, ProductType.Linter);
Expand All @@ -263,13 +263,13 @@ export class ProductInstaller implements IInstaller {

// tslint:disable-next-line:no-empty
public dispose() { }
public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
public async promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse> {
return this.createInstaller(product).promptToInstall(product, resource);
}
public async install(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
public async install(product: Product, resource?: Uri): Promise<InstallerResponse> {
return this.createInstaller(product).install(product, resource);
}
public async isInstalled(product: Product, resource?: vscode.Uri): Promise<boolean | undefined> {
public async isInstalled(product: Product, resource?: Uri): Promise<boolean | undefined> {
return this.createInstaller(product).isInstalled(product, resource);
}
public translateProductToModuleName(product: Product, purpose: ModuleNamePurpose): string {
Expand Down