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.

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

Remove isort extension dependency #20577

Merged
merged 4 commits into from Feb 6, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion gulpfile.js
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
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
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
@@ -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
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
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
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
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
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
@@ -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
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
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