From b3d93459d77de0f5c38be4565b45fdec758e4d2a Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 9 Jun 2021 07:36:00 -0700 Subject: [PATCH 1/8] Terminal profile contribution feedback Part of #120369 --- src/vs/vscode.proposed.d.ts | 18 ++++++++++++++++-- .../api/common/extHostTerminalService.ts | 10 +++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 472e6340889..ec68a958961 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -903,12 +903,26 @@ declare module 'vscode' { export function registerTerminalProfileProvider(id: string, provider: TerminalProfileProvider): Disposable; } + /** + * Provides a terminal profile for the contributed terminal profile when launched via the UI or + * command. + */ export interface TerminalProfileProvider { /** - * Provide terminal profile options for the requested terminal. + * Provide the terminal profile. * @param token A cancellation token that indicates the result is no longer needed. */ - provideProfileOptions(token: CancellationToken): ProviderResult; + provideTerminalProfile(token: CancellationToken): ProviderResult; + } + + /** + * A terminal profile defines how a termianl will be launched. + */ + export interface TerminalProfile { + /** + * The options that the terminal will launch with. + */ + options: TerminalOptions | ExtensionTerminalOptions; } //#endregion diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 23b91c7f505..3046587b253 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -598,18 +598,18 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I public async $createContributedProfileTerminal(id: string, isSplitTerminal: boolean): Promise { const token = new CancellationTokenSource().token; - const options = await this._profileProviders.get(id)?.provideProfileOptions(token); + const profile = await this._profileProviders.get(id)?.provideTerminalProfile(token); if (token.isCancellationRequested) { return; } - if (!options) { + if (!profile) { throw new Error(`No terminal profile options provided for id "${id}"`); } - if ('pty' in options) { - this.createExtensionTerminal(options, { isSplitTerminal }); + if ('pty' in profile.options) { + this.createExtensionTerminal(profile.options, { isSplitTerminal }); return; } - this.createTerminalFromOptions(options, { isSplitTerminal }); + this.createTerminalFromOptions(profile.options, { isSplitTerminal }); } public async $provideLinks(terminalId: number, line: string): Promise { From a0ef27405ce447a338c1ab5ca548ae153ab64fef Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 9 Jun 2021 07:47:03 -0700 Subject: [PATCH 2/8] Finalize terminal profiles contribution Fixes #120369 --- src/vs/vscode.d.ts | 28 ++++++++++++++++++++++++++++ src/vs/vscode.proposed.d.ts | 35 ----------------------------------- 2 files changed, 28 insertions(+), 35 deletions(-) diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index 2939fa91371..5369c742f31 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -5813,6 +5813,28 @@ declare module 'vscode' { tooltip?: string; } + /** + * Provides a terminal profile for the contributed terminal profile when launched via the UI or + * command. + */ + export interface TerminalProfileProvider { + /** + * Provide the terminal profile. + * @param token A cancellation token that indicates the result is no longer needed. + */ + provideTerminalProfile(token: CancellationToken): ProviderResult; + } + + /** + * A terminal profile defines how a termianl will be launched. + */ + export interface TerminalProfile { + /** + * The options that the terminal will launch with. + */ + options: TerminalOptions | ExtensionTerminalOptions; + } + /** * A file decoration represents metadata that can be rendered with a file. */ @@ -8994,6 +9016,12 @@ declare module 'vscode' { */ export function registerTerminalLinkProvider(provider: TerminalLinkProvider): Disposable; + /** + * Registers a provider for a contributed terminal profile. + * @param id The ID of the contributed terminal profile. + * @param provider The terminal profile provider. + */ + export function registerTerminalProfileProvider(id: string, provider: TerminalProfileProvider): Disposable; /** * Register a file decoration provider. * diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index ec68a958961..9ec3ab217e2 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -892,41 +892,6 @@ declare module 'vscode' { //#endregion - //#region Terminal profile provider https://github.com/microsoft/vscode/issues/120369 - - export namespace window { - /** - * Registers a provider for a contributed terminal profile. - * @param id The ID of the contributed terminal profile. - * @param provider The terminal profile provider. - */ - export function registerTerminalProfileProvider(id: string, provider: TerminalProfileProvider): Disposable; - } - - /** - * Provides a terminal profile for the contributed terminal profile when launched via the UI or - * command. - */ - export interface TerminalProfileProvider { - /** - * Provide the terminal profile. - * @param token A cancellation token that indicates the result is no longer needed. - */ - provideTerminalProfile(token: CancellationToken): ProviderResult; - } - - /** - * A terminal profile defines how a termianl will be launched. - */ - export interface TerminalProfile { - /** - * The options that the terminal will launch with. - */ - options: TerminalOptions | ExtensionTerminalOptions; - } - - //#endregion - // eslint-disable-next-line vscode-dts-region-comments //#region @jrieken -> exclusive document filters From d42985859e5a6f5153d837d42ca75ffcfc3e08ae Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 9 Jun 2021 07:49:16 -0700 Subject: [PATCH 3/8] Remove proposed API check --- .../contrib/terminal/common/terminalExtensionPoints.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts b/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts index c9ed249caae..89d198c77aa 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts @@ -47,7 +47,7 @@ export class TerminalContributionService implements ITerminalContributionService return e; }) || []; })); - this._terminalProfiles = flatten(contributions.filter(c => c.description.enableProposedApi).map(c => { + this._terminalProfiles = flatten(contributions.map(c => { return c.value?.profiles?.map(e => { // Only support $(id) for now, without that it should point to a path to be // consistent with other icon APIs From 04fef4e7a427945b193e6df4d52bba60a16fca2b Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 9 Jun 2021 07:51:10 -0700 Subject: [PATCH 4/8] Remove old terminal.types contribution --- .../terminal/browser/terminalActions.ts | 7 ------ .../terminal/browser/terminalService.ts | 25 ++----------------- .../contrib/terminal/browser/terminalView.ts | 4 --- .../contrib/terminal/common/terminal.ts | 8 ------ .../common/terminalExtensionPoints.ts | 22 +--------------- 5 files changed, 3 insertions(+), 63 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalActions.ts b/src/vs/workbench/contrib/terminal/browser/terminalActions.ts index 53ca5cc50c7..188d5da6875 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalActions.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalActions.ts @@ -33,7 +33,6 @@ import { FindInFilesCommand, IFindInFilesArgs } from 'vs/workbench/contrib/searc import { Direction, IRemoteTerminalService, ITerminalInstance, ITerminalInstanceService, ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal'; import { TerminalQuickAccessProvider } from 'vs/workbench/contrib/terminal/browser/terminalQuickAccess'; import { IRemoteTerminalAttachTarget, ITerminalConfigHelper, KEYBINDING_CONTEXT_TERMINAL_A11Y_TREE_FOCUS, KEYBINDING_CONTEXT_TERMINAL_ALT_BUFFER_ACTIVE, KEYBINDING_CONTEXT_TERMINAL_FIND_FOCUSED, KEYBINDING_CONTEXT_TERMINAL_FIND_NOT_VISIBLE, KEYBINDING_CONTEXT_TERMINAL_FIND_VISIBLE, KEYBINDING_CONTEXT_TERMINAL_FOCUS, KEYBINDING_CONTEXT_TERMINAL_IS_OPEN, KEYBINDING_CONTEXT_TERMINAL_PROCESS_SUPPORTED, KEYBINDING_CONTEXT_TERMINAL_TABS_FOCUS, KEYBINDING_CONTEXT_TERMINAL_TABS_SINGULAR_SELECTION, KEYBINDING_CONTEXT_TERMINAL_TEXT_SELECTED, TERMINAL_ACTION_CATEGORY, TerminalCommandId, TERMINAL_VIEW_ID } from 'vs/workbench/contrib/terminal/common/terminal'; -import { ITerminalContributionService } from 'vs/workbench/contrib/terminal/common/terminalExtensionPoints'; import { terminalStrings } from 'vs/workbench/contrib/terminal/common/terminalStrings'; import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; import { IHistoryService } from 'vs/workbench/services/history/common/history'; @@ -1798,8 +1797,6 @@ export function registerTerminalActions() { } async run(accessor: ServicesAccessor, item?: string) { const terminalService = accessor.get(ITerminalService); - const terminalContributionService = accessor.get(ITerminalContributionService); - const commandService = accessor.get(ICommandService); if (!item || !item.split) { return Promise.resolve(null); } @@ -1816,10 +1813,6 @@ export function registerTerminalActions() { terminalService.setActiveGroupByIndex(Number(indexMatches[1]) - 1); return terminalService.showPanel(true); } - const customType = terminalContributionService.terminalTypes.find(t => t.title === item); - if (customType) { - return commandService.executeCommand(customType.command); - } const quickSelectProfiles = terminalService.availableProfiles; diff --git a/src/vs/workbench/contrib/terminal/browser/terminalService.ts b/src/vs/workbench/contrib/terminal/browser/terminalService.ts index fc6fe60a52a..4069a5b1cbf 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalService.ts @@ -25,7 +25,7 @@ import { TerminalConfigHelper } from 'vs/workbench/contrib/terminal/browser/term import { TerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminalInstance'; import { TerminalGroup } from 'vs/workbench/contrib/terminal/browser/terminalGroup'; import { TerminalViewPane } from 'vs/workbench/contrib/terminal/browser/terminalView'; -import { IRemoteTerminalAttachTarget, IStartExtensionTerminalRequest, ITerminalConfigHelper, ITerminalProcessExtHostProxy, KEYBINDING_CONTEXT_TERMINAL_ALT_BUFFER_ACTIVE, KEYBINDING_CONTEXT_TERMINAL_FOCUS, KEYBINDING_CONTEXT_TERMINAL_IS_OPEN, KEYBINDING_CONTEXT_TERMINAL_PROCESS_SUPPORTED, KEYBINDING_CONTEXT_TERMINAL_SHELL_TYPE, TERMINAL_VIEW_ID, KEYBINDING_CONTEXT_TERMINAL_COUNT, ITerminalTypeContribution, KEYBINDING_CONTEXT_TERMINAL_TABS_MOUSE, KEYBINDING_CONTEXT_TERMINAL_GROUP_COUNT, ITerminalProfileContribution } from 'vs/workbench/contrib/terminal/common/terminal'; +import { IRemoteTerminalAttachTarget, IStartExtensionTerminalRequest, ITerminalConfigHelper, ITerminalProcessExtHostProxy, KEYBINDING_CONTEXT_TERMINAL_ALT_BUFFER_ACTIVE, KEYBINDING_CONTEXT_TERMINAL_FOCUS, KEYBINDING_CONTEXT_TERMINAL_IS_OPEN, KEYBINDING_CONTEXT_TERMINAL_PROCESS_SUPPORTED, KEYBINDING_CONTEXT_TERMINAL_SHELL_TYPE, TERMINAL_VIEW_ID, KEYBINDING_CONTEXT_TERMINAL_COUNT, KEYBINDING_CONTEXT_TERMINAL_TABS_MOUSE, KEYBINDING_CONTEXT_TERMINAL_GROUP_COUNT, ITerminalProfileContribution } from 'vs/workbench/contrib/terminal/common/terminal'; import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService'; import { ILifecycleService, ShutdownReason, WillShutdownEvent } from 'vs/workbench/services/lifecycle/common/lifecycle'; @@ -34,7 +34,6 @@ import { configureTerminalProfileIcon } from 'vs/workbench/contrib/terminal/brow import { equals } from 'vs/base/common/objects'; import { Codicon, iconRegistry } from 'vs/base/common/codicons'; import { ITerminalContributionService } from 'vs/workbench/contrib/terminal/common/terminalExtensionPoints'; -import { ICommandService } from 'vs/platform/commands/common/commands'; import { ILabelService } from 'vs/platform/label/common/label'; import { Schemas } from 'vs/base/common/network'; import { VirtualWorkspaceContext } from 'vs/workbench/browser/contextkeys'; @@ -148,7 +147,6 @@ export class TerminalService implements ITerminalService { @IRemoteTerminalService private readonly _remoteTerminalService: IRemoteTerminalService, @ITelemetryService private readonly _telemetryService: ITelemetryService, @ITerminalContributionService private readonly _terminalContributionService: ITerminalContributionService, - @ICommandService private readonly _commandService: ICommandService, @IExtensionService private readonly _extensionService: IExtensionService, @INotificationService private readonly _notificationService: INotificationService, @optional(ILocalTerminalService) localTerminalService: ILocalTerminalService @@ -956,9 +954,6 @@ export class TerminalService implements ITerminalService { // Add contributed profiles if (type === 'createInstance') { - if (this._terminalContributionService.terminalProfiles.length > 0 || this._terminalContributionService.terminalTypes.length > 0) { - quickPickItems.push({ type: 'separator', label: nls.localize('terminalProfiles.contributed', "contributed") }); - } for (const contributed of this._terminalContributionService.terminalProfiles) { const icon = contributed.icon ? (iconRegistry.get(contributed.icon) || Codicon.terminal) : Codicon.terminal; quickPickItems.push({ @@ -966,17 +961,6 @@ export class TerminalService implements ITerminalService { profile: contributed }); } - - // Add contributed types (legacy), these cannot be defaults - if (type === 'createInstance') { - for (const contributed of this._terminalContributionService.terminalTypes) { - const icon = contributed.icon ? (iconRegistry.get(contributed.icon) || Codicon.terminal) : Codicon.terminal; - quickPickItems.push({ - label: `$(${icon.id}) ${contributed.title}`, - profile: contributed - }); - } - } } if (autoDetectedProfiles.length > 0) { @@ -989,11 +973,6 @@ export class TerminalService implements ITerminalService { return; } if (type === 'createInstance') { - // Legacy implementation - remove when js-debug adopts new - if ('command' in value.profile) { - return this._commandService.executeCommand(value.profile.command); - } - const activeInstance = this.getActiveInstance(); let instance; @@ -1225,7 +1204,7 @@ export class TerminalService implements ITerminalService { } interface IProfileQuickPickItem extends IQuickPickItem { - profile: ITerminalProfile | ITerminalTypeContribution | ITerminalProfileContribution; + profile: ITerminalProfile | ITerminalProfileContribution; } interface IInstanceLocation { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalView.ts b/src/vs/workbench/contrib/terminal/browser/terminalView.ts index 862e5cb7b7a..6f28f55ae73 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalView.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalView.ts @@ -226,10 +226,6 @@ export class TerminalViewPane extends ViewPane { } } - for (const contributed of this._terminalContributionService.terminalTypes) { - dropdownActions.push(new MenuItemAction({ id: contributed.command, title: contributed.title, category: TerminalTabContextMenuGroup.Profile }, undefined, undefined, this._contextKeyService, this._commandService)); - } - for (const contributed of this._terminalContributionService.terminalProfiles) { dropdownActions.push(new Action(TerminalCommandId.NewWithProfile, contributed.title, undefined, true, () => this._terminalService.createContributedTerminalProfile(contributed.id, false))); submenuActions.push(new Action(TerminalCommandId.NewWithProfile, contributed.title, undefined, true, () => this._terminalService.createContributedTerminalProfile(contributed.id, true))); diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index fb62f0be0a7..641c4dc9455 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -573,17 +573,9 @@ export const DEFAULT_COMMANDS_TO_SKIP_SHELL: string[] = [ ]; export interface ITerminalContributions { - /** @deprecated */ - types?: ITerminalTypeContribution[]; profiles?: ITerminalProfileContribution[]; } -export interface ITerminalTypeContribution { - title: string; - command: string; - icon?: string; -} - export interface ITerminalProfileContribution { title: string; id: string; diff --git a/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts b/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts index 89d198c77aa..7aaf52ce172 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as extensionsRegistry from 'vs/workbench/services/extensions/common/extensionsRegistry'; -import { ITerminalTypeContribution, ITerminalContributions, terminalContributionsDescriptor, ITerminalProfileContribution } from 'vs/workbench/contrib/terminal/common/terminal'; +import { ITerminalContributions, terminalContributionsDescriptor, ITerminalProfileContribution } from 'vs/workbench/contrib/terminal/common/terminal'; import { flatten } from 'vs/base/common/arrays'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; @@ -14,7 +14,6 @@ export const terminalsExtPoint = extensionsRegistry.ExtensionsRegistry.registerE export interface ITerminalContributionService { readonly _serviceBrand: undefined; - readonly terminalTypes: ReadonlyArray; readonly terminalProfiles: ReadonlyArray; } @@ -23,30 +22,11 @@ export const ITerminalContributionService = createDecorator = []; - get terminalTypes() { return this._terminalTypes; } - private _terminalProfiles: ReadonlyArray = []; get terminalProfiles() { return this._terminalProfiles; } constructor() { terminalsExtPoint.setHandler(contributions => { - this._terminalTypes = flatten(contributions.filter(c => c.description.enableProposedApi).map(c => { - return c.value?.types?.map(e => { - // TODO: Remove after adoption in js-debug - if (!e.icon && c.description.identifier.value === 'ms-vscode.js-debug') { - e.icon = '$(debug)'; - } - // Only support $(id) for now, without that it should point to a path to be - // consistent with other icon APIs - if (e.icon && e.icon.startsWith('$(') && e.icon.endsWith(')')) { - e.icon = e.icon.substr(2, e.icon.length - 3); - } else { - e.icon = undefined; - } - return e; - }) || []; - })); this._terminalProfiles = flatten(contributions.map(c => { return c.value?.profiles?.map(e => { // Only support $(id) for now, without that it should point to a path to be From 0a9a394a6377cd5bbcd16380f386ffc1d44b0942 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 11 Jun 2021 05:01:40 -0700 Subject: [PATCH 5/8] Use a class for TerminalProfile --- src/vs/vscode.d.ts | 4 +++- src/vs/workbench/api/common/extHost.api.impl.ts | 1 + src/vs/workbench/api/common/extHostTypes.ts | 7 +++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index 14843de6649..03d1e143c88 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -5828,11 +5828,13 @@ declare module 'vscode' { /** * A terminal profile defines how a termianl will be launched. */ - export interface TerminalProfile { + export class TerminalProfile { /** * The options that the terminal will launch with. */ options: TerminalOptions | ExtensionTerminalOptions; + + constructor(options: TerminalOptions | ExtensionTerminalOptions); } /** diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 63eebb8e833..34723811671 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1252,6 +1252,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I TaskPanelKind: extHostTypes.TaskPanelKind, TaskRevealKind: extHostTypes.TaskRevealKind, TaskScope: extHostTypes.TaskScope, + TerminalProfile: extHostTypes.TerminalProfile, TextDocumentSaveReason: extHostTypes.TextDocumentSaveReason, TextEdit: extHostTypes.TextEdit, TextEditorCursorStyle: TextEditorCursorStyle, diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index ecf921e4544..dc07df89ab3 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -1729,6 +1729,13 @@ export enum SourceControlInputBoxValidationType { Information = 2 } +export class TerminalProfile implements vscode.TerminalProfile { + constructor( + public options: vscode.TerminalOptions | vscode.ExtensionTerminalOptions + ) { + } +} + export enum TaskRevealKind { Always = 1, From a742a9bbe8a37e96dfecf1a3cb3ceaec0a5aa7f9 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 11 Jun 2021 05:09:42 -0700 Subject: [PATCH 6/8] doc cleanup --- src/vs/vscode.d.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index 03d1e143c88..6e72bb7f6e5 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -5821,12 +5821,13 @@ declare module 'vscode' { /** * Provide the terminal profile. * @param token A cancellation token that indicates the result is no longer needed. + * @returns The terminal profile. */ provideTerminalProfile(token: CancellationToken): ProviderResult; } /** - * A terminal profile defines how a termianl will be launched. + * A terminal profile defines how a terminal will be launched. */ export class TerminalProfile { /** From 9677eddbffee9822314f1f89058c039272c759c0 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 11 Jun 2021 05:15:35 -0700 Subject: [PATCH 7/8] Show error notification when profile fails --- src/vs/workbench/api/common/extHostTerminalService.ts | 2 +- .../contrib/terminal/browser/terminalService.ts | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 3046587b253..b9a8f43505a 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -602,7 +602,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I if (token.isCancellationRequested) { return; } - if (!profile) { + if (!profile || !('options' in profile)) { throw new Error(`No terminal profile options provided for id "${id}"`); } if ('pty' in profile.options) { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalService.ts b/src/vs/workbench/contrib/terminal/browser/terminalService.ts index bc61666abbd..8cfc0e816fd 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalService.ts @@ -1024,9 +1024,13 @@ export class TerminalService implements ITerminalService { this._notificationService.error(`No terminal profile provider registered for id "${id}"`); return; } - await profileProvider.createContributedTerminalProfile(isSplitTerminal); - this.setActiveInstanceByIndex(this._terminalInstances.length - 1); - await this.getActiveInstance()?.focusWhenReady(); + try { + await profileProvider.createContributedTerminalProfile(isSplitTerminal); + this.setActiveInstanceByIndex(this._terminalInstances.length - 1); + await this.getActiveInstance()?.focusWhenReady(); + } catch (e) { + this._notificationService.error(e.message); + } } private _createProfileQuickPickItem(profile: ITerminalProfile): IProfileQuickPickItem { From 465759bc5855192aaa57ff09de70dd28f4ddf3ca Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 11 Jun 2021 06:45:36 -0700 Subject: [PATCH 8/8] Prevent other extensions registering terminal profiles --- .../api/browser/mainThreadTerminalService.ts | 8 +++++--- .../workbench/api/common/extHost.api.impl.ts | 2 +- .../workbench/api/common/extHost.protocol.ts | 2 +- .../api/common/extHostTerminalService.ts | 6 +++--- .../contrib/terminal/browser/terminal.ts | 4 ++-- .../terminal/browser/terminalService.ts | 20 ++++++++++++------- .../contrib/terminal/browser/terminalView.ts | 4 ++-- .../common/terminalExtensionPoints.ts | 6 +++--- 8 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index 7b52f5c75d1..d9e9523de37 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -211,10 +211,12 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape this._terminalService.registerProcessSupport(isSupported); } - public $registerProfileProvider(id: string): void { + public $registerProfileProvider(id: string, extensionIdentifier: string): void { // Proxy profile provider requests through the extension host - this._profileProviders.set(id, this._terminalService.registerTerminalProfileProvider(id, { - createContributedTerminalProfile: async (isSplitTerminal) => this._proxy.$createContributedProfileTerminal(id, isSplitTerminal) + this._profileProviders.set(id, this._terminalService.registerTerminalProfileProvider(extensionIdentifier, id, { + createContributedTerminalProfile: async (isSplitTerminal) => { + return this._proxy.$createContributedProfileTerminal(id, isSplitTerminal); + } })); } diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 34723811671..f24fc12689f 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -675,7 +675,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I return extHostTerminalService.registerLinkProvider(provider); }, registerTerminalProfileProvider(id: string, provider: vscode.TerminalProfileProvider): vscode.Disposable { - return extHostTerminalService.registerProfileProvider(id, provider); + return extHostTerminalService.registerProfileProvider(extension, id, provider); }, registerTreeDataProvider(viewId: string, treeDataProvider: vscode.TreeDataProvider): vscode.Disposable { return extHostTreeViews.registerTreeDataProvider(viewId, treeDataProvider, extension); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index f93362a38f0..568a235420d 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -486,7 +486,7 @@ export interface MainThreadTerminalServiceShape extends IDisposable { $startLinkProvider(): void; $stopLinkProvider(): void; $registerProcessSupport(isSupported: boolean): void; - $registerProfileProvider(id: string): void; + $registerProfileProvider(id: string, extensionIdentifier: string): void; $unregisterProfileProvider(id: string): void; $setEnvironmentVariableCollection(extensionIdentifier: string, persistent: boolean, collection: ISerializableEnvironmentVariableCollection | undefined): void; diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index b9a8f43505a..a28ff3647c8 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -43,7 +43,7 @@ export interface IExtHostTerminalService extends ExtHostTerminalServiceShape, ID getDefaultShell(useAutomationShell: boolean): string; getDefaultShellArgs(useAutomationShell: boolean): string[] | string; registerLinkProvider(provider: vscode.TerminalLinkProvider): vscode.Disposable; - registerProfileProvider(id: string, provider: vscode.TerminalProfileProvider): vscode.Disposable; + registerProfileProvider(extension: IExtensionDescription, id: string, provider: vscode.TerminalProfileProvider): vscode.Disposable; getEnvironmentVariableCollection(extension: IExtensionDescription, persistent?: boolean): vscode.EnvironmentVariableCollection; } @@ -584,12 +584,12 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I }); } - public registerProfileProvider(id: string, provider: vscode.TerminalProfileProvider): vscode.Disposable { + public registerProfileProvider(extension: IExtensionDescription, id: string, provider: vscode.TerminalProfileProvider): vscode.Disposable { if (this._profileProviders.has(id)) { throw new Error(`Terminal profile provider "${id}" already registered`); } this._profileProviders.set(id, provider); - this._proxy.$registerProfileProvider(id); + this._proxy.$registerProfileProvider(id, extension.identifier.value); return new VSCodeDisposable(() => { this._profileProviders.delete(id); this._proxy.$unregisterProfileProvider(id); diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index 31cac615fef..33932d8a39b 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -141,7 +141,7 @@ export interface ITerminalService { */ createTerminal(profile: ITerminalProfile): ITerminalInstance; - createContributedTerminalProfile(id: string, isSplitTerminal: boolean): Promise; + createContributedTerminalProfile(extensionIdentifier: string, id: string, isSplitTerminal: boolean): Promise; /** * Creates a raw terminal instance, this should not be used outside of the terminal part. @@ -201,7 +201,7 @@ export interface ITerminalService { */ registerLinkProvider(linkProvider: ITerminalExternalLinkProvider): IDisposable; - registerTerminalProfileProvider(id: string, profileProvider: ITerminalProfileProvider): IDisposable; + registerTerminalProfileProvider(extensionIdenfifier: string, id: string, profileProvider: ITerminalProfileProvider): IDisposable; showProfileQuickPick(type: 'setDefault' | 'createInstance', cwd?: string | URI): Promise; diff --git a/src/vs/workbench/contrib/terminal/browser/terminalService.ts b/src/vs/workbench/contrib/terminal/browser/terminalService.ts index 8cfc0e816fd..69c29005c37 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalService.ts @@ -57,7 +57,7 @@ export class TerminalService implements ITerminalService { private _findState: FindReplaceState; private _activeGroupIndex: number; private _activeInstanceIndex: number; - private readonly _profileProviders: Map = new Map(); + private readonly _profileProviders: Map> = new Map(); private _linkProviders: Set = new Set(); private _linkProviderDisposables: Map = new Map(); private _processSupportContextKey: IContextKey; @@ -811,8 +811,13 @@ export class TerminalService implements ITerminalService { }; } - registerTerminalProfileProvider(id: string, profileProvider: ITerminalProfileProvider): IDisposable { - this._profileProviders.set(id, profileProvider); + registerTerminalProfileProvider(extensionIdenfifier: string, id: string, profileProvider: ITerminalProfileProvider): IDisposable { + let extMap = this._profileProviders.get(extensionIdenfifier); + if (!extMap) { + extMap = new Map(); + this._profileProviders.set(extensionIdenfifier, extMap); + } + extMap.set(id, profileProvider); return toDisposable(() => this._profileProviders.delete(id)); } @@ -976,7 +981,7 @@ export class TerminalService implements ITerminalService { let instance; if ('id' in value.profile) { - await this.createContributedTerminalProfile(value.profile.id, !!(keyMods?.alt && activeInstance)); + await this.createContributedTerminalProfile(value.profile.extensionIdentifier, value.profile.id, !!(keyMods?.alt && activeInstance)); return; } else { if (keyMods?.alt && activeInstance) { @@ -1017,9 +1022,10 @@ export class TerminalService implements ITerminalService { return undefined; } - async createContributedTerminalProfile(id: string, isSplitTerminal: boolean): Promise { + async createContributedTerminalProfile(extensionIdentifier: string, id: string, isSplitTerminal: boolean): Promise { await this._extensionService.activateByEvent(`onTerminalProfile:${id}`); - const profileProvider = this._profileProviders.get(id); + const extMap = this._profileProviders.get(extensionIdentifier); + const profileProvider = extMap?.get(id); if (!profileProvider) { this._notificationService.error(`No terminal profile provider registered for id "${id}"`); return; @@ -1207,7 +1213,7 @@ export class TerminalService implements ITerminalService { } interface IProfileQuickPickItem extends IQuickPickItem { - profile: ITerminalProfile | ITerminalProfileContribution; + profile: ITerminalProfile | (ITerminalProfileContribution & { extensionIdentifier: string }); } interface IInstanceLocation { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalView.ts b/src/vs/workbench/contrib/terminal/browser/terminalView.ts index 6f28f55ae73..cdb7e78c3ae 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalView.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalView.ts @@ -227,8 +227,8 @@ export class TerminalViewPane extends ViewPane { } for (const contributed of this._terminalContributionService.terminalProfiles) { - dropdownActions.push(new Action(TerminalCommandId.NewWithProfile, contributed.title, undefined, true, () => this._terminalService.createContributedTerminalProfile(contributed.id, false))); - submenuActions.push(new Action(TerminalCommandId.NewWithProfile, contributed.title, undefined, true, () => this._terminalService.createContributedTerminalProfile(contributed.id, true))); + dropdownActions.push(new Action(TerminalCommandId.NewWithProfile, contributed.title, undefined, true, () => this._terminalService.createContributedTerminalProfile(contributed.extensionIdentifier, contributed.id, false))); + submenuActions.push(new Action(TerminalCommandId.NewWithProfile, contributed.title, undefined, true, () => this._terminalService.createContributedTerminalProfile(contributed.extensionIdentifier, contributed.id, true))); } if (dropdownActions.length > 0) { diff --git a/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts b/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts index 7aaf52ce172..77370d574ca 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts @@ -14,7 +14,7 @@ export const terminalsExtPoint = extensionsRegistry.ExtensionsRegistry.registerE export interface ITerminalContributionService { readonly _serviceBrand: undefined; - readonly terminalProfiles: ReadonlyArray; + readonly terminalProfiles: ReadonlyArray; } export const ITerminalContributionService = createDecorator('terminalContributionsService'); @@ -22,7 +22,7 @@ export const ITerminalContributionService = createDecorator = []; + private _terminalProfiles: ReadonlyArray = []; get terminalProfiles() { return this._terminalProfiles; } constructor() { @@ -36,7 +36,7 @@ export class TerminalContributionService implements ITerminalContributionService } else { e.icon = undefined; } - return e; + return { ...e, extensionIdentifier: c.description.identifier.value }; }) || []; })); });