Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/2 Fixes/16027.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Revert linter installation prompt removal.
107 changes: 103 additions & 4 deletions src/client/common/installer/productInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import { CancellationToken, OutputChannel, Uri } from 'vscode';
import '../extensions';
import { IInterpreterService } from '../../interpreter/contracts';
import { IServiceContainer } from '../../ioc/types';
import { LinterId } from '../../linters/types';
import { EnvironmentType, PythonEnvironment } from '../../pythonEnvironments/info';
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { IApplicationShell, IWorkspaceService } from '../application/types';
import { STANDARD_OUTPUT_CHANNEL } from '../constants';
import { IApplicationShell, ICommandManager, IWorkspaceService } from '../application/types';
import { Commands, STANDARD_OUTPUT_CHANNEL } from '../constants';
import { traceError, traceInfo } from '../logger';
import { IPlatformService } from '../platform/types';
import { IProcessServiceFactory, IPythonExecutionFactory } from '../process/types';
Expand All @@ -21,12 +22,13 @@ import {
IInstaller,
InstallerResponse,
IOutputChannel,
IPersistentStateFactory,
ProductInstallStatus,
ModuleNamePurpose,
Product,
ProductType,
} from '../types';
import { Installer } from '../utils/localize';
import { Common, Installer, Linters } from '../utils/localize';
import { isResource, noop } from '../utils/misc';
import { translateProductToModule } from './moduleInstaller';
import { ProductNames } from './productNames';
Expand Down Expand Up @@ -306,7 +308,102 @@ export class FormatterInstaller extends BaseInstaller {
return InstallerResponse.Ignore;
}
}
class TestFrameworkInstaller extends BaseInstaller {

export class LinterInstaller extends BaseInstaller {
constructor(protected serviceContainer: IServiceContainer, protected outputChannel: OutputChannel) {
super(serviceContainer, outputChannel);
}

protected async promptToInstallImplementation(
product: Product,
resource?: Uri,
cancel?: CancellationToken,
): Promise<InstallerResponse> {
sendTelemetryEvent(EventName.LINTER_INSTALL_PROMPT, undefined, {
prompt: 'old',
});

return this.oldPromptForInstallation(product, resource, cancel);
}

/**
* For installers that want to avoid prompting the user over and over, they can make use of a
* persisted true/false value representing user responses to 'stop showing this prompt'. This method
* gets the persisted value given the installer-defined key.
*
* @param key Key to use to get a persisted response value, each installer must define this for themselves.
* @returns Boolean: The current state of the stored response key given.
*/
protected getStoredResponse(key: string): boolean {
const factory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
const state = factory.createGlobalPersistentState<boolean | undefined>(key, undefined);
return state.value === true;
}

private async oldPromptForInstallation(product: Product, resource?: Uri, cancel?: CancellationToken) {
const productName = ProductNames.get(product)!;
const install = Common.install();
const doNotShowAgain = Common.doNotShowAgain();
const disableLinterInstallPromptKey = `${productName}_DisableLinterInstallPrompt`;
const selectLinter = Linters.selectLinter();

if (this.getStoredResponse(disableLinterInstallPromptKey) === true) {
return InstallerResponse.Ignore;
}

const options = [selectLinter, doNotShowAgain];

let message = `Linter ${productName} is not installed.`;
if (this.isExecutableAModule(product, resource)) {
options.splice(0, 0, install);
} else {
const executable = this.getExecutableNameFromSettings(product, resource);
message = `Path to the ${productName} linter is invalid (${executable})`;
}
const response = await this.appShell.showErrorMessage(message, ...options);
if (response === install) {
sendTelemetryEvent(EventName.LINTER_NOT_INSTALLED_PROMPT, undefined, {
tool: productName as LinterId,
action: 'install',
});
return this.install(product, resource, cancel);
}
if (response === doNotShowAgain) {
await this.setStoredResponse(disableLinterInstallPromptKey, true);
sendTelemetryEvent(EventName.LINTER_NOT_INSTALLED_PROMPT, undefined, {
tool: productName as LinterId,
action: 'disablePrompt',
});
return InstallerResponse.Ignore;
}

if (response === selectLinter) {
sendTelemetryEvent(EventName.LINTER_NOT_INSTALLED_PROMPT, undefined, { action: 'select' });
const commandManager = this.serviceContainer.get<ICommandManager>(ICommandManager);
await commandManager.executeCommand(Commands.Set_Linter);
}
return InstallerResponse.Ignore;
}

/**
* For installers that want to avoid prompting the user over and over, they can make use of a
* persisted true/false value representing user responses to 'stop showing this prompt'. This
* method will set that persisted value given the installer-defined key.
*
* @param key Key to use to get a persisted response value, each installer must define this for themselves.
* @param value Boolean value to store for the user - if they choose to not be prompted again for instance.
* @returns Boolean: The current state of the stored response key given.
*/
private async setStoredResponse(key: string, value: boolean): Promise<void> {
const factory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
const state = factory.createGlobalPersistentState<boolean | undefined>(key, undefined);
if (state && state.value !== value) {
await state.updateValue(value);
}
}
}

export class TestFrameworkInstaller extends BaseInstaller {
protected async promptToInstallImplementation(
product: Product,
resource?: Uri,
Expand Down Expand Up @@ -490,6 +587,8 @@ export class ProductInstaller implements IInstaller {
switch (productType) {
case ProductType.Formatter:
return new FormatterInstaller(this.serviceContainer, this.outputChannel);
case ProductType.Linter:
return new LinterInstaller(this.serviceContainer, this.outputChannel);
case ProductType.WorkspaceSymbols:
return new CTagsInstaller(this.serviceContainer, this.outputChannel);
case ProductType.TestFramework:
Expand Down
2 changes: 2 additions & 0 deletions src/client/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ export enum EventName {

SELECT_LINTER = 'LINTING.SELECT',

LINTER_NOT_INSTALLED_PROMPT = 'LINTER_NOT_INSTALLED_PROMPT',
LINTER_INSTALL_PROMPT = 'LINTER_INSTALL_PROMPT',
CONFIGURE_AVAILABLE_LINTER_PROMPT = 'CONFIGURE_AVAILABLE_LINTER_PROMPT',
HASHED_PACKAGE_NAME = 'HASHED_PACKAGE_NAME',
HASHED_PACKAGE_PERF = 'HASHED_PACKAGE_PERF',
Expand Down
33 changes: 33 additions & 0 deletions src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,39 @@ export interface IEventNamePropertyMapping {
hashedName: string;
};
[EventName.HASHED_PACKAGE_PERF]: never | undefined;
/**
* Telemetry event sent with details of selection in prompt
* `Prompt message` :- 'Linter ${productName} is not installed'
*/
[EventName.LINTER_NOT_INSTALLED_PROMPT]: {
/**
* Name of the linter
*
* @type {LinterId}
*/
tool?: LinterId;
/**
* `select` When 'Select linter' option is selected
* `disablePrompt` When 'Do not show again' option is selected
* `install` When 'Install' option is selected
*
* @type {('select' | 'disablePrompt' | 'install')}
*/
action: 'select' | 'disablePrompt' | 'install';
};

/**
* Telemetry event sent before showing the linter prompt to install
* pylint or flake8.
*/
[EventName.LINTER_INSTALL_PROMPT]: {
/**
* Identify which prompt was shown.
*
* @type {('old' | 'noPrompt' | 'pylintFirst' | 'flake8first')}
*/
prompt: 'old' | 'noPrompt' | 'pylintFirst' | 'flake8first';
};

/**
* Telemetry event sent when installing modules
Expand Down
6 changes: 2 additions & 4 deletions src/test/common/installer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,9 @@ suite('Installer', () => {
}
getNamesAndValues<Product>(Product).forEach((prod) => {
test(`Ensure isInstalled for Product: '${prod.name}' executes the right command`, async function () {
const productType = new ProductService().getProductType(prod.value);
if (productType === ProductType.DataScience || productType === ProductType.Linter) {
if (new ProductService().getProductType(prod.value) === ProductType.DataScience) {
return this.skip();
}

ioc.serviceManager.addSingletonInstance<IModuleInstaller>(
IModuleInstaller,
new MockModuleInstaller('one', false),
Expand Down Expand Up @@ -361,7 +359,7 @@ suite('Installer', () => {
getNamesAndValues<Product>(Product).forEach((prod) => {
test(`Ensure install for Product: '${prod.name}' executes the right command in IModuleInstaller`, async function () {
const productType = new ProductService().getProductType(prod.value);
if (productType === ProductType.DataScience || productType === ProductType.Linter) {
if (productType === ProductType.DataScience) {
return this.skip();
}
ioc.serviceManager.addSingletonInstance<IModuleInstaller>(
Expand Down
Loading