Skip to content

Commit

Permalink
Updating the TerminalInstance to only reigster link providers once. (#…
Browse files Browse the repository at this point in the history
…135419)

When a task reuses a terminal, the TerminalProcessManager would fire the processReady event each time a task is run, resulting in multiple registrations of the link providers. Re-registering the terminal link providers would cause xterm to trigger multiple overlapping providers on the same line of output, which is not supported by xterm at the moment. The net result is that no external link providers would work for terminals that are reused by tasks.
  • Loading branch information
ashgti authored Oct 19, 2021
1 parent 550ae45 commit 0db19c1
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class TerminalLinkManager extends DisposableStore {
private _widgetManager: TerminalWidgetManager | undefined;
private _processCwd: string | undefined;
private _standardLinkProviders: ILinkProvider[] = [];
private _standardLinkProvidersDisposables: IDisposable[] = [];
private _linkProvidersDisposables: IDisposable[] = [];

constructor(
private _xterm: Terminal,
Expand Down Expand Up @@ -137,18 +137,23 @@ export class TerminalLinkManager extends DisposableStore {
this._processCwd = processCwd;
}

private _clearLinkProviders(): void {
dispose(this._linkProvidersDisposables);
this._linkProvidersDisposables = [];
}

private _registerStandardLinkProviders(): void {
dispose(this._standardLinkProvidersDisposables);
this._standardLinkProvidersDisposables = [];
for (const p of this._standardLinkProviders) {
this._standardLinkProvidersDisposables.push(this._xterm.registerLinkProvider(p));
this._linkProvidersDisposables.push(this._xterm.registerLinkProvider(p));
}
}

registerExternalLinkProvider(instance: ITerminalInstance, linkProvider: ITerminalExternalLinkProvider): IDisposable {
// Clear and re-register the standard link providers so they are a lower priority that the new one
this._clearLinkProviders();
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._linkProvidersDisposables.push(newLinkProvider);
this._registerStandardLinkProviders();
return newLinkProvider;
}
Expand Down
5 changes: 5 additions & 0 deletions src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,11 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
// Init winpty compat and link handler after process creation as they rely on the
// underlying process OS
this._processManager.onProcessReady((processTraits) => {
// If links are ready, do not re-create the manager.
if (this._areLinksReady) {
return;
}

if (this._processManager.os === OperatingSystem.Windows) {
xterm.setOption('windowsMode', processTraits.requiresWindowsMode || false);
// Force line data to be sent when the cursor is moved, the main purpose for
Expand Down

0 comments on commit 0db19c1

Please sign in to comment.