Skip to content

Commit

Permalink
Remove isort extension dependency (#20577)
Browse files Browse the repository at this point in the history
Closes #20586
  • Loading branch information
karthiknadig authored Feb 6, 2023
1 parent fe4c5f1 commit cd6ca9d
Show file tree
Hide file tree
Showing 21 changed files with 274 additions and 86 deletions.
2 changes: 1 addition & 1 deletion gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ async function addExtensionPackDependencies() {
// extension dependencies need not be installed during development
const packageJsonContents = await fsExtra.readFile('package.json', 'utf-8');
const packageJson = JSON.parse(packageJsonContents);
packageJson.extensionPack = ['ms-toolsai.jupyter', 'ms-python.vscode-pylance', 'ms-python.isort'].concat(
packageJson.extensionPack = ['ms-toolsai.jupyter', 'ms-python.vscode-pylance'].concat(
packageJson.extensionPack ? packageJson.extensionPack : [],
);
await fsExtra.writeFile('package.json', JSON.stringify(packageJson, null, 4), 'utf-8');
Expand Down
14 changes: 0 additions & 14 deletions pythonFiles/sortImports.py

This file was deleted.

17 changes: 0 additions & 17 deletions src/client/common/process/internal/scripts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,23 +58,6 @@ export function interpreterInfo(): [string[], (out: string) => InterpreterInfoJs
return [args, parse];
}

// sortImports.py

export function sortImports(filename: string, sortArgs?: string[]): [string[], (out: string) => string] {
const script = path.join(SCRIPTS_DIR, 'sortImports.py');
const args = [script, filename, '--diff'];
if (sortArgs) {
args.push(...sortArgs);
}

function parse(out: string) {
// It should just be a diff that the extension will use directly.
return out;
}

return [args, parse];
}

// normalizeSelection.py

export function normalizeSelection(): [string[], (out: string) => string] {
Expand Down
4 changes: 4 additions & 0 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,10 @@ export namespace ToolsExtensions {
export const pylintPromptMessage = l10n.t(
'Use the Pylint extension to enable easier configuration and new features such as quick fixes.',
);
export const isortPromptMessage = l10n.t(
'To use sort imports, please install the isort extension. It provides easier configuration and new features such as code actions.',
);
export const installPylintExtension = l10n.t('Install Pylint extension');
export const installFlake8Extension = l10n.t('Install Flake8 extension');
export const installISortExtension = l10n.t('Install isort extension');
}
30 changes: 30 additions & 0 deletions src/client/common/vscodeApis/extensionsApi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import * as path from 'path';
import * as fs from 'fs-extra';
import { Extension, extensions } from 'vscode';
import { PVSC_EXTENSION_ID } from '../constants';

export function getExtension<T = unknown>(extensionId: string): Extension<T> | undefined {
return extensions.getExtension(extensionId);
}

export function isExtensionEnabled(extensionId: string): boolean {
return extensions.getExtension(extensionId) !== undefined;
}

export function isExtensionDisabled(extensionId: string): boolean {
// We need an enabled extension to find the extensions dir.
const pythonExt = getExtension(PVSC_EXTENSION_ID);
if (pythonExt) {
let found = false;
fs.readdirSync(path.dirname(pythonExt.extensionPath), { withFileTypes: false }).forEach((s) => {
if (s.toString().startsWith(extensionId)) {
found = true;
}
});
return found;
}
return false;
}
10 changes: 8 additions & 2 deletions src/client/linters/flake8.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { Product } from '../common/types';
import { IServiceContainer } from '../ioc/types';
import { traceLog } from '../logging';
import { BaseLinter } from './baseLinter';
import { isExtensionEnabled } from './prompts/common';
import { FLAKE8_EXTENSION } from './prompts/flake8Prompt';
import { IToolsExtensionPrompt } from './prompts/types';
import { ILintMessage } from './types';

Expand All @@ -15,8 +17,12 @@ export class Flake8 extends BaseLinter {
}

protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
if (await this.prompt.showPrompt()) {
traceLog('LINTING: Skipping linting from Python extension, since Flake8 extension is installed.');
await this.prompt.showPrompt();

if (isExtensionEnabled(this.serviceContainer, FLAKE8_EXTENSION)) {
traceLog(
'LINTING: Skipping linting from Python extension, since Flake8 extension is installed and enabled.',
);
return [];
}

Expand Down
12 changes: 6 additions & 6 deletions src/client/linters/prompts/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { IExperimentService, IExtensions, IPersistentState, IPersistentStateFact
import { IServiceContainer } from '../../ioc/types';
import { traceLog } from '../../logging';

function isExtensionInstalledButDisabled(extensions: IExtensions, extensionId: string): boolean {
export function isExtensionDisabled(serviceContainer: IServiceContainer, extensionId: string): boolean {
const extensions: IExtensions = serviceContainer.get<IExtensions>(IExtensions);
// When debugging the python extension this `extensionPath` below will point to your repo.
// If you are debugging this feature then set the `extensionPath` to right location after
// the next line.
Expand All @@ -26,13 +27,12 @@ function isExtensionInstalledButDisabled(extensions: IExtensions, extensionId: s
return false;
}

export function isExtensionInstalled(serviceContainer: IServiceContainer, extensionId: string): boolean {
/**
* Detects if extension is installed and enabled.
*/
export function isExtensionEnabled(serviceContainer: IServiceContainer, extensionId: string): boolean {
const extensions: IExtensions = serviceContainer.get<IExtensions>(IExtensions);
const extension = extensions.getExtension(extensionId);
if (!extension) {
// The extension you are looking for might be disabled.
return isExtensionInstalledButDisabled(extensions, extensionId);
}
return extension !== undefined;
}

Expand Down
6 changes: 4 additions & 2 deletions src/client/linters/prompts/flake8Prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { showInformationMessage } from '../../common/vscodeApis/windowApis';
import { IServiceContainer } from '../../ioc/types';
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { isExtensionInstalled, doNotShowPromptState, inToolsExtensionsExperiment } from './common';
import { doNotShowPromptState, inToolsExtensionsExperiment, isExtensionDisabled, isExtensionEnabled } from './common';
import { IToolsExtensionPrompt } from './types';

export const FLAKE8_EXTENSION = 'ms-python.flake8';
Expand All @@ -20,9 +20,11 @@ export class Flake8ExtensionPrompt implements IToolsExtensionPrompt {
public constructor(private readonly serviceContainer: IServiceContainer) {}

public async showPrompt(): Promise<boolean> {
if (isExtensionInstalled(this.serviceContainer, FLAKE8_EXTENSION)) {
const isEnabled = isExtensionEnabled(this.serviceContainer, FLAKE8_EXTENSION);
if (isEnabled || isExtensionDisabled(this.serviceContainer, FLAKE8_EXTENSION)) {
sendTelemetryEvent(EventName.TOOLS_EXTENSIONS_ALREADY_INSTALLED, undefined, {
extensionId: FLAKE8_EXTENSION,
isEnabled,
});
return true;
}
Expand Down
6 changes: 4 additions & 2 deletions src/client/linters/prompts/pylintPrompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { showInformationMessage } from '../../common/vscodeApis/windowApis';
import { IServiceContainer } from '../../ioc/types';
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { doNotShowPromptState, inToolsExtensionsExperiment, isExtensionInstalled } from './common';
import { doNotShowPromptState, inToolsExtensionsExperiment, isExtensionDisabled, isExtensionEnabled } from './common';
import { IToolsExtensionPrompt } from './types';

export const PYLINT_EXTENSION = 'ms-python.pylint';
Expand All @@ -20,9 +20,11 @@ export class PylintExtensionPrompt implements IToolsExtensionPrompt {
public constructor(private readonly serviceContainer: IServiceContainer) {}

public async showPrompt(): Promise<boolean> {
if (isExtensionInstalled(this.serviceContainer, PYLINT_EXTENSION)) {
const isEnabled = isExtensionEnabled(this.serviceContainer, PYLINT_EXTENSION);
if (isEnabled || isExtensionDisabled(this.serviceContainer, PYLINT_EXTENSION)) {
sendTelemetryEvent(EventName.TOOLS_EXTENSIONS_ALREADY_INSTALLED, undefined, {
extensionId: PYLINT_EXTENSION,
isEnabled,
});
return true;
}
Expand Down
10 changes: 8 additions & 2 deletions src/client/linters/pylint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { Product } from '../common/types';
import { IServiceContainer } from '../ioc/types';
import { traceError, traceLog } from '../logging';
import { BaseLinter } from './baseLinter';
import { isExtensionEnabled } from './prompts/common';
import { PYLINT_EXTENSION } from './prompts/pylintPrompt';
import { IToolsExtensionPrompt } from './prompts/types';
import { ILintMessage } from './types';

Expand All @@ -26,8 +28,12 @@ export class Pylint extends BaseLinter {
}

protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
if (await this.prompt.showPrompt()) {
traceLog('LINTING: Skipping linting from Python extension, since Pylint extension is installed.');
await this.prompt.showPrompt();

if (isExtensionEnabled(this.serviceContainer, PYLINT_EXTENSION)) {
traceLog(
'LINTING: Skipping linting from Python extension, since Pylint extension is installed and enabled.',
);
return [];
}

Expand Down
89 changes: 89 additions & 0 deletions src/client/providers/codeActionProvider/isortPrompt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { IApplicationEnvironment } from '../../common/application/types';
import { IPersistentState, IPersistentStateFactory } from '../../common/types';
import { Common, ToolsExtensions } from '../../common/utils/localize';
import { executeCommand } from '../../common/vscodeApis/commandApis';
import { isExtensionDisabled, isExtensionEnabled } from '../../common/vscodeApis/extensionsApi';
import { showInformationMessage } from '../../common/vscodeApis/windowApis';
import { IServiceContainer } from '../../ioc/types';
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';

export const ISORT_EXTENSION = 'ms-python.isort';
const ISORT_PROMPT_DONOTSHOW_KEY = 'showISortExtensionPrompt';

function doNotShowPromptState(serviceContainer: IServiceContainer, promptKey: string): IPersistentState<boolean> {
const persistFactory: IPersistentStateFactory = serviceContainer.get<IPersistentStateFactory>(
IPersistentStateFactory,
);
return persistFactory.createWorkspacePersistentState<boolean>(promptKey, false);
}

export class ISortExtensionPrompt {
private shownThisSession = false;

public constructor(private readonly serviceContainer: IServiceContainer) {}

public async showPrompt(): Promise<boolean> {
const isEnabled = isExtensionEnabled(ISORT_EXTENSION);
if (isEnabled || isExtensionDisabled(ISORT_EXTENSION)) {
sendTelemetryEvent(EventName.TOOLS_EXTENSIONS_ALREADY_INSTALLED, undefined, {
extensionId: ISORT_EXTENSION,
isEnabled,
});
return true;
}

const doNotShow = doNotShowPromptState(this.serviceContainer, ISORT_PROMPT_DONOTSHOW_KEY);
if (this.shownThisSession || doNotShow.value) {
return false;
}

sendTelemetryEvent(EventName.TOOLS_EXTENSIONS_PROMPT_SHOWN, undefined, { extensionId: ISORT_EXTENSION });
this.shownThisSession = true;
const response = await showInformationMessage(
ToolsExtensions.isortPromptMessage,
ToolsExtensions.installISortExtension,
Common.doNotShowAgain,
);

if (response === Common.doNotShowAgain) {
await doNotShow.updateValue(true);
sendTelemetryEvent(EventName.TOOLS_EXTENSIONS_PROMPT_DISMISSED, undefined, {
extensionId: ISORT_EXTENSION,
dismissType: 'doNotShow',
});
return false;
}

if (response === ToolsExtensions.installISortExtension) {
sendTelemetryEvent(EventName.TOOLS_EXTENSIONS_INSTALL_SELECTED, undefined, {
extensionId: ISORT_EXTENSION,
});
const appEnv: IApplicationEnvironment = this.serviceContainer.get<IApplicationEnvironment>(
IApplicationEnvironment,
);
await executeCommand('workbench.extensions.installExtension', ISORT_EXTENSION, {
installPreReleaseVersion: appEnv.extensionChannel === 'insiders',
});
return true;
}

sendTelemetryEvent(EventName.TOOLS_EXTENSIONS_PROMPT_DISMISSED, undefined, {
extensionId: ISORT_EXTENSION,
dismissType: 'close',
});

return false;
}
}

let _prompt: ISortExtensionPrompt | undefined;
export function getOrCreateISortPrompt(serviceContainer: IServiceContainer): ISortExtensionPrompt {
if (!_prompt) {
_prompt = new ISortExtensionPrompt(serviceContainer);
}
return _prompt;
}
22 changes: 20 additions & 2 deletions src/client/providers/codeActionProvider/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,20 @@ import { IExtensionSingleActivationService } from '../../activation/types';
import { Commands } from '../../common/constants';
import { IDisposableRegistry } from '../../common/types';
import { executeCommand, registerCommand } from '../../common/vscodeApis/commandApis';
import { isExtensionEnabled } from '../../common/vscodeApis/extensionsApi';
import { IServiceContainer } from '../../ioc/types';
import { traceLog } from '../../logging';
import { getOrCreateISortPrompt, ISORT_EXTENSION } from './isortPrompt';
import { LaunchJsonCodeActionProvider } from './launchJsonCodeActionProvider';

@injectable()
export class CodeActionProviderService implements IExtensionSingleActivationService {
public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: false };

constructor(@inject(IDisposableRegistry) private disposableRegistry: IDisposableRegistry) {}
constructor(
@inject(IDisposableRegistry) private disposableRegistry: IDisposableRegistry,
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
) {}

public async activate(): Promise<void> {
// eslint-disable-next-line global-require
Expand All @@ -29,7 +36,18 @@ export class CodeActionProviderService implements IExtensionSingleActivationServ
}),
);
this.disposableRegistry.push(
registerCommand(Commands.Sort_Imports, () => executeCommand('editor.action.organizeImports')),
registerCommand(Commands.Sort_Imports, async () => {
const prompt = getOrCreateISortPrompt(this.serviceContainer);
await prompt.showPrompt();
if (!isExtensionEnabled(ISORT_EXTENSION)) {
traceLog(
'Sort Imports: Please install and enable `ms-python.isort` extension to use this feature.',
);
return;
}

executeCommand('editor.action.organizeImports');
}),
);
}
}
9 changes: 5 additions & 4 deletions src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2080,7 +2080,8 @@ export interface IEventNamePropertyMapping {
}
*/
[EventName.TOOLS_EXTENSIONS_ALREADY_INSTALLED]: {
extensionId: 'ms-python.pylint' | 'ms-python.flake8';
extensionId: 'ms-python.pylint' | 'ms-python.flake8' | 'ms-python.isort';
isEnabled: boolean;
};
/**
* Telemetry event sent when install linter or formatter extension prompt is shown.
Expand All @@ -2091,7 +2092,7 @@ export interface IEventNamePropertyMapping {
}
*/
[EventName.TOOLS_EXTENSIONS_PROMPT_SHOWN]: {
extensionId: 'ms-python.pylint' | 'ms-python.flake8';
extensionId: 'ms-python.pylint' | 'ms-python.flake8' | 'ms-python.isort';
};
/**
* Telemetry event sent when clicking to install linter or formatter extension from the suggestion prompt.
Expand All @@ -2102,7 +2103,7 @@ export interface IEventNamePropertyMapping {
}
*/
[EventName.TOOLS_EXTENSIONS_INSTALL_SELECTED]: {
extensionId: 'ms-python.pylint' | 'ms-python.flake8';
extensionId: 'ms-python.pylint' | 'ms-python.flake8' | 'ms-python.isort';
};
/**
* Telemetry event sent when dismissing prompt suggesting to install the linter or formatter extension.
Expand All @@ -2114,7 +2115,7 @@ export interface IEventNamePropertyMapping {
}
*/
[EventName.TOOLS_EXTENSIONS_PROMPT_DISMISSED]: {
extensionId: 'ms-python.pylint' | 'ms-python.flake8';
extensionId: 'ms-python.pylint' | 'ms-python.flake8' | 'ms-python.isort';
dismissType: 'close' | 'doNotShow';
};
/* __GDPR__
Expand Down
Loading

0 comments on commit cd6ca9d

Please sign in to comment.