From 2c29a9d033a17a5ac8055701df442da9d19032f5 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 14 Mar 2023 22:28:10 -0700 Subject: [PATCH 1/5] Add formatter prompt classes --- src/client/common/application/commands.ts | 2 +- src/client/common/experiments/groups.ts | 4 + src/client/common/utils/localize.ts | 20 ++++ .../prompts/installFormatterPrompt.ts | 104 ++++++++++++++++++ .../prompts/selectFormatterPrompt.ts | 92 ++++++++++++++++ src/client/providers/prompts/types.ts | 13 +++ 6 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 src/client/providers/prompts/installFormatterPrompt.ts create mode 100644 src/client/providers/prompts/selectFormatterPrompt.ts create mode 100644 src/client/providers/prompts/types.ts diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 18d035cde6c9..277baffd19a4 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -62,7 +62,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu ['workbench.action.quickOpen']: [string]; ['workbench.action.openWalkthrough']: [string | { category: string; step: string }, boolean | undefined]; ['workbench.extensions.installExtension']: [ - Uri | 'ms-python.python', + Uri | string, ( | { installOnlyNewlyAddedFromExtensionPackVSIX?: boolean; diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index b575a116096c..5884aafd122d 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -10,3 +10,7 @@ export enum ShowToolsExtensionPrompt { export enum TerminalEnvVarActivation { experiment = 'pythonTerminalEnvVarActivation', } + +export enum ShowFormatterExtensionPrompt { + experiment = 'pythonPromptNewFormatterExt', +} diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 8673fe7cc8cd..2ffe80118ffa 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -473,4 +473,24 @@ export namespace ToolsExtensions { export const installPylintExtension = l10n.t('Install Pylint extension'); export const installFlake8Extension = l10n.t('Install Flake8 extension'); export const installISortExtension = l10n.t('Install isort extension'); + + export const selectBlackFormatterPrompt = l10n.t( + 'You have Black formatter extension installed, would you like to use that as the default formatter?', + ); + + export const selectAutopep8FormatterPrompt = l10n.t( + 'You have Autopep8 formatter extension installed, would you like to use that as the default formatter?', + ); + + export const selectMultipleFormattersPrompt = l10n.t( + 'You have multiple formatters installed, would you like to select one as the default formatter?', + ); + + export const installBlackFormatterPrompt = l10n.t( + 'You triggered formatting with Black, would you like to install the Black formatter extension?', + ); + + export const installAutopep8FormatterPrompt = l10n.t( + 'You triggered formatting with Autopep8, would you like to install the Autopep8 extension?', + ); } diff --git a/src/client/providers/prompts/installFormatterPrompt.ts b/src/client/providers/prompts/installFormatterPrompt.ts new file mode 100644 index 000000000000..510c336ce6fe --- /dev/null +++ b/src/client/providers/prompts/installFormatterPrompt.ts @@ -0,0 +1,104 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { ConfigurationTarget, Uri } from 'vscode'; +import { + IApplicationEnvironment, + IApplicationShell, + ICommandManager, + IWorkspaceService, +} from '../../common/application/types'; +import { ShowFormatterExtensionPrompt } from '../../common/experiments/groups'; +import { IExperimentService, IExtensions, IPersistentStateFactory } from '../../common/types'; +import { Common, ToolsExtensions } from '../../common/utils/localize'; +import { IServiceContainer } from '../../ioc/types'; +import { AUTOPEP8_EXTENSION, BLACK_EXTENSION, IInstallFormatterPrompt } from './types'; + +const SHOW_FORMATTER_INSTALL_PROMPT_DONOTSHOW_KEY = 'showFormatterExtensionInstallPrompt'; + +export class SelectFormatterPrompt implements IInstallFormatterPrompt { + private shownThisSession = false; + + private readonly extensions: IExtensions; + + private readonly workspaceService: IWorkspaceService; + + constructor(private readonly serviceContainer: IServiceContainer) { + this.extensions = this.serviceContainer.get(IExtensions); + this.workspaceService = this.serviceContainer.get(IWorkspaceService); + } + + public async showInstallFormatterPrompt(resource?: Uri): Promise { + const experiment = this.serviceContainer.get(IExperimentService); + if (!(await experiment.inExperiment(ShowFormatterExtensionPrompt.experiment))) { + return; + } + + const persistFactory = this.serviceContainer.get(IPersistentStateFactory); + const promptState = persistFactory.createWorkspacePersistentState( + SHOW_FORMATTER_INSTALL_PROMPT_DONOTSHOW_KEY, + false, + ); + if (this.shownThisSession || promptState.value) { + return; + } + + const config = this.workspaceService.getConfiguration('python', resource); + const formatter = config.get('formatting.provider', 'none'); + + if (!['autopep8', 'black'].includes(formatter)) { + return; + } + + const black = this.extensions.getExtension(BLACK_EXTENSION); + const autopep8 = this.extensions.getExtension(AUTOPEP8_EXTENSION); + + const appShell: IApplicationShell = this.serviceContainer.get(IApplicationShell); + this.shownThisSession = true; + + if (formatter === 'black' && !black) { + const selection = await appShell.showInformationMessage( + ToolsExtensions.installBlackFormatterPrompt, + Common.install, + Common.doNotShowAgain, + ); + if (selection === Common.doNotShowAgain) { + await promptState.updateValue(true); + } else if (selection === Common.install) { + await this.installExtension(BLACK_EXTENSION); + } + } else if (formatter === 'autopep8' && !autopep8) { + const selection = await appShell.showInformationMessage( + ToolsExtensions.installAutopep8FormatterPrompt, + Common.install, + Common.doNotShowAgain, + ); + if (selection === Common.doNotShowAgain) { + await promptState.updateValue(true); + } else if (selection === Common.install) { + await this.installExtension(AUTOPEP8_EXTENSION); + } + } + } + + private async installExtension(extensionId: string, resource?: Uri): Promise { + const appEnv = this.serviceContainer.get(IApplicationEnvironment); + const commandManager = this.serviceContainer.get(ICommandManager); + await commandManager.executeCommand('workbench.extensions.installExtension', extensionId, { + installPreReleaseVersion: appEnv.extensionChannel === 'insiders', + }); + + const extension = this.extensions.getExtension(extensionId); + if (!extension) { + return; + } + + await extension.activate(); + + const config = this.workspaceService.getConfiguration('editor', resource); + const scope = this.workspaceService.getWorkspaceFolder(resource) + ? ConfigurationTarget.Workspace + : ConfigurationTarget.Global; + await config.update('defaultFormatter', extensionId, scope, true); + } +} diff --git a/src/client/providers/prompts/selectFormatterPrompt.ts b/src/client/providers/prompts/selectFormatterPrompt.ts new file mode 100644 index 000000000000..259b3789eec8 --- /dev/null +++ b/src/client/providers/prompts/selectFormatterPrompt.ts @@ -0,0 +1,92 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { ConfigurationTarget, Uri } from 'vscode'; +import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; +import { ShowFormatterExtensionPrompt } from '../../common/experiments/groups'; +import { IExperimentService, IExtensions, IPersistentStateFactory } from '../../common/types'; +import { Common, ToolsExtensions } from '../../common/utils/localize'; +import { IServiceContainer } from '../../ioc/types'; +import { traceVerbose } from '../../logging'; +import { AUTOPEP8_EXTENSION, BLACK_EXTENSION, ISelectFormatterPrompt } from './types'; + +const SELECT_FORMATTER_PROMPT_DONOTSHOW_KEY = 'showSelectFormatterExtensionPrompt'; + +export class SelectFormatterPrompt implements ISelectFormatterPrompt { + private shownThisSession = false; + + constructor(private readonly serviceContainer: IServiceContainer) {} + + public async showSelectFormatterPrompt(resource?: Uri): Promise { + const experiment = this.serviceContainer.get(IExperimentService); + if (!(await experiment.inExperiment(ShowFormatterExtensionPrompt.experiment))) { + return; + } + + const persistFactory = this.serviceContainer.get(IPersistentStateFactory); + const promptState = persistFactory.createWorkspacePersistentState( + SELECT_FORMATTER_PROMPT_DONOTSHOW_KEY, + false, + ); + if (this.shownThisSession || promptState.value) { + return; + } + + const extensions = this.serviceContainer.get(IExtensions); + const black = extensions.getExtension(BLACK_EXTENSION); + const autopep8 = extensions.getExtension(AUTOPEP8_EXTENSION); + if (!black && !autopep8) { + return; + } + + const workspaceService = this.serviceContainer.get(IWorkspaceService); + const scope = workspaceService.getWorkspaceFolder(resource) + ? ConfigurationTarget.Workspace + : ConfigurationTarget.Global; + const config = workspaceService.getConfiguration('editor', resource); + const defaultFormatter = config.get('defaultFormatter'); + if (defaultFormatter !== 'ms-python.python') { + traceVerbose(`Not showing select formatter prompt, 'editor.defaultFormatter' set to ${defaultFormatter}`); + return; + } + + this.shownThisSession = true; + const appShell = this.serviceContainer.get(IApplicationShell); + let selection: string | undefined; + + if (black && autopep8) { + selection = await appShell.showInformationMessage( + ToolsExtensions.selectMultipleFormattersPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ); + } else if (black) { + selection = await appShell.showInformationMessage( + ToolsExtensions.selectBlackFormatterPrompt, + Common.bannerLabelYes, + Common.doNotShowAgain, + ); + if (selection === Common.bannerLabelYes) { + selection = 'Black'; + } + } else if (autopep8) { + selection = await appShell.showInformationMessage( + ToolsExtensions.selectAutopep8FormatterPrompt, + Common.bannerLabelYes, + Common.doNotShowAgain, + ); + if (selection === Common.bannerLabelYes) { + selection = 'Autopep8'; + } + } + + if (selection === 'Black') { + await config.update('defaultFormatter', BLACK_EXTENSION, scope, true); + } else if (selection === 'Autopep8') { + await config.update('defaultFormatter', AUTOPEP8_EXTENSION, scope, true); + } else if (selection === Common.doNotShowAgain) { + await promptState.updateValue(true); + } + } +} diff --git a/src/client/providers/prompts/types.ts b/src/client/providers/prompts/types.ts new file mode 100644 index 000000000000..eeef6d12f206 --- /dev/null +++ b/src/client/providers/prompts/types.ts @@ -0,0 +1,13 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +export const BLACK_EXTENSION = 'ms-python.black-formatter'; +export const AUTOPEP8_EXTENSION = 'ms-python.autopep8'; + +export interface ISelectFormatterPrompt { + showSelectFormatterPrompt(): Promise; +} + +export interface IInstallFormatterPrompt { + showInstallFormatterPrompt(): Promise; +} From 18c058cf842b16eaf8a7bd6d3670fb4da13d8a9b Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 15 Mar 2023 15:26:34 -0700 Subject: [PATCH 2/5] Simplifying the prompt --- package-lock.json | 2 +- src/client/common/utils/localize.ts | 4 +- src/client/common/vscodeApis/extensionsApi.ts | 12 +- src/client/common/vscodeApis/workspaceApis.ts | 50 +++--- src/client/extensionActivation.ts | 5 +- .../prompts/installFormatterPrompt.ts | 150 ++++++++++-------- src/client/providers/prompts/promptUtils.ts | 44 +++++ .../prompts/selectFormatterPrompt.ts | 92 ----------- src/client/providers/prompts/types.ts | 4 - 9 files changed, 172 insertions(+), 191 deletions(-) create mode 100644 src/client/providers/prompts/promptUtils.ts delete mode 100644 src/client/providers/prompts/selectFormatterPrompt.ts diff --git a/package-lock.json b/package-lock.json index ef50646d7457..20e07c98ec63 100644 --- a/package-lock.json +++ b/package-lock.json @@ -129,7 +129,7 @@ "yargs": "^15.3.1" }, "engines": { - "vscode": "^1.75.0" + "vscode": "^1.76.0" } }, "node_modules/@azure/abort-controller": { diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 2ffe80118ffa..a7e63458be56 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -487,10 +487,10 @@ export namespace ToolsExtensions { ); export const installBlackFormatterPrompt = l10n.t( - 'You triggered formatting with Black, would you like to install the Black formatter extension?', + 'You triggered formatting with Black, would you like to install one of the formatter extensions? This will also set it as the default formatter for python.', ); export const installAutopep8FormatterPrompt = l10n.t( - 'You triggered formatting with Autopep8, would you like to install the Autopep8 extension?', + 'You triggered formatting with Autopep8, would you like to install one of the formatter extension? This will also set it as the default formatter for python.', ); } diff --git a/src/client/common/vscodeApis/extensionsApi.ts b/src/client/common/vscodeApis/extensionsApi.ts index 27e0657f0687..ece424847a16 100644 --- a/src/client/common/vscodeApis/extensionsApi.ts +++ b/src/client/common/vscodeApis/extensionsApi.ts @@ -3,15 +3,15 @@ import * as path from 'path'; import * as fs from 'fs-extra'; -import { Extension, extensions } from 'vscode'; +import * as vscode from 'vscode'; import { PVSC_EXTENSION_ID } from '../constants'; -export function getExtension(extensionId: string): Extension | undefined { - return extensions.getExtension(extensionId); +export function getExtension(extensionId: string): vscode.Extension | undefined { + return vscode.extensions.getExtension(extensionId); } export function isExtensionEnabled(extensionId: string): boolean { - return extensions.getExtension(extensionId) !== undefined; + return vscode.extensions.getExtension(extensionId) !== undefined; } export function isExtensionDisabled(extensionId: string): boolean { @@ -28,3 +28,7 @@ export function isExtensionDisabled(extensionId: string): boolean { } return false; } + +export function isInsider(): boolean { + return vscode.env.appName.includes('Insider'); +} diff --git a/src/client/common/vscodeApis/workspaceApis.ts b/src/client/common/vscodeApis/workspaceApis.ts index fda05e2477af..5db6752f7f9c 100644 --- a/src/client/common/vscodeApis/workspaceApis.ts +++ b/src/client/common/vscodeApis/workspaceApis.ts @@ -1,43 +1,45 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { - CancellationToken, - ConfigurationScope, - GlobPattern, - Uri, - workspace, - WorkspaceConfiguration, - WorkspaceEdit, - WorkspaceFolder, -} from 'vscode'; +import * as vscode from 'vscode'; import { Resource } from '../types'; -export function getWorkspaceFolders(): readonly WorkspaceFolder[] | undefined { - return workspace.workspaceFolders; +export function getWorkspaceFolders(): readonly vscode.WorkspaceFolder[] | undefined { + return vscode.workspace.workspaceFolders; } -export function getWorkspaceFolder(uri: Resource): WorkspaceFolder | undefined { - return uri ? workspace.getWorkspaceFolder(uri) : undefined; +export function getWorkspaceFolder(uri: Resource): vscode.WorkspaceFolder | undefined { + return uri ? vscode.workspace.getWorkspaceFolder(uri) : undefined; } export function getWorkspaceFolderPaths(): string[] { - return workspace.workspaceFolders?.map((w) => w.uri.fsPath) ?? []; + return vscode.workspace.workspaceFolders?.map((w) => w.uri.fsPath) ?? []; } -export function getConfiguration(section?: string, scope?: ConfigurationScope | null): WorkspaceConfiguration { - return workspace.getConfiguration(section, scope); +export function getConfiguration( + section?: string, + scope?: vscode.ConfigurationScope | null, +): vscode.WorkspaceConfiguration { + return vscode.workspace.getConfiguration(section, scope); } -export function applyEdit(edit: WorkspaceEdit): Thenable { - return workspace.applyEdit(edit); +export function applyEdit(edit: vscode.WorkspaceEdit): Thenable { + return vscode.workspace.applyEdit(edit); } export function findFiles( - include: GlobPattern, - exclude?: GlobPattern | null, + include: vscode.GlobPattern, + exclude?: vscode.GlobPattern | null, maxResults?: number, - token?: CancellationToken, -): Thenable { - return workspace.findFiles(include, exclude, maxResults, token); + token?: vscode.CancellationToken, +): Thenable { + return vscode.workspace.findFiles(include, exclude, maxResults, token); +} + +export function onDidSaveTextDocument( + listener: (e: vscode.TextDocument) => unknown, + thisArgs?: unknown, + disposables?: vscode.Disposable[], +): vscode.Disposable { + return vscode.workspace.onDidSaveTextDocument(listener, thisArgs, disposables); } diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 1f2e3e94cefe..af11da753242 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -62,6 +62,7 @@ import { WorkspaceService } from './common/application/workspace'; import { DynamicPythonDebugConfigurationService } from './debugger/extension/configuration/dynamicdebugConfigurationService'; import { registerCreateEnvironmentFeatures } from './pythonEnvironments/creation/createEnvApi'; import { IInterpreterQuickPick } from './interpreter/configuration/types'; +import { registerInstallFormatterPrompt } from './providers/prompts/installFormatterPrompt'; export async function activateComponents( // `ext` is passed to any extra activation funcs. @@ -206,13 +207,15 @@ async function activateLegacy(ext: ExtensionState): Promise { }); // register a dynamic configuration provider for 'python' debug type - context.subscriptions.push( + disposables.push( debug.registerDebugConfigurationProvider( DebuggerTypeName, serviceContainer.get(IDynamicDebugConfigurationService), DebugConfigurationProviderTriggerKind.Dynamic, ), ); + + registerInstallFormatterPrompt(serviceContainer); } } diff --git a/src/client/providers/prompts/installFormatterPrompt.ts b/src/client/providers/prompts/installFormatterPrompt.ts index 510c336ce6fe..72ec48e7a14b 100644 --- a/src/client/providers/prompts/installFormatterPrompt.ts +++ b/src/client/providers/prompts/installFormatterPrompt.ts @@ -1,104 +1,128 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { ConfigurationTarget, Uri } from 'vscode'; -import { - IApplicationEnvironment, - IApplicationShell, - ICommandManager, - IWorkspaceService, -} from '../../common/application/types'; -import { ShowFormatterExtensionPrompt } from '../../common/experiments/groups'; -import { IExperimentService, IExtensions, IPersistentStateFactory } from '../../common/types'; +import { Uri } from 'vscode'; +import { IDisposableRegistry } from '../../common/types'; import { Common, ToolsExtensions } from '../../common/utils/localize'; +import { isExtensionEnabled } from '../../common/vscodeApis/extensionsApi'; +import { showInformationMessage } from '../../common/vscodeApis/windowApis'; +import { getConfiguration, onDidSaveTextDocument } from '../../common/vscodeApis/workspaceApis'; import { IServiceContainer } from '../../ioc/types'; +import { + doNotShowPromptState, + inFormatterExtensionExperiment, + installFormatterExtension, + updateDefaultFormatter, +} from './promptUtils'; import { AUTOPEP8_EXTENSION, BLACK_EXTENSION, IInstallFormatterPrompt } from './types'; const SHOW_FORMATTER_INSTALL_PROMPT_DONOTSHOW_KEY = 'showFormatterExtensionInstallPrompt'; -export class SelectFormatterPrompt implements IInstallFormatterPrompt { +export class InstallFormatterPrompt implements IInstallFormatterPrompt { private shownThisSession = false; - private readonly extensions: IExtensions; - - private readonly workspaceService: IWorkspaceService; - - constructor(private readonly serviceContainer: IServiceContainer) { - this.extensions = this.serviceContainer.get(IExtensions); - this.workspaceService = this.serviceContainer.get(IWorkspaceService); - } + constructor(private readonly serviceContainer: IServiceContainer) {} public async showInstallFormatterPrompt(resource?: Uri): Promise { - const experiment = this.serviceContainer.get(IExperimentService); - if (!(await experiment.inExperiment(ShowFormatterExtensionPrompt.experiment))) { + if (!(await inFormatterExtensionExperiment(this.serviceContainer))) { return; } - const persistFactory = this.serviceContainer.get(IPersistentStateFactory); - const promptState = persistFactory.createWorkspacePersistentState( - SHOW_FORMATTER_INSTALL_PROMPT_DONOTSHOW_KEY, - false, - ); + const promptState = doNotShowPromptState(SHOW_FORMATTER_INSTALL_PROMPT_DONOTSHOW_KEY, this.serviceContainer); if (this.shownThisSession || promptState.value) { return; } - const config = this.workspaceService.getConfiguration('python', resource); + const config = getConfiguration('python', resource); const formatter = config.get('formatting.provider', 'none'); - if (!['autopep8', 'black'].includes(formatter)) { return; } - const black = this.extensions.getExtension(BLACK_EXTENSION); - const autopep8 = this.extensions.getExtension(AUTOPEP8_EXTENSION); + const editorConfig = getConfiguration('editor', { uri: resource, languageId: 'python' }); + const defaultFormatter = editorConfig.get('defaultFormatter', ''); + if ([BLACK_EXTENSION, AUTOPEP8_EXTENSION].includes(defaultFormatter)) { + return; + } - const appShell: IApplicationShell = this.serviceContainer.get(IApplicationShell); - this.shownThisSession = true; + const black = isExtensionEnabled(BLACK_EXTENSION); + const autopep8 = isExtensionEnabled(AUTOPEP8_EXTENSION); + + let selection: string | undefined; if (formatter === 'black' && !black) { - const selection = await appShell.showInformationMessage( + this.shownThisSession = true; + selection = await showInformationMessage( ToolsExtensions.installBlackFormatterPrompt, - Common.install, + 'Black', + 'Autopep8', Common.doNotShowAgain, ); - if (selection === Common.doNotShowAgain) { - await promptState.updateValue(true); - } else if (selection === Common.install) { - await this.installExtension(BLACK_EXTENSION); - } } else if (formatter === 'autopep8' && !autopep8) { - const selection = await appShell.showInformationMessage( + this.shownThisSession = true; + selection = await showInformationMessage( ToolsExtensions.installAutopep8FormatterPrompt, - Common.install, + 'Black', + 'Autopep8', Common.doNotShowAgain, ); - if (selection === Common.doNotShowAgain) { - await promptState.updateValue(true); - } else if (selection === Common.install) { - await this.installExtension(AUTOPEP8_EXTENSION); + } else if (black || autopep8) { + this.shownThisSession = true; + if (black && autopep8) { + selection = await showInformationMessage( + ToolsExtensions.selectMultipleFormattersPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ); + } else if (black) { + selection = await showInformationMessage( + ToolsExtensions.selectBlackFormatterPrompt, + Common.bannerLabelYes, + Common.doNotShowAgain, + ); + if (selection === Common.bannerLabelYes) { + selection = 'Black'; + } + } else if (autopep8) { + selection = await showInformationMessage( + ToolsExtensions.selectAutopep8FormatterPrompt, + Common.bannerLabelYes, + Common.doNotShowAgain, + ); + if (selection === Common.bannerLabelYes) { + selection = 'Autopep8'; + } } } - } - - private async installExtension(extensionId: string, resource?: Uri): Promise { - const appEnv = this.serviceContainer.get(IApplicationEnvironment); - const commandManager = this.serviceContainer.get(ICommandManager); - await commandManager.executeCommand('workbench.extensions.installExtension', extensionId, { - installPreReleaseVersion: appEnv.extensionChannel === 'insiders', - }); - const extension = this.extensions.getExtension(extensionId); - if (!extension) { - return; + if (selection === 'Black') { + if (black) { + await updateDefaultFormatter(BLACK_EXTENSION, resource); + } else { + await installFormatterExtension(BLACK_EXTENSION, resource); + } + } else if (selection === 'Autopep8') { + if (autopep8) { + await updateDefaultFormatter(AUTOPEP8_EXTENSION, resource); + } else { + await installFormatterExtension(AUTOPEP8_EXTENSION, resource); + } + } else if (selection === Common.doNotShowAgain) { + await promptState.updateValue(true); } - - await extension.activate(); - - const config = this.workspaceService.getConfiguration('editor', resource); - const scope = this.workspaceService.getWorkspaceFolder(resource) - ? ConfigurationTarget.Workspace - : ConfigurationTarget.Global; - await config.update('defaultFormatter', extensionId, scope, true); } } + +export function registerInstallFormatterPrompt(serviceContainer: IServiceContainer): void { + const disposables = serviceContainer.get(IDisposableRegistry); + const installFormatterPrompt = new InstallFormatterPrompt(serviceContainer); + disposables.push( + onDidSaveTextDocument(async (e) => { + const editorConfig = getConfiguration('editor', { uri: e.uri, languageId: 'python' }); + if (e.languageId === 'python' && editorConfig.get('formatOnSave')) { + await installFormatterPrompt.showInstallFormatterPrompt(e.uri); + } + }), + ); +} diff --git a/src/client/providers/prompts/promptUtils.ts b/src/client/providers/prompts/promptUtils.ts new file mode 100644 index 000000000000..0776bc1f03d3 --- /dev/null +++ b/src/client/providers/prompts/promptUtils.ts @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { ConfigurationTarget, Uri } from 'vscode'; +import { ShowFormatterExtensionPrompt } from '../../common/experiments/groups'; +import { IExperimentService, IPersistentState, IPersistentStateFactory } from '../../common/types'; +import { executeCommand } from '../../common/vscodeApis/commandApis'; +import { isInsider, getExtension } from '../../common/vscodeApis/extensionsApi'; +import { getConfiguration, getWorkspaceFolder } from '../../common/vscodeApis/workspaceApis'; +import { IServiceContainer } from '../../ioc/types'; + +export function inFormatterExtensionExperiment(serviceContainer: IServiceContainer): Promise { + const experiment = serviceContainer.get(IExperimentService); + return experiment.inExperiment(ShowFormatterExtensionPrompt.experiment); +} + +export function doNotShowPromptState(key: string, serviceContainer: IServiceContainer): IPersistentState { + const persistFactory = serviceContainer.get(IPersistentStateFactory); + const promptState = persistFactory.createWorkspacePersistentState(key, false); + return promptState; +} + +export async function updateDefaultFormatter(extensionId: string, resource?: Uri): Promise { + const scope = getWorkspaceFolder(resource) ? ConfigurationTarget.Workspace : ConfigurationTarget.Global; + + const config = getConfiguration('python', resource); + const editorConfig = getConfiguration('editor', { uri: resource, languageId: 'python' }); + await editorConfig.update('defaultFormatter', extensionId, scope, true); + await config.update('formatting.provider', 'none', scope); +} + +export async function installFormatterExtension(extensionId: string, resource?: Uri): Promise { + await executeCommand('workbench.extensions.installExtension', extensionId, { + installPreReleaseVersion: isInsider(), + }); + + const extension = getExtension(extensionId); + if (!extension) { + return; + } + + await extension.activate(); + await updateDefaultFormatter(extensionId, resource); +} diff --git a/src/client/providers/prompts/selectFormatterPrompt.ts b/src/client/providers/prompts/selectFormatterPrompt.ts deleted file mode 100644 index 259b3789eec8..000000000000 --- a/src/client/providers/prompts/selectFormatterPrompt.ts +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { ConfigurationTarget, Uri } from 'vscode'; -import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; -import { ShowFormatterExtensionPrompt } from '../../common/experiments/groups'; -import { IExperimentService, IExtensions, IPersistentStateFactory } from '../../common/types'; -import { Common, ToolsExtensions } from '../../common/utils/localize'; -import { IServiceContainer } from '../../ioc/types'; -import { traceVerbose } from '../../logging'; -import { AUTOPEP8_EXTENSION, BLACK_EXTENSION, ISelectFormatterPrompt } from './types'; - -const SELECT_FORMATTER_PROMPT_DONOTSHOW_KEY = 'showSelectFormatterExtensionPrompt'; - -export class SelectFormatterPrompt implements ISelectFormatterPrompt { - private shownThisSession = false; - - constructor(private readonly serviceContainer: IServiceContainer) {} - - public async showSelectFormatterPrompt(resource?: Uri): Promise { - const experiment = this.serviceContainer.get(IExperimentService); - if (!(await experiment.inExperiment(ShowFormatterExtensionPrompt.experiment))) { - return; - } - - const persistFactory = this.serviceContainer.get(IPersistentStateFactory); - const promptState = persistFactory.createWorkspacePersistentState( - SELECT_FORMATTER_PROMPT_DONOTSHOW_KEY, - false, - ); - if (this.shownThisSession || promptState.value) { - return; - } - - const extensions = this.serviceContainer.get(IExtensions); - const black = extensions.getExtension(BLACK_EXTENSION); - const autopep8 = extensions.getExtension(AUTOPEP8_EXTENSION); - if (!black && !autopep8) { - return; - } - - const workspaceService = this.serviceContainer.get(IWorkspaceService); - const scope = workspaceService.getWorkspaceFolder(resource) - ? ConfigurationTarget.Workspace - : ConfigurationTarget.Global; - const config = workspaceService.getConfiguration('editor', resource); - const defaultFormatter = config.get('defaultFormatter'); - if (defaultFormatter !== 'ms-python.python') { - traceVerbose(`Not showing select formatter prompt, 'editor.defaultFormatter' set to ${defaultFormatter}`); - return; - } - - this.shownThisSession = true; - const appShell = this.serviceContainer.get(IApplicationShell); - let selection: string | undefined; - - if (black && autopep8) { - selection = await appShell.showInformationMessage( - ToolsExtensions.selectMultipleFormattersPrompt, - 'Black', - 'Autopep8', - Common.doNotShowAgain, - ); - } else if (black) { - selection = await appShell.showInformationMessage( - ToolsExtensions.selectBlackFormatterPrompt, - Common.bannerLabelYes, - Common.doNotShowAgain, - ); - if (selection === Common.bannerLabelYes) { - selection = 'Black'; - } - } else if (autopep8) { - selection = await appShell.showInformationMessage( - ToolsExtensions.selectAutopep8FormatterPrompt, - Common.bannerLabelYes, - Common.doNotShowAgain, - ); - if (selection === Common.bannerLabelYes) { - selection = 'Autopep8'; - } - } - - if (selection === 'Black') { - await config.update('defaultFormatter', BLACK_EXTENSION, scope, true); - } else if (selection === 'Autopep8') { - await config.update('defaultFormatter', AUTOPEP8_EXTENSION, scope, true); - } else if (selection === Common.doNotShowAgain) { - await promptState.updateValue(true); - } - } -} diff --git a/src/client/providers/prompts/types.ts b/src/client/providers/prompts/types.ts index eeef6d12f206..47fead687cf5 100644 --- a/src/client/providers/prompts/types.ts +++ b/src/client/providers/prompts/types.ts @@ -4,10 +4,6 @@ export const BLACK_EXTENSION = 'ms-python.black-formatter'; export const AUTOPEP8_EXTENSION = 'ms-python.autopep8'; -export interface ISelectFormatterPrompt { - showSelectFormatterPrompt(): Promise; -} - export interface IInstallFormatterPrompt { showInstallFormatterPrompt(): Promise; } From bacc31b8a48eaef04c287475529d9427341c8332 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 15 Mar 2023 15:26:42 -0700 Subject: [PATCH 3/5] Adding tests --- .../installFormatterPrompt.unit.test.ts | 335 ++++++++++++++++++ 1 file changed, 335 insertions(+) create mode 100644 src/test/providers/prompt/installFormatterPrompt.unit.test.ts diff --git a/src/test/providers/prompt/installFormatterPrompt.unit.test.ts b/src/test/providers/prompt/installFormatterPrompt.unit.test.ts new file mode 100644 index 000000000000..65d540459e4c --- /dev/null +++ b/src/test/providers/prompt/installFormatterPrompt.unit.test.ts @@ -0,0 +1,335 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { assert } from 'chai'; +import * as sinon from 'sinon'; +import * as TypeMoq from 'typemoq'; +import { WorkspaceConfiguration } from 'vscode'; +import { IPersistentState } from '../../../client/common/types'; +import * as workspaceApis from '../../../client/common/vscodeApis/workspaceApis'; +import * as windowApis from '../../../client/common/vscodeApis/windowApis'; +import * as extensionsApi from '../../../client/common/vscodeApis/extensionsApi'; +import { IServiceContainer } from '../../../client/ioc/types'; +import { InstallFormatterPrompt } from '../../../client/providers/prompts/installFormatterPrompt'; +import * as promptUtils from '../../../client/providers/prompts/promptUtils'; +import { AUTOPEP8_EXTENSION, BLACK_EXTENSION, IInstallFormatterPrompt } from '../../../client/providers/prompts/types'; +import { Common, ToolsExtensions } from '../../../client/common/utils/localize'; + +suite('Formatter Extension prompt tests', () => { + let inFormatterExtensionExperimentStub: sinon.SinonStub; + let doNotShowPromptStateStub: sinon.SinonStub; + let prompt: IInstallFormatterPrompt; + let serviceContainer: TypeMoq.IMock; + let persistState: TypeMoq.IMock>; + let getConfigurationStub: sinon.SinonStub; + let isExtensionEnabledStub: sinon.SinonStub; + let pythonConfig: TypeMoq.IMock; + let editorConfig: TypeMoq.IMock; + let showInformationMessageStub: sinon.SinonStub; + let installFormatterExtensionStub: sinon.SinonStub; + let updateDefaultFormatterStub: sinon.SinonStub; + + setup(() => { + inFormatterExtensionExperimentStub = sinon.stub(promptUtils, 'inFormatterExtensionExperiment'); + inFormatterExtensionExperimentStub.resolves(true); + + doNotShowPromptStateStub = sinon.stub(promptUtils, 'doNotShowPromptState'); + persistState = TypeMoq.Mock.ofType>(); + doNotShowPromptStateStub.returns(persistState.object); + + getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration'); + pythonConfig = TypeMoq.Mock.ofType(); + editorConfig = TypeMoq.Mock.ofType(); + getConfigurationStub.callsFake((section: string) => { + if (section === 'python') { + return pythonConfig.object; + } + return editorConfig.object; + }); + isExtensionEnabledStub = sinon.stub(extensionsApi, 'isExtensionEnabled'); + showInformationMessageStub = sinon.stub(windowApis, 'showInformationMessage'); + installFormatterExtensionStub = sinon.stub(promptUtils, 'installFormatterExtension'); + updateDefaultFormatterStub = sinon.stub(promptUtils, 'updateDefaultFormatter'); + + serviceContainer = TypeMoq.Mock.ofType(); + + prompt = new InstallFormatterPrompt(serviceContainer.object); + }); + + teardown(() => { + sinon.restore(); + }); + + test('Not in experiment', async () => { + inFormatterExtensionExperimentStub.resolves(false); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue(doNotShowPromptStateStub.notCalled); + }); + + test('Do not show was set', async () => { + persistState.setup((p) => p.value).returns(() => true); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue(getConfigurationStub.notCalled); + }); + + test('Formatting provider is set to none', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'none'); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue(isExtensionEnabledStub.notCalled); + }); + + test('Formatting provider is set to yapf', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'yapf'); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue(isExtensionEnabledStub.notCalled); + }); + + test('Formatting provider is set to autopep8, and autopep8 extension is set as default formatter', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'autopep8'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => AUTOPEP8_EXTENSION); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue(isExtensionEnabledStub.notCalled); + }); + + test('Formatting provider is set to black, and black extension is set as default formatter', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'black'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => BLACK_EXTENSION); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue(isExtensionEnabledStub.notCalled); + }); + + test('Prompt: user selects do not show', async () => { + persistState.setup((p) => p.value).returns(() => false); + persistState + .setup((p) => p.updateValue(true)) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.atLeastOnce()); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'autopep8'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => ''); + isExtensionEnabledStub.returns(undefined); + + showInformationMessageStub.resolves(Common.doNotShowAgain); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue( + showInformationMessageStub.calledWith( + ToolsExtensions.installAutopep8FormatterPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ), + 'showInformationMessage should be called', + ); + persistState.verifyAll(); + }); + + test('Prompt (autopep8): user selects Autopep8', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'autopep8'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => ''); + isExtensionEnabledStub.returns(undefined); + + showInformationMessageStub.resolves('Autopep8'); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue( + showInformationMessageStub.calledWith( + ToolsExtensions.installAutopep8FormatterPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ), + 'showInformationMessage should be called', + ); + assert.isTrue( + installFormatterExtensionStub.calledWith(AUTOPEP8_EXTENSION, undefined), + 'installFormatterExtension should be called', + ); + }); + + test('Prompt (autopep8): user selects Black', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'autopep8'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => ''); + isExtensionEnabledStub.returns(undefined); + + showInformationMessageStub.resolves('Black'); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue( + showInformationMessageStub.calledWith( + ToolsExtensions.installAutopep8FormatterPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ), + 'showInformationMessage should be called', + ); + assert.isTrue( + installFormatterExtensionStub.calledWith(BLACK_EXTENSION, undefined), + 'installFormatterExtension should be called', + ); + }); + + test('Prompt (black): user selects Autopep8', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'black'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => ''); + isExtensionEnabledStub.returns(undefined); + + showInformationMessageStub.resolves('Autopep8'); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue( + showInformationMessageStub.calledWith( + ToolsExtensions.installBlackFormatterPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ), + 'showInformationMessage should be called', + ); + assert.isTrue( + installFormatterExtensionStub.calledWith(AUTOPEP8_EXTENSION, undefined), + 'installFormatterExtension should be called', + ); + }); + + test('Prompt (black): user selects Black', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'black'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => ''); + isExtensionEnabledStub.returns(undefined); + + showInformationMessageStub.resolves('Black'); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue( + showInformationMessageStub.calledWith( + ToolsExtensions.installBlackFormatterPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ), + 'showInformationMessage should be called', + ); + assert.isTrue( + installFormatterExtensionStub.calledWith(BLACK_EXTENSION, undefined), + 'installFormatterExtension should be called', + ); + }); + + test('Prompt: Black and Autopep8 installed user selects Black as default', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'black'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => ''); + isExtensionEnabledStub.returns({}); + + showInformationMessageStub.resolves('Black'); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue( + showInformationMessageStub.calledWith( + ToolsExtensions.selectMultipleFormattersPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ), + 'showInformationMessage should be called', + ); + assert.isTrue( + updateDefaultFormatterStub.calledWith(BLACK_EXTENSION, undefined), + 'updateDefaultFormatter should be called', + ); + }); + + test('Prompt: Black and Autopep8 installed user selects Autopep8 as default', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'autopep8'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => ''); + isExtensionEnabledStub.returns({}); + + showInformationMessageStub.resolves('Autopep8'); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue( + showInformationMessageStub.calledWith( + ToolsExtensions.selectMultipleFormattersPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ), + 'showInformationMessage should be called', + ); + assert.isTrue( + updateDefaultFormatterStub.calledWith(AUTOPEP8_EXTENSION, undefined), + 'updateDefaultFormatter should be called', + ); + }); + + test('Prompt: Black installed user selects Black as default', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'black'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => ''); + isExtensionEnabledStub.callsFake((extensionId) => { + if (extensionId === BLACK_EXTENSION) { + return {}; + } + return undefined; + }); + + showInformationMessageStub.resolves('Black'); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue( + showInformationMessageStub.calledWith( + ToolsExtensions.selectBlackFormatterPrompt, + Common.bannerLabelYes, + Common.doNotShowAgain, + ), + 'showInformationMessage should be called', + ); + assert.isTrue( + updateDefaultFormatterStub.calledWith(BLACK_EXTENSION, undefined), + 'updateDefaultFormatter should be called', + ); + }); + + test('Prompt: Autopep8 installed user selects Autopep8 as default', async () => { + persistState.setup((p) => p.value).returns(() => false); + pythonConfig.setup((p) => p.get('formatting.provider', TypeMoq.It.isAny())).returns(() => 'autopep8'); + editorConfig.setup((p) => p.get('defaultFormatter', TypeMoq.It.isAny())).returns(() => ''); + isExtensionEnabledStub.callsFake((extensionId) => { + if (extensionId === AUTOPEP8_EXTENSION) { + return {}; + } + return undefined; + }); + + showInformationMessageStub.resolves('Autopep8'); + + await prompt.showInstallFormatterPrompt(); + assert.isTrue( + showInformationMessageStub.calledWith( + ToolsExtensions.selectAutopep8FormatterPrompt, + Common.bannerLabelYes, + Common.doNotShowAgain, + ), + 'showInformationMessage should be called', + ); + assert.isTrue( + updateDefaultFormatterStub.calledWith(AUTOPEP8_EXTENSION, undefined), + 'updateDefaultFormatter should be called', + ); + }); +}); From 5ffb3f896b741eec06ce4b31da45999bc50a68d4 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 15 Mar 2023 15:57:43 -0700 Subject: [PATCH 4/5] Ensure installs are checked first. --- .../prompts/installFormatterPrompt.ts | 34 +++++++++---------- src/client/providers/prompts/promptUtils.ts | 8 +---- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/client/providers/prompts/installFormatterPrompt.ts b/src/client/providers/prompts/installFormatterPrompt.ts index 72ec48e7a14b..4c6fb0adda7a 100644 --- a/src/client/providers/prompts/installFormatterPrompt.ts +++ b/src/client/providers/prompts/installFormatterPrompt.ts @@ -50,23 +50,7 @@ export class InstallFormatterPrompt implements IInstallFormatterPrompt { let selection: string | undefined; - if (formatter === 'black' && !black) { - this.shownThisSession = true; - selection = await showInformationMessage( - ToolsExtensions.installBlackFormatterPrompt, - 'Black', - 'Autopep8', - Common.doNotShowAgain, - ); - } else if (formatter === 'autopep8' && !autopep8) { - this.shownThisSession = true; - selection = await showInformationMessage( - ToolsExtensions.installAutopep8FormatterPrompt, - 'Black', - 'Autopep8', - Common.doNotShowAgain, - ); - } else if (black || autopep8) { + if (black || autopep8) { this.shownThisSession = true; if (black && autopep8) { selection = await showInformationMessage( @@ -94,6 +78,22 @@ export class InstallFormatterPrompt implements IInstallFormatterPrompt { selection = 'Autopep8'; } } + } else if (formatter === 'black' && !black) { + this.shownThisSession = true; + selection = await showInformationMessage( + ToolsExtensions.installBlackFormatterPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ); + } else if (formatter === 'autopep8' && !autopep8) { + this.shownThisSession = true; + selection = await showInformationMessage( + ToolsExtensions.installAutopep8FormatterPrompt, + 'Black', + 'Autopep8', + Common.doNotShowAgain, + ); } if (selection === 'Black') { diff --git a/src/client/providers/prompts/promptUtils.ts b/src/client/providers/prompts/promptUtils.ts index 0776bc1f03d3..3a9f201c03b7 100644 --- a/src/client/providers/prompts/promptUtils.ts +++ b/src/client/providers/prompts/promptUtils.ts @@ -5,7 +5,7 @@ import { ConfigurationTarget, Uri } from 'vscode'; import { ShowFormatterExtensionPrompt } from '../../common/experiments/groups'; import { IExperimentService, IPersistentState, IPersistentStateFactory } from '../../common/types'; import { executeCommand } from '../../common/vscodeApis/commandApis'; -import { isInsider, getExtension } from '../../common/vscodeApis/extensionsApi'; +import { isInsider } from '../../common/vscodeApis/extensionsApi'; import { getConfiguration, getWorkspaceFolder } from '../../common/vscodeApis/workspaceApis'; import { IServiceContainer } from '../../ioc/types'; @@ -34,11 +34,5 @@ export async function installFormatterExtension(extensionId: string, resource?: installPreReleaseVersion: isInsider(), }); - const extension = getExtension(extensionId); - if (!extension) { - return; - } - - await extension.activate(); await updateDefaultFormatter(extensionId, resource); } From db04ec60ff78f3d74417c20421e97edfcac72e1e Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 16 Mar 2023 12:51:54 -0700 Subject: [PATCH 5/5] Address comments. --- src/client/common/utils/localize.ts | 4 ++-- src/client/providers/prompts/installFormatterPrompt.ts | 2 +- src/client/providers/prompts/promptUtils.ts | 4 ++-- src/test/providers/prompt/installFormatterPrompt.unit.test.ts | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index a7e63458be56..06f7e34e0742 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -487,10 +487,10 @@ export namespace ToolsExtensions { ); export const installBlackFormatterPrompt = l10n.t( - 'You triggered formatting with Black, would you like to install one of the formatter extensions? This will also set it as the default formatter for python.', + 'You triggered formatting with Black, would you like to install one of our new formatter extensions? This will also set it as the default formatter for Python.', ); export const installAutopep8FormatterPrompt = l10n.t( - 'You triggered formatting with Autopep8, would you like to install one of the formatter extension? This will also set it as the default formatter for python.', + 'You triggered formatting with Autopep8, would you like to install one of our new formatter extension? This will also set it as the default formatter for Python.', ); } diff --git a/src/client/providers/prompts/installFormatterPrompt.ts b/src/client/providers/prompts/installFormatterPrompt.ts index 4c6fb0adda7a..db23e130d1fd 100644 --- a/src/client/providers/prompts/installFormatterPrompt.ts +++ b/src/client/providers/prompts/installFormatterPrompt.ts @@ -24,7 +24,7 @@ export class InstallFormatterPrompt implements IInstallFormatterPrompt { constructor(private readonly serviceContainer: IServiceContainer) {} public async showInstallFormatterPrompt(resource?: Uri): Promise { - if (!(await inFormatterExtensionExperiment(this.serviceContainer))) { + if (!inFormatterExtensionExperiment(this.serviceContainer)) { return; } diff --git a/src/client/providers/prompts/promptUtils.ts b/src/client/providers/prompts/promptUtils.ts index 3a9f201c03b7..05b1b28f061a 100644 --- a/src/client/providers/prompts/promptUtils.ts +++ b/src/client/providers/prompts/promptUtils.ts @@ -9,9 +9,9 @@ import { isInsider } from '../../common/vscodeApis/extensionsApi'; import { getConfiguration, getWorkspaceFolder } from '../../common/vscodeApis/workspaceApis'; import { IServiceContainer } from '../../ioc/types'; -export function inFormatterExtensionExperiment(serviceContainer: IServiceContainer): Promise { +export function inFormatterExtensionExperiment(serviceContainer: IServiceContainer): boolean { const experiment = serviceContainer.get(IExperimentService); - return experiment.inExperiment(ShowFormatterExtensionPrompt.experiment); + return experiment.inExperimentSync(ShowFormatterExtensionPrompt.experiment); } export function doNotShowPromptState(key: string, serviceContainer: IServiceContainer): IPersistentState { diff --git a/src/test/providers/prompt/installFormatterPrompt.unit.test.ts b/src/test/providers/prompt/installFormatterPrompt.unit.test.ts index 65d540459e4c..fbd3a72d8cef 100644 --- a/src/test/providers/prompt/installFormatterPrompt.unit.test.ts +++ b/src/test/providers/prompt/installFormatterPrompt.unit.test.ts @@ -31,7 +31,7 @@ suite('Formatter Extension prompt tests', () => { setup(() => { inFormatterExtensionExperimentStub = sinon.stub(promptUtils, 'inFormatterExtensionExperiment'); - inFormatterExtensionExperimentStub.resolves(true); + inFormatterExtensionExperimentStub.returns(true); doNotShowPromptStateStub = sinon.stub(promptUtils, 'doNotShowPromptState'); persistState = TypeMoq.Mock.ofType>(); @@ -61,7 +61,7 @@ suite('Formatter Extension prompt tests', () => { }); test('Not in experiment', async () => { - inFormatterExtensionExperimentStub.resolves(false); + inFormatterExtensionExperimentStub.returns(false); await prompt.showInstallFormatterPrompt(); assert.isTrue(doNotShowPromptStateStub.notCalled);