Skip to content

Commit 4c2655f

Browse files
author
Kartik Raj
authored
Displayed actionable message when LS is not supported (#3864)
* Displayed actionable message when LS is not supported * News entry * Updated tests to support changes * Added tests for buttons * Addressed reviews * Addressed new reviews * Injected message service * removed negation
1 parent fe92d5b commit 4c2655f

File tree

9 files changed

+210
-22
lines changed

9 files changed

+210
-22
lines changed

‎news/1 Enhancements/3634.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Display actionable message when LS is not supported

‎package.nls.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@
130130
"diagnostics.disableSourceMaps": "Disable Source Map Support",
131131
"diagnostics.warnBeforeEnablingSourceMaps": "Enabling source map support in the Python Extension will adversely impact performance of the extension.",
132132
"diagnostics.enableSourceMapsAndReloadVSC": "Enable and reload Window",
133+
"diagnostics.lsNotSupported": "Your operating system does not meet the minimum requirements of the Language Server. Reverting to the alternative, Jedi.",
133134
"DataScience.interruptKernel": "Interrupt iPython Kernel",
134135
"DataScience.exportingFormat": "Exporting {0}",
135136
"DataScience.exportCancel": "Cancel",
@@ -175,5 +176,5 @@
175176
"debug.attachRemoteHostValidationError": "Enter a valid host name or IP address",
176177
"UnitTests.testErrorDiagnosticMessage": "Error",
177178
"UnitTests.testFailDiagnosticMessage": "Fail",
178-
"UnitTests.testSkippedDiagnosticMessage": "Skipped"
179-
}
179+
"UnitTests.testSkippedDiagnosticMessage": "Skipped"
180+
}

‎src/client/activation/activationService.ts

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55

66
import { inject, injectable } from 'inversify';
77
import {
8-
ConfigurationChangeEvent, Disposable,
9-
OutputChannel, Uri
8+
ConfigurationChangeEvent,
9+
Disposable, OutputChannel, Uri
1010
} from 'vscode';
11+
import { LSNotSupportedDiagnosticServiceId } from '../application/diagnostics/checks/lsNotSupported';
12+
import { IDiagnosticsService } from '../application/diagnostics/types';
1113
import {
1214
IApplicationShell, ICommandManager,
1315
IWorkspaceService
@@ -23,8 +25,7 @@ import { sendTelemetryEvent } from '../telemetry';
2325
import { PYTHON_LANGUAGE_SERVER_PLATFORM_NOT_SUPPORTED } from '../telemetry/constants';
2426
import {
2527
ExtensionActivators, IExtensionActivationService,
26-
IExtensionActivator,
27-
ILanguageServerCompatibilityService
28+
IExtensionActivator
2829
} from './types';
2930

3031
const jediEnabledSetting: keyof IPythonSettings = 'jediEnabled';
@@ -36,13 +37,13 @@ export class ExtensionActivationService implements IExtensionActivationService,
3637
private readonly workspaceService: IWorkspaceService;
3738
private readonly output: OutputChannel;
3839
private readonly appShell: IApplicationShell;
40+
private readonly lsNotSupportedDiagnosticService: IDiagnosticsService;
3941

40-
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer,
41-
@inject(ILanguageServerCompatibilityService) private readonly lsCompatibility: ILanguageServerCompatibilityService) {
42+
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
4243
this.workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
4344
this.output = this.serviceContainer.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
4445
this.appShell = this.serviceContainer.get<IApplicationShell>(IApplicationShell);
45-
46+
this.lsNotSupportedDiagnosticService = this.serviceContainer.get<IDiagnosticsService>(IDiagnosticsService, LSNotSupportedDiagnosticServiceId);
4647
const disposables = serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
4748
disposables.push(this);
4849
disposables.push(this.workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this)));
@@ -54,10 +55,13 @@ export class ExtensionActivationService implements IExtensionActivationService,
5455
}
5556

5657
let jedi = this.useJedi();
57-
if (!jedi && !await this.lsCompatibility.isSupported()) {
58-
this.appShell.showWarningMessage('The Python Language Server is not supported on your platform.');
59-
sendTelemetryEvent(PYTHON_LANGUAGE_SERVER_PLATFORM_NOT_SUPPORTED);
60-
jedi = true;
58+
if (!jedi) {
59+
const diagnostic = await this.lsNotSupportedDiagnosticService.diagnose();
60+
this.lsNotSupportedDiagnosticService.handle(diagnostic).ignoreErrors();
61+
if (diagnostic.length){
62+
sendTelemetryEvent(PYTHON_LANGUAGE_SERVER_PLATFORM_NOT_SUPPORTED);
63+
jedi = true;
64+
}
6165
}
6266

6367
await this.logStartup(jedi);

‎src/client/application/diagnostics/checks/lsNotSupported.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import { inject, named } from 'inversify';
7+
import { DiagnosticSeverity } from 'vscode';
8+
import { ILanguageServerCompatibilityService } from '../../../activation/types';
9+
import { Diagnostics } from '../../../common/utils/localize';
10+
import { IServiceContainer } from '../../../ioc/types';
11+
import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
12+
import { IDiagnosticsCommandFactory } from '../commands/types';
13+
import { DiagnosticCodes } from '../constants';
14+
import { DiagnosticCommandPromptHandlerServiceId, MessageCommandPrompt } from '../promptHandler';
15+
import { DiagnosticScope, IDiagnostic, IDiagnosticHandlerService } from '../types';
16+
17+
export class LSNotSupportedDiagnostic extends BaseDiagnostic {
18+
constructor(message) {
19+
super(DiagnosticCodes.LSNotSupportedDiagnostic,
20+
message, DiagnosticSeverity.Warning, DiagnosticScope.Global);
21+
}
22+
}
23+
24+
export const LSNotSupportedDiagnosticServiceId = 'LSNotSupportedDiagnosticServiceId';
25+
26+
export class LSNotSupportedDiagnosticService extends BaseDiagnosticsService {
27+
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer,
28+
@inject(ILanguageServerCompatibilityService) private readonly lsCompatibility: ILanguageServerCompatibilityService,
29+
@inject(IDiagnosticHandlerService) @named(DiagnosticCommandPromptHandlerServiceId) protected readonly messageService: IDiagnosticHandlerService<MessageCommandPrompt>) {
30+
super([DiagnosticCodes.LSNotSupportedDiagnostic], serviceContainer);
31+
}
32+
public async diagnose(): Promise<IDiagnostic[]>{
33+
if (await this.lsCompatibility.isSupported()) {
34+
return [];
35+
} else{
36+
return [new LSNotSupportedDiagnostic(Diagnostics.lsNotSupported())];
37+
}
38+
}
39+
public async handle(diagnostics: IDiagnostic[]): Promise<void>{
40+
if (diagnostics.length === 0 || !this.canHandle(diagnostics[0])) {
41+
return;
42+
}
43+
const diagnostic = diagnostics[0];
44+
if (await this.filterService.shouldIgnoreDiagnostic(diagnostic.code)) {
45+
return;
46+
}
47+
const commandFactory = this.serviceContainer.get<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory);
48+
const options = [
49+
{
50+
prompt: 'More Info',
51+
command: commandFactory.createCommand(diagnostic, { type: 'launch', options: 'https://aka.ms/AA3qqka' })
52+
},
53+
{
54+
prompt: 'Do not show again',
55+
command: commandFactory.createCommand(diagnostic, { type: 'ignore', options: DiagnosticScope.Global })
56+
}
57+
];
58+
59+
await this.messageService.handle(diagnostic, { commandPrompts: options });
60+
}
61+
}

‎src/client/application/diagnostics/constants.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,6 @@ export enum DiagnosticCodes {
1111
MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic = 'MacInterpreterSelectedAndHaveOtherInterpretersDiagnostic',
1212
InvalidPythonPathInDebuggerDiagnostic = 'InvalidPythonPathInDebuggerDiagnostic',
1313
EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic = 'EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic',
14-
NoCurrentlySelectedPythonInterpreterDiagnostic = 'InvalidPythonInterpreterDiagnostic'
14+
NoCurrentlySelectedPythonInterpreterDiagnostic = 'InvalidPythonInterpreterDiagnostic',
15+
LSNotSupportedDiagnostic = 'LSNotSupportedDiagnostic'
1516
}

‎src/client/application/diagnostics/serviceRegistry.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { ApplicationDiagnostics } from './applicationDiagnostics';
99
import { EnvironmentPathVariableDiagnosticsService, EnvironmentPathVariableDiagnosticsServiceId } from './checks/envPathVariable';
1010
import { InvalidDebuggerTypeDiagnosticsService, InvalidDebuggerTypeDiagnosticsServiceId } from './checks/invalidDebuggerType';
1111
import { InvalidPythonPathInDebuggerService, InvalidPythonPathInDebuggerServiceId } from './checks/invalidPythonPathInDebugger';
12+
import { LSNotSupportedDiagnosticService, LSNotSupportedDiagnosticServiceId } from './checks/lsNotSupported';
1213
import { PowerShellActivationHackDiagnosticsService, PowerShellActivationHackDiagnosticsServiceId } from './checks/powerShellActivation';
1314
import { InvalidPythonInterpreterService, InvalidPythonInterpreterServiceId } from './checks/pythonInterpreter';
1415
import { DiagnosticsCommandFactory } from './commands/factory';
@@ -24,6 +25,7 @@ export function registerTypes(serviceManager: IServiceManager) {
2425
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, InvalidDebuggerTypeDiagnosticsService, InvalidDebuggerTypeDiagnosticsServiceId);
2526
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, InvalidPythonInterpreterService, InvalidPythonInterpreterServiceId);
2627
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, InvalidPythonPathInDebuggerService, InvalidPythonPathInDebuggerServiceId);
28+
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, LSNotSupportedDiagnosticService, LSNotSupportedDiagnosticServiceId);
2729
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, PowerShellActivationHackDiagnosticsService, PowerShellActivationHackDiagnosticsServiceId);
2830
serviceManager.addSingleton<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory, DiagnosticsCommandFactory);
2931
serviceManager.addSingleton<IApplicationDiagnostics>(IApplicationDiagnostics, ApplicationDiagnostics);

‎src/client/common/utils/localize.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export namespace Diagnostics {
1313
export const disableSourceMaps = localize('diagnostics.disableSourceMaps', 'Disable Source Map Support');
1414
export const warnBeforeEnablingSourceMaps = localize('diagnostics.warnBeforeEnablingSourceMaps', 'Enabling source map support in the Python Extension will adversely impact performance of the extension.');
1515
export const enableSourceMapsAndReloadVSC = localize('diagnostics.enableSourceMapsAndReloadVSC', 'Enable and reload Window.');
16+
export const lsNotSupported = localize('diagnostics.lsNotSupported', 'Your operating system does not meet the minimum requirements of the Language Server. Reverting to the alternative, Jedi.');
1617
}
1718

1819
export namespace Common {

‎src/test/activation/activationService.unit.test.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
ILanguageServerCompatibilityService,
1616
ILanguageServerFolderService
1717
} from '../../client/activation/types';
18+
import { LSNotSupportedDiagnosticServiceId } from '../../client/application/diagnostics/checks/lsNotSupported';
19+
import { IDiagnosticsService } from '../../client/application/diagnostics/types';
1820
import {
1921
IApplicationShell, ICommandManager,
2022
IWorkspaceService
@@ -36,6 +38,7 @@ suite('Activation - ActivationService', () => {
3638
let workspaceService: TypeMoq.IMock<IWorkspaceService>;
3739
let platformService: TypeMoq.IMock<IPlatformService>;
3840
let lanagueServerSupportedService: TypeMoq.IMock<ILanguageServerCompatibilityService>;
41+
let lsNotSupportedDiagnosticService: TypeMoq.IMock<IDiagnosticsService>;
3942
setup(() => {
4043
serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
4144
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
@@ -50,6 +53,7 @@ suite('Activation - ActivationService', () => {
5053
version: new SemVer('1.2.3')
5154
};
5255
lanagueServerSupportedService = TypeMoq.Mock.ofType<ILanguageServerCompatibilityService>();
56+
lsNotSupportedDiagnosticService = TypeMoq.Mock.ofType<IDiagnosticsService>();
5357
workspaceService.setup(w => w.hasWorkspaceFolders).returns(() => false);
5458
workspaceService.setup(w => w.workspaceFolders).returns(() => []);
5559
configService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object);
@@ -64,6 +68,7 @@ suite('Activation - ActivationService', () => {
6468
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ICommandManager))).returns(() => cmdManager.object);
6569
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPlatformService))).returns(() => platformService.object);
6670
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ILanguageServerFolderService))).returns(() => langFolderServiceMock.object);
71+
serviceContainer.setup(s => s.get(TypeMoq.It.isValue(IDiagnosticsService), TypeMoq.It.isValue(LSNotSupportedDiagnosticServiceId))).returns(() => lsNotSupportedDiagnosticService.object);
6772
});
6873

6974
async function testActivation(activationService: IExtensionActivationService, activator: TypeMoq.IMock<IExtensionActivator>, lsSupported: boolean = true) {
@@ -74,6 +79,14 @@ suite('Activation - ActivationService', () => {
7479
if (lsSupported && !jediIsEnabled) {
7580
activatorName = ExtensionActivators.DotNet;
7681
}
82+
let diagnostics;
83+
if (!lsSupported && !jediIsEnabled) {
84+
diagnostics = [TypeMoq.It.isAny()];
85+
} else{
86+
diagnostics = [];
87+
}
88+
lsNotSupportedDiagnosticService.setup(l => l.diagnose()).returns(() => diagnostics);
89+
lsNotSupportedDiagnosticService.setup(l => l.handle(TypeMoq.It.isValue(diagnostics))).returns(() => Promise.resolve());
7790
serviceContainer
7891
.setup(c => c.get(TypeMoq.It.isValue(IExtensionActivator), TypeMoq.It.isValue(activatorName)))
7992
.returns(() => activator.object)
@@ -89,15 +102,15 @@ suite('Activation - ActivationService', () => {
89102
lanagueServerSupportedService.setup(ls => ls.isSupported()).returns(() => Promise.resolve(true));
90103
pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled);
91104
const activator = TypeMoq.Mock.ofType<IExtensionActivator>();
92-
const activationService = new ExtensionActivationService(serviceContainer.object, lanagueServerSupportedService.object);
105+
const activationService = new ExtensionActivationService(serviceContainer.object);
93106

94107
await testActivation(activationService, activator, true);
95108
});
96109
test('LS is not supported', async () => {
97110
lanagueServerSupportedService.setup(ls => ls.isSupported()).returns(() => Promise.resolve(false));
98111
pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled);
99112
const activator = TypeMoq.Mock.ofType<IExtensionActivator>();
100-
const activationService = new ExtensionActivationService(serviceContainer.object, lanagueServerSupportedService.object);
113+
const activationService = new ExtensionActivationService(serviceContainer.object);
101114

102115
await testActivation(activationService, activator, false);
103116
});
@@ -106,15 +119,15 @@ suite('Activation - ActivationService', () => {
106119
lanagueServerSupportedService.setup(ls => ls.isSupported()).returns(() => Promise.resolve(true));
107120
pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled);
108121
const activator = TypeMoq.Mock.ofType<IExtensionActivator>();
109-
const activationService = new ExtensionActivationService(serviceContainer.object, lanagueServerSupportedService.object);
122+
const activationService = new ExtensionActivationService(serviceContainer.object);
110123

111124
await testActivation(activationService, activator);
112125
});
113126
test('Activatory must be deactivated', async () => {
114127
lanagueServerSupportedService.setup(ls => ls.isSupported()).returns(() => Promise.resolve(true));
115128
pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled);
116129
const activator = TypeMoq.Mock.ofType<IExtensionActivator>();
117-
const activationService = new ExtensionActivationService(serviceContainer.object, lanagueServerSupportedService.object);
130+
const activationService = new ExtensionActivationService(serviceContainer.object);
118131

119132
await testActivation(activationService, activator);
120133

@@ -137,7 +150,7 @@ suite('Activation - ActivationService', () => {
137150

138151
pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabledValueInSetting);
139152
const activator = TypeMoq.Mock.ofType<IExtensionActivator>();
140-
const activationService = new ExtensionActivationService(serviceContainer.object, lanagueServerSupportedService.object);
153+
const activationService = new ExtensionActivationService(serviceContainer.object);
141154

142155
workspaceService.verifyAll();
143156
await testActivation(activationService, activator);
@@ -172,7 +185,7 @@ suite('Activation - ActivationService', () => {
172185

173186
pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabledValueInSetting);
174187
const activator = TypeMoq.Mock.ofType<IExtensionActivator>();
175-
const activationService = new ExtensionActivationService(serviceContainer.object, lanagueServerSupportedService.object);
188+
const activationService = new ExtensionActivationService(serviceContainer.object);
176189

177190
workspaceService.verifyAll();
178191
await testActivation(activationService, activator);
@@ -206,7 +219,7 @@ suite('Activation - ActivationService', () => {
206219

207220
pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled);
208221
const activator = TypeMoq.Mock.ofType<IExtensionActivator>();
209-
const activationService = new ExtensionActivationService(serviceContainer.object, lanagueServerSupportedService.object);
222+
const activationService = new ExtensionActivationService(serviceContainer.object);
210223

211224
workspaceService.verifyAll();
212225
await testActivation(activationService, activator);
@@ -239,7 +252,7 @@ suite('Activation - ActivationService', () => {
239252

240253
pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled);
241254
const activator = TypeMoq.Mock.ofType<IExtensionActivator>();
242-
const activationService = new ExtensionActivationService(serviceContainer.object, lanagueServerSupportedService.object);
255+
const activationService = new ExtensionActivationService(serviceContainer.object);
243256

244257
workspaceService.verifyAll();
245258
await testActivation(activationService, activator);

0 commit comments

Comments
 (0)