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

Change "Pylance not installed" prompt to allow reverting to Jedi #15420

Merged
merged 4 commits into from
Feb 17, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,11 @@
"Pylance.proposePylanceMessage": "Try out a new faster, feature-rich language server for Python by Microsoft, Pylance! Install the extension now.",
"Pylance.tryItNow": "Try it now",
"Pylance.remindMeLater": "Remind me later",
"Pylance.installPylanceMessage": "Pylance extension is not installed. Click Yes to open Pylance installation page.",
"Pylance.pylanceNotInstalledMessage": "Pylance extension is not installed.",
"Pylance.pylanceInstalledReloadPromptMessage": "Pylance extension is now installed. Reload window to activate?",
"Pylance.pylanceRevertToJediPrompt": "The Pylance extension is not installed but the python.languageServer value is set to \"Pylance\". Would you like to install the Pylance extension to use Pylance, or revert back to Jedi?",
"Pylance.pylanceInstallPylance": "Install Pylance",
"Pylance.pylanceRevertToJedi": "Revert to Jedi",
jakebailey marked this conversation as resolved.
Show resolved Hide resolved
"Experiments.inGroup": "User belongs to experiment group '{0}'",
"Experiments.optedOutOf": "User opted out of experiment group '{0}'",
"Interpreters.RefreshingInterpreters": "Refreshing Python Interpreters",
Expand Down
1 change: 0 additions & 1 deletion package.nls.ru.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
"Pylance.proposePylanceMessage": "Попробуйте новый языковый сервер для Python от Microsoft: Pylance! Установите расширение Pylance.",
"Pylance.tryItNow": "Да, хочу",
"Pylance.remindMeLater": "Напомните позже",
"Pylance.installPylanceMessage": "Расширение Pylance не установлено. Нажмите Да чтобы открыть страницу установки Pylance.",
"Pylance.pylanceNotInstalledMessage": "Расширение Pylance не установлено.",
"Pylance.pylanceInstalledReloadPromptMessage": "Расширение Pylance установлено. Перезагрузить окно для его активации?",
"LanguageService.reloadAfterLanguageServerChange": "Пожалуйста, перезагрузите окно после смены типа языкового сервера."
Expand Down
1 change: 0 additions & 1 deletion package.nls.zh-cn.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
"Pylance.proposePylanceMessage": "试试微软新的更快的、功能丰富的语言服务器 Pylance! ",
"Pylance.tryItNow": "立即安装",
"Pylance.remindMeLater": "稍后提醒",
"Pylance.installPylanceMessage": "Pylance 扩展未安装。点击 \"\" 打开 Pylance 安装页面。",
"Pylance.pylanceNotInstalledMessage": "Pylance 扩展未安装。",
"Pylance.pylanceInstalledReloadPromptMessage": "Pylance 扩展未安装。重新加载窗口以激活?",
"Experiments.inGroup": "用户属于 '{0}' 实验组",
Expand Down
2 changes: 2 additions & 0 deletions src/client/activation/activationService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ export class LanguageServerExtensionActivationService
this.serviceContainer.get<IExtensions>(IExtensions),
this.serviceContainer.get<IApplicationShell>(IApplicationShell),
this.serviceContainer.get<ICommandManager>(ICommandManager),
this.serviceContainer.get<IWorkspaceService>(IWorkspaceService),
this.serviceContainer.get<IConfigurationService>(IConfigurationService),
);
disposables.push(this.languageServerChangeHandler);
}
Expand Down
2 changes: 1 addition & 1 deletion src/client/activation/common/activatorBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export abstract class LanguageServerActivatorBase implements ILanguageServerActi
protected resource?: Resource;
constructor(
protected readonly manager: ILanguageServerManager,
private readonly workspace: IWorkspaceService,
protected readonly workspace: IWorkspaceService,
protected readonly fs: IFileSystem,
protected readonly configurationService: IConfigurationService,
) {}
Expand Down
36 changes: 27 additions & 9 deletions src/client/activation/common/languageServerChangeHandler.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,43 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { Disposable } from 'vscode';
import { IApplicationShell, ICommandManager } from '../../common/application/types';
import { ConfigurationTarget, Disposable } from 'vscode';
import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types';
import { PYLANCE_EXTENSION_ID } from '../../common/constants';
import { IExtensions } from '../../common/types';
import { IConfigurationService, IExtensions } from '../../common/types';
import { createDeferred } from '../../common/utils/async';
import { Common, LanguageService, Pylance } from '../../common/utils/localize';
import { LanguageServerType } from '../types';

export async function promptForPylanceInstall(
appShell: IApplicationShell,
commandManager: ICommandManager,
workspace: IWorkspaceService,
configService: IConfigurationService,
): Promise<void> {
// If not installed, point user to Pylance at the store.
const response = await appShell.showWarningMessage(
Pylance.installPylanceMessage(),
Common.bannerLabelYes(),
Common.bannerLabelNo(),
Pylance.pylanceRevertToJediPrompt(),
Pylance.pylanceInstallPylance(),
Pylance.pylanceRevertToJedi(),
Pylance.remindMeLater(),
);

if (response === Common.bannerLabelYes()) {
if (response === Pylance.pylanceInstallPylance()) {
commandManager.executeCommand('extension.open', PYLANCE_EXTENSION_ID);
} else if (response === Pylance.pylanceRevertToJedi()) {
const inspection = workspace.getConfiguration('python').inspect<string>('languageServer');

let target: ConfigurationTarget | undefined;
if (inspection?.workspaceValue) {
target = ConfigurationTarget.Workspace;
} else if (inspection?.globalValue) {
target = ConfigurationTarget.Global;
}

if (target) {
await configService.updateSetting('languageServer', LanguageServerType.Jedi, undefined, target);
commandManager.executeCommand('workbench.action.reloadWindow');
}
}
}

Expand All @@ -37,6 +53,8 @@ export class LanguageServerChangeHandler implements Disposable {
private readonly extensions: IExtensions,
private readonly appShell: IApplicationShell,
private readonly commands: ICommandManager,
private readonly workspace: IWorkspaceService,
private readonly configService: IConfigurationService,
) {
this.pylanceInstalled = this.isPylanceInstalled();
this.disposables.push(
Expand Down Expand Up @@ -70,7 +88,7 @@ export class LanguageServerChangeHandler implements Disposable {
let response: string | undefined;
if (lsType === LanguageServerType.Node && !this.isPylanceInstalled()) {
// If not installed, point user to Pylance at the store.
await promptForPylanceInstall(this.appShell, this.commands);
await promptForPylanceInstall(this.appShell, this.commands, this.workspace, this.configService);
// At this point Pylance is not yet installed. Skip reload prompt
// since we are going to show it when Pylance becomes available.
} else {
Expand Down
7 changes: 6 additions & 1 deletion src/client/activation/node/activator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ export class NodeLanguageServerActivator extends LanguageServerActivatorBase {
// Pylance is not yet installed. Throw will cause activator to use Jedi
// temporarily. Language server installation tracker will prompt for window
// reload when Pylance becomes available.
await promptForPylanceInstall(this.appShell, this.commandManager);
await promptForPylanceInstall(
this.appShell,
this.commandManager,
this.workspace,
this.configurationService,
);
throw new Error(Pylance.pylanceNotInstalledMessage());
}
}
Expand Down
11 changes: 7 additions & 4 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,6 @@ export namespace Pylance {
export const tryItNow = localize('Pylance.tryItNow', 'Try it now');
export const remindMeLater = localize('Pylance.remindMeLater', 'Remind me later');

export const installPylanceMessage = localize(
'Pylance.installPylanceMessage',
'Pylance extension is not installed. Click Yes to open Pylance installation page.',
);
export const pylanceNotInstalledMessage = localize(
'Pylance.pylanceNotInstalledMessage',
'Pylance extension is not installed.',
Expand All @@ -126,6 +122,13 @@ export namespace Pylance {
'Pylance.pylanceInstalledReloadPromptMessage',
'Pylance extension is now installed. Reload window to activate?',
);

export const pylanceRevertToJediPrompt = localize(
'Pylance.pylanceRevertToJediPrompt',
'The Pylance extension is not installed but the python.languageServer value is set to "Pylance". Would you like to install the Pylance extension to use Pylance, or revert back to Jedi?',
);
export const pylanceInstallPylance = localize('Pylance.pylanceInstallPylance', 'Install Pylance');
export const pylanceRevertToJedi = localize('Pylance.pylanceRevertToJedi', 'Revert to Jedi');
}

export namespace Jupyter {
Expand Down
27 changes: 15 additions & 12 deletions src/test/activation/node/activator.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { PYLANCE_EXTENSION_ID } from '../../../client/common/constants';
import { FileSystem } from '../../../client/common/platform/fileSystem';
import { IFileSystem } from '../../../client/common/platform/types';
import { IConfigurationService, IExtensions, IPythonSettings } from '../../../client/common/types';
import { Common, Pylance } from '../../../client/common/utils/localize';
import { Pylance } from '../../../client/common/utils/localize';

suite('Pylance Language Server - Activator', () => {
let activator: NodeLanguageServerActivator;
Expand Down Expand Up @@ -92,20 +92,22 @@ suite('Pylance Language Server - Activator', () => {
test('When Pylance is not installed activator should show install prompt ', async () => {
when(
appShell.showWarningMessage(
Pylance.installPylanceMessage(),
Common.bannerLabelYes(),
Common.bannerLabelNo(),
Pylance.pylanceRevertToJediPrompt(),
Pylance.pylanceInstallPylance(),
Pylance.pylanceRevertToJedi(),
Pylance.remindMeLater(),
),
).thenReturn(Promise.resolve(Common.bannerLabelNo()));
).thenReturn(Promise.resolve(Pylance.remindMeLater()));

try {
await activator.start(undefined);
} catch {}
verify(
appShell.showWarningMessage(
Pylance.installPylanceMessage(),
Common.bannerLabelYes(),
Common.bannerLabelNo(),
Pylance.pylanceRevertToJediPrompt(),
Pylance.pylanceInstallPylance(),
Pylance.pylanceRevertToJedi(),
Pylance.remindMeLater(),
),
).once();
verify(commandManager.executeCommand('extension.open', PYLANCE_EXTENSION_ID)).never();
Expand All @@ -114,11 +116,12 @@ suite('Pylance Language Server - Activator', () => {
test('When Pylance is not installed activator should open Pylance install page if users clicks Yes', async () => {
when(
appShell.showWarningMessage(
Pylance.installPylanceMessage(),
Common.bannerLabelYes(),
Common.bannerLabelNo(),
Pylance.pylanceRevertToJediPrompt(),
Pylance.pylanceInstallPylance(),
Pylance.pylanceRevertToJedi(),
Pylance.remindMeLater(),
),
).thenReturn(Promise.resolve(Common.bannerLabelYes()));
).thenReturn(Promise.resolve(Pylance.pylanceInstallPylance()));

try {
await activator.start(undefined);
Expand Down
Loading