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

Jump to bottom

Remove old linter and formatter prompts and commands #21979

Merged
merged 3 commits into from
Sep 18, 2023
Merged

Remove old linter and formatter prompts and commands #21979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ab0da55
Remove old linter and formatter prompts and commands
karthiknadig Sep 12, 2023
b76f100
Test fixes
karthiknadig Sep 13, 2023
a673313
Improve error messaging for linters
karthiknadig Sep 14, 2023
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
35 changes: 1 addition & 34 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,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 @@ -431,21 +426,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 @@ -1808,12 +1793,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 @@ -1886,24 +1865,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 Expand Up @@ -2221,4 +2188,4 @@
"webpack-require-from": "^1.8.6",
"yargs": "^15.3.1"
}
}
}
3 changes: 0 additions & 3 deletions package.nls.json
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop

This file was deleted.

Loading
Loading