Skip to content

Commit

Permalink
Remove terminal link handlers
Browse files Browse the repository at this point in the history 
Fixes #91606
  • Loading branch information
@Tyriar
Tyriar committed Aug 10, 2020
1 parent 62d2d60 commit 2b353aa
Show file tree
Hide file tree
Showing 10 changed files with 9 additions and 375 deletions.
29 changes: 0 additions & 29 deletions src/vs/vscode.proposed.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -927,35 +927,6 @@ declare module 'vscode' {

//#endregion

//#region Terminal link handlers https://github.com/microsoft/vscode/issues/91606

export namespace window {
/**
* Register a [TerminalLinkHandler](#TerminalLinkHandler) that can be used to intercept and
* handle links that are activated within terminals.
* @param handler The link handler being registered.
* @return A disposable that unregisters the link handler.
*/
export function registerTerminalLinkHandler(handler: TerminalLinkHandler): Disposable;
}

/**
* Describes how to handle terminal links.
*/
export interface TerminalLinkHandler {
/**
* Handles a link that is activated within the terminal.
*
* @param terminal The terminal the link was activated on.
* @param link The text of the link activated.
* @return Whether the link was handled, if the link was handled this link will not be
* considered by any other extension or by the default built-in link handler.
*/
handleLink(terminal: Terminal, link: string): ProviderResult<boolean>;
}

//#endregion

//#region @jrieken -> exclusive document filters

export interface DocumentFilter {
Expand Down
21 changes: 1 addition & 20 deletions src/vs/workbench/api/browser/mainThreadTerminalService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceS
import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers';
import { URI } from 'vs/base/common/uri';
import { StopWatch } from 'vs/base/common/stopwatch';
import { ITerminalInstanceService, ITerminalService, ITerminalInstance, ITerminalBeforeHandleLinkEvent, ITerminalExternalLinkProvider, ITerminalLink } from 'vs/workbench/contrib/terminal/browser/terminal';
import { ITerminalInstanceService, ITerminalService, ITerminalInstance, ITerminalExternalLinkProvider, ITerminalLink } from 'vs/workbench/contrib/terminal/browser/terminal';
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { TerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminalDataBuffering';
Expand All @@ -25,7 +25,6 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
private readonly _toDispose = new DisposableStore();
private readonly _terminalProcessProxies = new Map<number, ITerminalProcessExtHostProxy>();
private _dataEventTracker: TerminalDataEventTracker | undefined;
private _linkHandler: IDisposable | undefined;
/**
* A single shared terminal link provider for the exthost. When an ext registers a link
* provider, this is registered with the terminal on the renderer side and all links are
Expand Down Expand Up @@ -95,7 +94,6 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape

public dispose(): void {
this._toDispose.dispose();
this._linkHandler?.dispose();
this._linkProvider?.dispose();

// TODO@Daniel: Should all the previously created terminals be disposed
Expand Down Expand Up @@ -166,16 +164,6 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
}
}

public $startHandlingLinks(): void {
this._linkHandler?.dispose();
this._linkHandler = this._terminalService.addLinkHandler(this._remoteAuthority || '', e => this._handleLink(e));
}

public $stopHandlingLinks(): void {
this._linkHandler?.dispose();
this._linkHandler = undefined;
}

public $startLinkProvider(): void {
this._linkProvider?.dispose();
this._linkProvider = this._terminalService.registerLinkProvider(new ExtensionTerminalLinkProvider(this._proxy));
Expand All @@ -186,13 +174,6 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
this._linkProvider = undefined;
}

private async _handleLink(e: ITerminalBeforeHandleLinkEvent): Promise<boolean> {
if (!e.terminal) {
return false;
}
return this._proxy.$handleLink(e.terminal.id, e.link);
}

private _onActiveTerminalChanged(terminalId: number | null): void {
this._proxy.$acceptActiveTerminalChanged(terminalId);
}
Expand Down
4 changes: 0 additions & 4 deletions src/vs/workbench/api/common/extHost.api.impl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -579,10 +579,6 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
}
return extHostTerminalService.createTerminal(nameOrOptions, shellPath, shellArgs);
},
registerTerminalLinkHandler(handler: vscode.TerminalLinkHandler): vscode.Disposable {
checkProposedApiEnabled(extension);
return extHostTerminalService.registerLinkHandler(handler);
},
registerTerminalLinkProvider(handler: vscode.TerminalLinkProvider): vscode.Disposable {
return extHostTerminalService.registerLinkProvider(handler);
},
Expand Down
3 changes: 0 additions & 3 deletions src/vs/workbench/api/common/extHost.protocol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,6 @@ export interface MainThreadTerminalServiceShape extends IDisposable {
$show(terminalId: number, preserveFocus: boolean): void;
$startSendingDataEvents(): void;
$stopSendingDataEvents(): void;
$startHandlingLinks(): void;
$stopHandlingLinks(): void;
$startLinkProvider(): void;
$stopLinkProvider(): void;
$setEnvironmentVariableCollection(extensionIdentifier: string, persistent: boolean, collection: ISerializableEnvironmentVariableCollection | undefined): void;
Expand Down Expand Up @@ -1434,7 +1432,6 @@ export interface ExtHostTerminalServiceShape {
$acceptWorkspacePermissionsChanged(isAllowed: boolean): void;
$getAvailableShells(): Promise<IShellDefinitionDto[]>;
$getDefaultShellAndArgs(useAutomationShell: boolean): Promise<IShellAndArgsDto>;
$handleLink(id: number, link: string): Promise<boolean>;
$provideLinks(id: number, line: string): Promise<ITerminalLinkDto[]>;
$activateLink(id: number, linkId: number): void;
$initEnvironmentVariableCollections(collections: [string, ISerializableEnvironmentVariableCollection][]): void;
Expand Down
34 changes: 0 additions & 34 deletions src/vs/workbench/api/common/extHostTerminalService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export interface IExtHostTerminalService extends ExtHostTerminalServiceShape {
attachPtyToTerminal(id: number, pty: vscode.Pseudoterminal): void;
getDefaultShell(useAutomationShell: boolean, configProvider: ExtHostConfigProvider): string;
getDefaultShellArgs(useAutomationShell: boolean, configProvider: ExtHostConfigProvider): string[] | string;
registerLinkHandler(handler: vscode.TerminalLinkHandler): vscode.Disposable;
registerLinkProvider(provider: vscode.TerminalLinkProvider): vscode.Disposable;
getEnvironmentVariableCollection(extension: IExtensionDescription, persistent?: boolean): vscode.EnvironmentVariableCollection;
}
Expand Down Expand Up @@ -318,7 +317,6 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
protected _environmentVariableCollections: Map<string, EnvironmentVariableCollection> = new Map();

private readonly _bufferer: TerminalDataBufferer;
private readonly _linkHandlers: Set<vscode.TerminalLinkHandler> = new Set();
private readonly _linkProviders: Set<vscode.TerminalLinkProvider> = new Set();
private readonly _terminalLinkCache: Map<number, Map<number, ICachedLinkEntry>> = new Map();
private readonly _terminalLinkCancellationSource: Map<number, CancellationTokenSource> = new Map();
Expand Down Expand Up @@ -559,19 +557,6 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
return id;
}

public registerLinkHandler(handler: vscode.TerminalLinkHandler): vscode.Disposable {
this._linkHandlers.add(handler);
if (this._linkHandlers.size === 1 && this._linkProviders.size === 0) {
this._proxy.$startHandlingLinks();
}
return new VSCodeDisposable(() => {
this._linkHandlers.delete(handler);
if (this._linkHandlers.size === 0 && this._linkProviders.size === 0) {
this._proxy.$stopHandlingLinks();
}
});
}

public registerLinkProvider(provider: vscode.TerminalLinkProvider): vscode.Disposable {
this._linkProviders.add(provider);
if (this._linkProviders.size === 1) {
Expand All @@ -585,25 +570,6 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
});
}

public async $handleLink(id: number, link: string): Promise<boolean> {
const terminal = this._getTerminalById(id);
if (!terminal) {
return false;
}

// Call each handler synchronously so multiple handlers aren't triggered at once
const it = this._linkHandlers.values();
let next = it.next();
while (!next.done) {
const handled = await next.value.handleLink(terminal, link);
if (handled) {
return true;
}
next = it.next();
}
return false;
}

public async $provideLinks(terminalId: number, line: string): Promise<ITerminalLinkDto[]> {
const terminal = this._getTerminalById(terminalId);
if (!terminal) {
Expand Down
58 changes: 6 additions & 52 deletions src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@ import { IFileService } from 'vs/platform/files/common/files';
import { Terminal, IViewportRange, ILinkProvider } from 'xterm';
import { REMOTE_HOST_SCHEME } from 'vs/platform/remote/common/remoteHosts';
import { posix, win32 } from 'vs/base/common/path';
import { ITerminalBeforeHandleLinkEvent, LINK_INTERCEPT_THRESHOLD, ITerminalExternalLinkProvider, ITerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal';
import { ITerminalExternalLinkProvider, ITerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal';
import { OperatingSystem, isMacintosh, OS } from 'vs/base/common/platform';
import { IMarkdownString, MarkdownString } from 'vs/base/common/htmlContent';
import { Emitter, Event } from 'vs/base/common/event';
import { ILogService } from 'vs/platform/log/common/log';
import { TerminalProtocolLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalProtocolLinkProvider';
import { TerminalValidatedLocalLinkProvider, lineAndColumnClause, unixLocalLinkClause, winLocalLinkClause, winDrivePrefix, winLineAndColumnMatchIndex, unixLineAndColumnMatchIndex, lineAndColumnClauseGroupCount } from 'vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider';
import { TerminalWordLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider';
Expand All @@ -44,39 +42,23 @@ interface IPath {
export class TerminalLinkManager extends DisposableStore {
private _widgetManager: TerminalWidgetManager | undefined;
private _processCwd: string | undefined;
private _hasBeforeHandleLinkListeners = false;
private _standardLinkProviders: ILinkProvider[] = [];
private _standardLinkProvidersDisposables: IDisposable[] = [];

protected static _LINK_INTERCEPT_THRESHOLD = LINK_INTERCEPT_THRESHOLD;
public static readonly LINK_INTERCEPT_THRESHOLD = TerminalLinkManager._LINK_INTERCEPT_THRESHOLD;

private readonly _onBeforeHandleLink = this.add(new Emitter<ITerminalBeforeHandleLinkEvent>({
onFirstListenerAdd: () => this._hasBeforeHandleLinkListeners = true,
onLastListenerRemove: () => this._hasBeforeHandleLinkListeners = false
}));
/**
* Allows intercepting links and handling them outside of the default link handler. When fired
* the listener has a set amount of time to handle the link or the default handler will fire.
* This was designed to only be handled by a single listener.
*/
public get onBeforeHandleLink(): Event<ITerminalBeforeHandleLinkEvent> { return this._onBeforeHandleLink.event; }

constructor(
private _xterm: Terminal,
private readonly _processManager: ITerminalProcessManager,
@IOpenerService private readonly _openerService: IOpenerService,
@IEditorService private readonly _editorService: IEditorService,
@IConfigurationService private readonly _configurationService: IConfigurationService,
@IFileService private readonly _fileService: IFileService,
@ILogService private readonly _logService: ILogService,
@IInstantiationService private readonly _instantiationService: IInstantiationService
) {
super();

// Protocol links
const wrappedActivateCallback = this._wrapLinkHandler((_, link) => this._handleProtocolLink(link));
const protocolProvider = this._instantiationService.createInstance(TerminalProtocolLinkProvider, this._xterm, wrappedActivateCallback, this._tooltipCallback2.bind(this));
const protocolProvider = this._instantiationService.createInstance(TerminalProtocolLinkProvider, this._xterm, wrappedActivateCallback, this._tooltipCallback.bind(this));
this._standardLinkProviders.push(protocolProvider);

// Validated local links
Expand All @@ -87,19 +69,19 @@ export class TerminalLinkManager extends DisposableStore {
this._processManager.os || OS,
wrappedTextLinkActivateCallback,
this._wrapLinkHandler.bind(this),
this._tooltipCallback2.bind(this),
this._tooltipCallback.bind(this),
async (link, cb) => cb(await this._resolvePath(link)));
this._standardLinkProviders.push(validatedProvider);
}

// Word links
const wordProvider = this._instantiationService.createInstance(TerminalWordLinkProvider, this._xterm, this._wrapLinkHandler.bind(this), this._tooltipCallback2.bind(this));
const wordProvider = this._instantiationService.createInstance(TerminalWordLinkProvider, this._xterm, this._wrapLinkHandler.bind(this), this._tooltipCallback.bind(this));
this._standardLinkProviders.push(wordProvider);

this._registerStandardLinkProviders();
}

private _tooltipCallback2(link: TerminalLink, viewportRange: IViewportRange, modifierDownCallback?: () => void, modifierUpCallback?: () => void) {
private _tooltipCallback(link: TerminalLink, viewportRange: IViewportRange, modifierDownCallback?: () => void, modifierUpCallback?: () => void) {
if (!this._widgetManager) {
return;
}
Expand Down Expand Up @@ -156,7 +138,7 @@ export class TerminalLinkManager extends DisposableStore {
}

public registerExternalLinkProvider(instance: ITerminalInstance, linkProvider: ITerminalExternalLinkProvider): IDisposable {
const wrappedLinkProvider = this._instantiationService.createInstance(TerminalExternalLinkProviderAdapter, this._xterm, instance, linkProvider, this._wrapLinkHandler.bind(this), this._tooltipCallback2.bind(this));
const wrappedLinkProvider = this._instantiationService.createInstance(TerminalExternalLinkProviderAdapter, this._xterm, instance, linkProvider, this._wrapLinkHandler.bind(this), this._tooltipCallback.bind(this));
const newLinkProvider = this._xterm.registerLinkProvider(wrappedLinkProvider);
// Re-register the standard link providers so they are a lower priority that the new one
this._registerStandardLinkProviders();
Expand All @@ -173,38 +155,11 @@ export class TerminalLinkManager extends DisposableStore {
return;
}

// Allow the link to be intercepted if there are listeners
if (this._hasBeforeHandleLinkListeners) {
const wasHandled = await this._triggerBeforeHandleLinkListeners(link);
if (!wasHandled) {
handler(event, link);
}
return;
}

// Just call the handler if there is no before listener
handler(event, link);
};
}

private async _triggerBeforeHandleLinkListeners(link: string): Promise<boolean> {
return new Promise<boolean>(r => {
const timeoutId = setTimeout(() => {
canceled = true;
this._logService.error(`An extension intecepted a terminal link but it timed out after ${TerminalLinkManager.LINK_INTERCEPT_THRESHOLD / 1000} seconds`);
r(false);
}, TerminalLinkManager.LINK_INTERCEPT_THRESHOLD);
let canceled = false;
const resolve = (handled: boolean) => {
if (!canceled) {
clearTimeout(timeoutId);
r(handled);
}
};
this._onBeforeHandleLink.fire({ link, resolve });
});
}

protected get _localLinkRegex(): RegExp {
if (!this._processManager) {
throw new Error('Process manager is required');
Expand Down Expand Up @@ -369,7 +324,6 @@ export class TerminalLinkManager extends DisposableStore {
* @param link Url link which may contain line and column number.
*/
public extractLineColumnInfo(link: string): LineColumnInfo {

const matches: string[] | null = this._localLinkRegex.exec(link);
const lineColumnInfo: LineColumnInfo = {
lineNumber: 1,
Expand Down
17 changes: 0 additions & 17 deletions src/vs/workbench/contrib/terminal/browser/terminal.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,6 @@ export interface ITerminalService {
findNext(): void;
findPrevious(): void;

/**
* Link handlers can be registered here to allow intercepting links clicked in the terminal.
* When a link is clicked, the link will be considered handled when the first interceptor
* resolves with true. It will be considered not handled when _all_ link handlers resolve with
* false, or 3 seconds have elapsed.
*/
addLinkHandler(key: string, callback: TerminalLinkHandlerCallback): IDisposable;

/**
* Registers a link provider that enables integrators to add links to the terminal.
* @param linkProvider When registered, the link provider is asked whenever a cell is hovered
Expand Down Expand Up @@ -215,8 +207,6 @@ export enum WindowsShellType {
}
export type TerminalShellType = WindowsShellType | undefined;

export const LINK_INTERCEPT_THRESHOLD = 3000;

export interface ITerminalBeforeHandleLinkEvent {
terminal?: ITerminalInstance;
/** The text of the link */
Expand All @@ -225,8 +215,6 @@ export interface ITerminalBeforeHandleLinkEvent {
resolve(wasHandled: boolean): void;
}

export type TerminalLinkHandlerCallback = (e: ITerminalBeforeHandleLinkEvent) => Promise<boolean>;

export interface ITerminalInstance {
/**
* The ID of the terminal instance, this is an arbitrary number only used to identify the
Expand Down Expand Up @@ -289,11 +277,6 @@ export interface ITerminalInstance {
*/
onExit: Event<number | undefined>;

/**
* Attach a listener to intercept and handle link clicks in the terminal.
*/
onBeforeHandleLink: Event<ITerminalBeforeHandleLinkEvent>;

readonly exitCode: number | undefined;

readonly areLinksReady: boolean;
Expand Down
8 changes: 1 addition & 7 deletions src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { ansiColorIdentifiers, TERMINAL_BACKGROUND_COLOR, TERMINAL_CURSOR_BACKGR
import { TerminalConfigHelper } from 'vs/workbench/contrib/terminal/browser/terminalConfigHelper';
import { TerminalLinkManager } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkManager';
import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility';
import { ITerminalInstanceService, ITerminalInstance, TerminalShellType, WindowsShellType, ITerminalBeforeHandleLinkEvent, ITerminalExternalLinkProvider } from 'vs/workbench/contrib/terminal/browser/terminal';
import { ITerminalInstanceService, ITerminalInstance, TerminalShellType, WindowsShellType, ITerminalExternalLinkProvider } from 'vs/workbench/contrib/terminal/browser/terminal';
import { TerminalProcessManager } from 'vs/workbench/contrib/terminal/browser/terminalProcessManager';
import { Terminal as XTermTerminal, IBuffer, ITerminalAddon } from 'xterm';
import { SearchAddon, ISearchOptions } from 'xterm-addon-search';
Expand Down Expand Up @@ -164,8 +164,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
public get onMaximumDimensionsChanged(): Event<void> { return this._onMaximumDimensionsChanged.event; }
private readonly _onFocus = new Emitter<ITerminalInstance>();
public get onFocus(): Event<ITerminalInstance> { return this._onFocus.event; }
private readonly _onBeforeHandleLink = new Emitter<ITerminalBeforeHandleLinkEvent>();
public get onBeforeHandleLink(): Event<ITerminalBeforeHandleLinkEvent> { return this._onBeforeHandleLink.event; }

public constructor(
private readonly _terminalFocusContextKey: IContextKey<boolean>,
Expand Down Expand Up @@ -419,10 +417,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
});
}
this._linkManager = this._instantiationService.createInstance(TerminalLinkManager, xterm, this._processManager!);
this._linkManager.onBeforeHandleLink(e => {
e.terminal = this;
this._onBeforeHandleLink.fire(e);
});
this._areLinksReady = true;
this._onLinksReady.fire(this);
});
Expand Down
Loading

0 comments on commit 2b353aa

Please sign in to comment.