Skip to content

Commit

Permalink
Remove old linter and formatter prompts and commands (#21979)
Browse files Browse the repository at this point in the history
  • Loading branch information
@karthiknadig
karthiknadig authored Sep 18, 2023
1 parent 05ae266 commit b41fee7
Show file tree
Hide file tree
Showing 18 changed files with 64 additions and 1,228 deletions.
33 changes: 0 additions & 33 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,11 +364,6 @@
"command": "python.createEnvironment-button",
"title": "%python.command.python.createEnvironment.title%"
},
{
"category": "Python",
"command": "python.enableLinting",
"title": "%python.command.python.enableLinting.title%"
},
{
"category": "Python",
"command": "python.enableSourceMapSupport",
Expand Down Expand Up @@ -430,21 +425,11 @@
"icon": "$(run-errors)",
"title": "%python.command.testing.rerunFailedTests.title%"
},
{
"category": "Python",
"command": "python.runLinting",
"title": "%python.command.python.runLinting.title%"
},
{
"category": "Python",
"command": "python.setInterpreter",
"title": "%python.command.python.setInterpreter.title%"
},
{
"category": "Python",
"command": "python.setLinter",
"title": "%python.command.python.setLinter.title%"
},
{
"category": "Python Refactor",
"command": "python.sortImports",
Expand Down Expand Up @@ -1807,12 +1792,6 @@
"title": "%python.command.python.createTerminal.title%",
"when": "!virtualWorkspace && shellExecutionSupported"
},
{
"category": "Python",
"command": "python.enableLinting",
"title": "%python.command.python.enableLinting.title%",
"when": "!virtualWorkspace && shellExecutionSupported && editorLangId == python"
},
{
"category": "Python",
"command": "python.enableSourceMapSupport",
Expand Down Expand Up @@ -1885,24 +1864,12 @@
"title": "%python.command.testing.rerunFailedTests.title%",
"when": "!virtualWorkspace && shellExecutionSupported"
},
{
"category": "Python",
"command": "python.runLinting",
"title": "%python.command.python.runLinting.title%",
"when": "!virtualWorkspace && shellExecutionSupported && editorLangId == python"
},
{
"category": "Python",
"command": "python.setInterpreter",
"title": "%python.command.python.setInterpreter.title%",
"when": "!virtualWorkspace && shellExecutionSupported"
},
{
"category": "Python",
"command": "python.setLinter",
"title": "%python.command.python.setLinter.title%",
"when": "!virtualWorkspace && shellExecutionSupported && editorLangId == python"
},
{
"category": "Python Refactor",
"command": "python.sortImports",
Expand Down
3 changes: 0 additions & 3 deletions package.nls.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
"python.command.python.execSelectionInTerminal.title": "Run Selection/Line in Python Terminal",
"python.command.python.execSelectionInDjangoShell.title": "Run Selection/Line in Django Shell",
"python.command.python.reportIssue.title": "Report Issue...",
"python.command.python.setLinter.title": "Select Linter",
"python.command.python.enableLinting.title": "Enable/Disable Linting",
"python.command.python.runLinting.title": "Run Linting",
"python.command.python.enableSourceMapSupport.title": "Enable Source Map Support For Extension Debugging",
"python.command.python.clearCacheAndReload.title": "Clear Cache and Reload Window",
"python.command.python.analysis.restartLanguageServer.title": "Restart Language Server",
Expand Down
3 changes: 0 additions & 3 deletions src/client/common/application/commands.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ interface ICommandNameWithoutArgumentTypeMapping {
[Commands.ClearWorkspaceInterpreter]: [];
[Commands.Set_Interpreter]: [];
[Commands.Set_ShebangInterpreter]: [];
[Commands.Run_Linter]: [];
[Commands.Enable_Linter]: [];
['workbench.action.showCommands']: [];
['workbench.action.debug.continue']: [];
['workbench.action.debug.stepOver']: [];
Expand All @@ -35,7 +33,6 @@ interface ICommandNameWithoutArgumentTypeMapping {
['editor.action.formatDocument']: [];
['editor.action.rename']: [];
[Commands.ViewOutput]: [];
[Commands.Set_Linter]: [];
[Commands.Start_REPL]: [];
[Commands.Enable_SourceMap_Support]: [];
[Commands.Exec_Selection_In_Terminal]: [];
Expand Down
3 changes: 0 additions & 3 deletions src/client/common/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ export namespace Commands {
export const Create_Environment_Button = 'python.createEnvironment-button';
export const Create_Terminal = 'python.createTerminal';
export const Debug_In_Terminal = 'python.debugInTerminal';
export const Enable_Linter = 'python.enableLinting';
export const Enable_SourceMap_Support = 'python.enableSourceMapSupport';
export const Exec_In_Terminal = 'python.execInTerminal';
export const Exec_In_Terminal_Icon = 'python.execInTerminal-icon';
Expand All @@ -56,9 +55,7 @@ export namespace Commands {
export const PickLocalProcess = 'python.pickLocalProcess';
export const RefreshTensorBoard = 'python.refreshTensorBoard';
export const ReportIssue = 'python.reportIssue';
export const Run_Linter = 'python.runLinting';
export const Set_Interpreter = 'python.setInterpreter';
export const Set_Linter = 'python.setLinter';
export const Set_ShebangInterpreter = 'python.setShebangInterpreter';
export const Sort_Imports = 'python.sortImports';
export const Start_REPL = 'python.startREPL';
Expand Down
162 changes: 2 additions & 160 deletions src/client/common/installer/productInstaller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ import { CancellationToken, l10n, Uri } from 'vscode';
import '../extensions';
import { IInterpreterService } from '../../interpreter/contracts';
import { IServiceContainer } from '../../ioc/types';
import { LinterId } from '../../linters/types';
import { EnvironmentType, ModuleInstallerType, PythonEnvironment } from '../../pythonEnvironments/info';
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { IApplicationShell, ICommandManager, IWorkspaceService } from '../application/types';
import { Commands } from '../constants';
import { IApplicationShell, IWorkspaceService } from '../application/types';
import { IProcessServiceFactory, IPythonExecutionFactory } from '../process/types';
import {
IConfigurationService,
Expand All @@ -22,7 +20,7 @@ import {
Product,
ProductType,
} from '../types';
import { Common, Linters } from '../utils/localize';
import { Common } from '../utils/localize';
import { isResource, noop } from '../utils/misc';
import { translateProductToModule } from './moduleInstaller';
import { ProductNames } from './productNames';
Expand Down Expand Up @@ -225,158 +223,6 @@ abstract class BaseInstaller implements IBaseInstaller {
}
}

const doNotDisplayFormatterPromptStateKey = 'FORMATTER_NOT_INSTALLED_KEY';

export class FormatterInstaller extends BaseInstaller {
protected async promptToInstallImplementation(
product: Product,
resource?: Uri,
cancel?: CancellationToken,
_flags?: ModuleInstallFlags,
): Promise<InstallerResponse> {
const neverShowAgain = this.persistentStateFactory.createGlobalPersistentState(
doNotDisplayFormatterPromptStateKey,
false,
);

if (neverShowAgain.value) {
return InstallerResponse.Ignore;
}

// Hard-coded on purpose because the UI won't necessarily work having
// another formatter.
const formatters = [Product.autopep8, Product.black, Product.yapf];
const formatterNames = formatters.map((formatter) => ProductNames.get(formatter)!);
const productName = ProductNames.get(product)!;
formatterNames.splice(formatterNames.indexOf(productName), 1);
const useOptions = formatterNames.map((name) => l10n.t('Use {0}', name));
const yesChoice = Common.bannerLabelYes;

const options = [...useOptions, Common.doNotShowAgain];
let message = l10n.t('Formatter {0} is not installed. Install?', productName);
if (this.isExecutableAModule(product, resource)) {
options.splice(0, 0, yesChoice);
} else {
const executable = this.getExecutableNameFromSettings(product, resource);
message = l10n.t('Path to the {0} formatter is invalid ({1})', productName, executable);
}

const item = await this.appShell.showErrorMessage(message, ...options);
if (item === yesChoice) {
return this.install(product, resource, cancel);
}

if (item === Common.doNotShowAgain) {
neverShowAgain.updateValue(true);
return InstallerResponse.Ignore;
}

if (typeof item === 'string') {
for (const formatter of formatters) {
const formatterName = ProductNames.get(formatter)!;

if (item.endsWith(formatterName)) {
await this.configService.updateSetting('formatting.provider', formatterName, resource);
return this.install(formatter, resource, cancel);
}
}
}

return InstallerResponse.Ignore;
}
}

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

protected async promptToInstallImplementation(
product: Product,
resource?: Uri,
cancel?: CancellationToken,
_flags?: ModuleInstallFlags,
): Promise<InstallerResponse> {
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;
const { doNotShowAgain } = Common;
const disableLinterInstallPromptKey = `${productName}_DisableLinterInstallPrompt`;
const { selectLinter } = Linters;

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

const options = [selectLinter, doNotShowAgain];

let message = l10n.t('Linter {0} is not installed.', productName);
if (this.isExecutableAModule(product, resource)) {
options.splice(0, 0, install);
} else {
const executable = this.getExecutableNameFromSettings(product, resource);
message = l10n.t('Path to the {0} linter is invalid ({1})', productName, 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,
Expand Down Expand Up @@ -687,10 +533,6 @@ export class ProductInstaller implements IInstaller {
private createInstaller(product: Product): IBaseInstaller {
const productType = this.productService.getProductType(product);
switch (productType) {
case ProductType.Formatter:
return new FormatterInstaller(this.serviceContainer);
case ProductType.Linter:
return new LinterInstaller(this.serviceContainer);
case ProductType.TestFramework:
return new TestFrameworkInstaller(this.serviceContainer);
case ProductType.DataScience:
Expand Down
3 changes: 0 additions & 3 deletions src/client/extensionActivation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import { IDebugConfigurationService, IDynamicDebugConfigurationService } from '.
import { registerTypes as formattersRegisterTypes } from './formatters/serviceRegistry';
import { IInterpreterService } from './interpreter/contracts';
import { getLanguageConfiguration } from './language/languageConfiguration';
import { LinterCommands } from './linters/linterCommands';
import { registerTypes as lintersRegisterTypes } from './linters/serviceRegistry';
import { PythonFormattingEditProvider } from './providers/formatProvider';
import { ReplProvider } from './providers/replProvider';
Expand Down Expand Up @@ -168,8 +167,6 @@ async function activateLegacy(ext: ExtensionState): Promise<ActivationResult> {

serviceManager.get<ICodeExecutionManager>(ICodeExecutionManager).registerCommands();

disposables.push(new LinterCommands(serviceManager));

if (
pythonSettings &&
pythonSettings.formatting &&
Expand Down
6 changes: 1 addition & 5 deletions src/client/linters/errorHandlers/errorHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@ import { ExecutionInfo, Product } from '../../common/types';
import { IServiceContainer } from '../../ioc/types';
import { IErrorHandler } from '../types';
import { BaseErrorHandler } from './baseErrorHandler';
import { NotInstalledErrorHandler } from './notInstalled';
import { StandardErrorHandler } from './standard';

export class ErrorHandler implements IErrorHandler {
private handler: BaseErrorHandler;

constructor(product: Product, serviceContainer: IServiceContainer) {
// Create chain of handlers.
const standardErrorHandler = new StandardErrorHandler(product, serviceContainer);
this.handler = new NotInstalledErrorHandler(product, serviceContainer);
this.handler.setNextHandler(standardErrorHandler);
this.handler = new StandardErrorHandler(product, serviceContainer);
}

public handleError(error: Error, resource: Uri, execInfo: ExecutionInfo): Promise<boolean> {
Expand Down
29 changes: 0 additions & 29 deletions src/client/linters/errorHandlers/notInstalled.ts
View file

This file was deleted.

Loading

0 comments on commit b41fee7

Please sign in to comment.