Skip to content

Commit 5264b20

Browse files
jakebaileyjakebailey
authored
Switch to extension.open for opening the marketplace (microsoft#15321)
1 parent 5388c51 commit 5264b20

12 files changed

Lines changed: 72 additions & 73 deletions

File tree

‎news/2 Fixes/14264.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix marketplace links in popups opening a non-browser VS Code instance in Codespaces.

‎src/client/activation/activationService.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,7 @@ import { ConfigurationChangeEvent, Disposable, OutputChannel, Uri } from 'vscode
77

88
import { LSNotSupportedDiagnosticServiceId } from '../application/diagnostics/checks/lsNotSupported';
99
import { IDiagnosticsService } from '../application/diagnostics/types';
10-
import {
11-
IApplicationEnvironment,
12-
IApplicationShell,
13-
ICommandManager,
14-
IWorkspaceService,
15-
} from '../common/application/types';
10+
import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types';
1611
import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
1712
import { traceError } from '../common/logger';
1813
import {
@@ -90,7 +85,6 @@ export class LanguageServerExtensionActivationService
9085
this.getCurrentLanguageServerType(),
9186
this.serviceContainer.get<IExtensions>(IExtensions),
9287
this.serviceContainer.get<IApplicationShell>(IApplicationShell),
93-
this.serviceContainer.get<IApplicationEnvironment>(IApplicationEnvironment),
9488
this.serviceContainer.get<ICommandManager>(ICommandManager),
9589
);
9690
disposables.push(this.languageServerChangeHandler);

‎src/client/activation/common/languageServerChangeHandler.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,16 @@
22
// Licensed under the MIT License.
33

44
import { Disposable } from 'vscode';
5-
import { IApplicationEnvironment, IApplicationShell, ICommandManager } from '../../common/application/types';
5+
import { IApplicationShell, ICommandManager } from '../../common/application/types';
66
import { PYLANCE_EXTENSION_ID } from '../../common/constants';
77
import { IExtensions } from '../../common/types';
88
import { createDeferred } from '../../common/utils/async';
99
import { Common, LanguageService, Pylance } from '../../common/utils/localize';
10-
import { getPylanceExtensionUri } from '../../languageServices/proposeLanguageServerBanner';
1110
import { LanguageServerType } from '../types';
1211

1312
export async function promptForPylanceInstall(
1413
appShell: IApplicationShell,
15-
appEnv: IApplicationEnvironment,
14+
commandManager: ICommandManager,
1615
): Promise<void> {
1716
// If not installed, point user to Pylance at the store.
1817
const response = await appShell.showWarningMessage(
@@ -22,7 +21,7 @@ export async function promptForPylanceInstall(
2221
);
2322

2423
if (response === Common.bannerLabelYes()) {
25-
appShell.openUrl(getPylanceExtensionUri(appEnv));
24+
commandManager.executeCommand('extension.open', PYLANCE_EXTENSION_ID);
2625
}
2726
}
2827

@@ -37,7 +36,6 @@ export class LanguageServerChangeHandler implements Disposable {
3736
private currentLsType: LanguageServerType | undefined,
3837
private readonly extensions: IExtensions,
3938
private readonly appShell: IApplicationShell,
40-
private readonly appEnv: IApplicationEnvironment,
4139
private readonly commands: ICommandManager,
4240
) {
4341
this.pylanceInstalled = this.isPylanceInstalled();
@@ -72,7 +70,7 @@ export class LanguageServerChangeHandler implements Disposable {
7270
let response: string | undefined;
7371
if (lsType === LanguageServerType.Node && !this.isPylanceInstalled()) {
7472
// If not installed, point user to Pylance at the store.
75-
await promptForPylanceInstall(this.appShell, this.appEnv);
73+
await promptForPylanceInstall(this.appShell, this.commands);
7674
// At this point Pylance is not yet installed. Skip reload prompt
7775
// since we are going to show it when Pylance becomes available.
7876
} else {

‎src/client/activation/node/activator.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { CancellationToken, CompletionItem, ProviderResult } from 'vscode';
66

77
import ProtocolCompletionItem from 'vscode-languageclient/lib/common/protocolCompletionItem';
88
import { CompletionResolveRequest } from 'vscode-languageclient/node';
9-
import { IApplicationEnvironment, IApplicationShell, IWorkspaceService } from '../../common/application/types';
9+
import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types';
1010
import { PYLANCE_EXTENSION_ID } from '../../common/constants';
1111
import { IFileSystem } from '../../common/platform/types';
1212
import { IConfigurationService, IExtensions, Resource } from '../../common/types';
@@ -32,7 +32,7 @@ export class NodeLanguageServerActivator extends LanguageServerActivatorBase {
3232
@inject(IConfigurationService) configurationService: IConfigurationService,
3333
@inject(IExtensions) private readonly extensions: IExtensions,
3434
@inject(IApplicationShell) private readonly appShell: IApplicationShell,
35-
@inject(IApplicationEnvironment) private readonly appEnv: IApplicationEnvironment,
35+
@inject(ICommandManager) readonly commandManager: ICommandManager,
3636
) {
3737
super(manager, workspace, fs, configurationService);
3838
}
@@ -47,7 +47,7 @@ export class NodeLanguageServerActivator extends LanguageServerActivatorBase {
4747
// Pylance is not yet installed. Throw will cause activator to use Jedi
4848
// temporarily. Language server installation tracker will prompt for window
4949
// reload when Pylance becomes available.
50-
await promptForPylanceInstall(this.appShell, this.appEnv);
50+
await promptForPylanceInstall(this.appShell, this.commandManager);
5151
throw new Error(Pylance.pylanceNotInstalledMessage());
5252
}
5353
}

‎src/client/common/application/commandManager.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export class CommandManager implements ICommandManager {
7474
U extends ICommandNameArgumentTypeMapping[E]
7575
>(command: E, ...rest: U): Thenable<T | undefined> {
7676
if (command.includes('jupyter') && !this.jupyterExtensionDependencyManager.isJupyterExtensionInstalled) {
77-
return this.jupyterExtensionDependencyManager.installJupyterExtension();
77+
return this.jupyterExtensionDependencyManager.installJupyterExtension(this);
7878
} else {
7979
return commands.executeCommand<T>(command, ...rest);
8080
}

‎src/client/common/application/commands.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
8484
['workbench.action.files.save']: [Uri];
8585
['jupyter.opennotebook']: [undefined | Uri, undefined | CommandSource];
8686
['jupyter.runallcells']: [Uri];
87+
['extension.open']: [string];
8788
[Commands.GetSelectedInterpreterPath]: [{ workspaceFolder: string } | string[]];
8889
[Commands.Build_Workspace_Symbols]: [boolean, CancellationToken];
8990
[Commands.Sort_Imports]: [undefined, Uri];

‎src/client/common/application/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ export interface ICommandManager {
503503
export const IJupyterExtensionDependencyManager = Symbol('IJupyterExtensionDependencyManager');
504504
export interface IJupyterExtensionDependencyManager {
505505
readonly isJupyterExtensionInstalled: boolean;
506-
installJupyterExtension(): Promise<undefined>;
506+
installJupyterExtension(commandManager: ICommandManager): Promise<undefined>;
507507
}
508508

509509
export const IDocumentManager = Symbol('IDocumentManager');

‎src/client/jupyter/jupyterExtensionDependencyManager.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
import { inject, injectable } from 'inversify';
2-
import {
3-
IApplicationEnvironment,
4-
IApplicationShell,
5-
IJupyterExtensionDependencyManager,
6-
} from '../common/application/types';
2+
import { IApplicationShell, ICommandManager, IJupyterExtensionDependencyManager } from '../common/application/types';
73
import { JUPYTER_EXTENSION_ID } from '../common/constants';
84
import { IExtensions } from '../common/types';
95
import { Common, Jupyter } from '../common/utils/localize';
@@ -13,19 +9,18 @@ export class JupyterExtensionDependencyManager implements IJupyterExtensionDepen
139
constructor(
1410
@inject(IExtensions) private extensions: IExtensions,
1511
@inject(IApplicationShell) private appShell: IApplicationShell,
16-
@inject(IApplicationEnvironment) private appEnv: IApplicationEnvironment,
1712
) {}
1813

1914
public get isJupyterExtensionInstalled(): boolean {
2015
return this.extensions.getExtension(JUPYTER_EXTENSION_ID) !== undefined;
2116
}
2217

23-
public async installJupyterExtension(): Promise<undefined> {
18+
public async installJupyterExtension(commandManager: ICommandManager): Promise<undefined> {
2419
const yes = Common.bannerLabelYes();
2520
const no = Common.bannerLabelNo();
2621
const answer = await this.appShell.showErrorMessage(Jupyter.jupyterExtensionRequired(), yes, no);
2722
if (answer === yes) {
28-
this.appShell.openUrl(`${this.appEnv.uriScheme}:extension/${JUPYTER_EXTENSION_ID}`);
23+
commandManager.executeCommand('extension.open', JUPYTER_EXTENSION_ID);
2924
}
3025
return undefined;
3126
}

‎src/client/languageServices/proposeLanguageServerBanner.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { inject, injectable } from 'inversify';
77
import { LanguageServerType } from '../activation/types';
8-
import { IApplicationEnvironment, IApplicationShell } from '../common/application/types';
8+
import { IApplicationShell, ICommandManager } from '../common/application/types';
99
import { PYLANCE_EXTENSION_ID } from '../common/constants';
1010
import { TryPylance } from '../common/experiments/groups';
1111
import '../common/extensions';
@@ -20,10 +20,6 @@ import { Common, Pylance } from '../common/utils/localize';
2020
import { sendTelemetryEvent } from '../telemetry';
2121
import { EventName } from '../telemetry/constants';
2222

23-
export function getPylanceExtensionUri(appEnv: IApplicationEnvironment): string {
24-
return `${appEnv.uriScheme}:extension/${PYLANCE_EXTENSION_ID}`;
25-
}
26-
2723
// persistent state names, exported to make use of in testing
2824
export enum ProposeLSStateKeys {
2925
ShowBanner = 'TryPylanceBanner',
@@ -42,7 +38,7 @@ export class ProposePylanceBanner implements IPythonExtensionBanner {
4238

4339
constructor(
4440
@inject(IApplicationShell) private appShell: IApplicationShell,
45-
@inject(IApplicationEnvironment) private appEnv: IApplicationEnvironment,
41+
@inject(ICommandManager) readonly commandManager: ICommandManager,
4642
@inject(IPersistentStateFactory) private persistentState: IPersistentStateFactory,
4743
@inject(IConfigurationService) private configuration: IConfigurationService,
4844
@inject(IExperimentService) private experiments: IExperimentService,
@@ -73,7 +69,7 @@ export class ProposePylanceBanner implements IPythonExtensionBanner {
7369

7470
let userAction: string;
7571
if (response === Pylance.tryItNow()) {
76-
this.appShell.openUrl(getPylanceExtensionUri(this.appEnv));
72+
this.commandManager.executeCommand('extension.open', PYLANCE_EXTENSION_ID);
7773
userAction = 'yes';
7874
await this.disable();
7975
} else if (response === Common.bannerLabelNo()) {

‎src/test/activation/node/activator.unit.test.ts‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,7 @@ import { EventEmitter, Extension, Uri } from 'vscode';
99
import { NodeLanguageServerActivator } from '../../../client/activation/node/activator';
1010
import { NodeLanguageServerManager } from '../../../client/activation/node/manager';
1111
import { ILanguageServerManager } from '../../../client/activation/types';
12-
import {
13-
IApplicationEnvironment,
14-
IApplicationShell,
15-
IWorkspaceService,
16-
} from '../../../client/common/application/types';
12+
import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../client/common/application/types';
1713
import { WorkspaceService } from '../../../client/common/application/workspace';
1814
import { PythonSettings } from '../../../client/common/configSettings';
1915
import { ConfigurationService } from '../../../client/common/configuration/service';
@@ -32,7 +28,7 @@ suite('Pylance Language Server - Activator', () => {
3228
let settings: IPythonSettings;
3329
let extensions: IExtensions;
3430
let appShell: IApplicationShell;
35-
let appEnv: IApplicationEnvironment;
31+
let commandManager: ICommandManager;
3632
let extensionsChangedEvent: EventEmitter<void>;
3733

3834
let pylanceExtension: Extension<any>;
@@ -44,12 +40,10 @@ suite('Pylance Language Server - Activator', () => {
4440
settings = mock(PythonSettings);
4541
extensions = mock<IExtensions>();
4642
appShell = mock<IApplicationShell>();
47-
appEnv = mock<IApplicationEnvironment>();
48-
when(appEnv.uriScheme).thenReturn('scheme');
43+
commandManager = mock<ICommandManager>();
4944

5045
pylanceExtension = mock<Extension<any>>();
5146
when(configuration.getSettings(anything())).thenReturn(instance(settings));
52-
when(appEnv.uriScheme).thenReturn('scheme');
5347

5448
extensionsChangedEvent = new EventEmitter<void>();
5549
when(extensions.onDidChange).thenReturn(extensionsChangedEvent.event);
@@ -61,7 +55,7 @@ suite('Pylance Language Server - Activator', () => {
6155
instance(configuration),
6256
instance(extensions),
6357
instance(appShell),
64-
instance(appEnv),
58+
instance(commandManager),
6559
);
6660
});
6761
teardown(() => {
@@ -114,7 +108,7 @@ suite('Pylance Language Server - Activator', () => {
114108
Common.bannerLabelNo(),
115109
),
116110
).once();
117-
verify(appShell.openUrl(`scheme:extension/${PYLANCE_EXTENSION_ID}`)).never();
111+
verify(commandManager.executeCommand('extension.open', PYLANCE_EXTENSION_ID)).never();
118112
});
119113

120114
test('When Pylance is not installed activator should open Pylance install page if users clicks Yes', async () => {
@@ -129,7 +123,7 @@ suite('Pylance Language Server - Activator', () => {
129123
try {
130124
await activator.start(undefined);
131125
} catch {}
132-
verify(appShell.openUrl(`scheme:extension/${PYLANCE_EXTENSION_ID}`)).once();
126+
verify(commandManager.executeCommand('extension.open', PYLANCE_EXTENSION_ID)).once();
133127
});
134128

135129
test('Activator should throw if Pylance is not installed', async () => {

0 commit comments

Comments
 (0)