Merge pull request #4924 from Tyriar/link_refactor

Split Linkifier2 (always active) into Linkifier (after attach) and LinkProviderService (always active)

.github/workflows/ci.yml

@@ -153,7 +153,7 @@ jobs:
     timeout-minutes: 20
     strategy:
       matrix:
-        node-version: [16]
+        node-version: [18]
         runs-on: [ubuntu, macos, windows]
     runs-on: ${{ matrix.runs-on }}-latest
     steps:
@@ -167,6 +167,11 @@ jobs:
       run: |
         yarn --frozen-lockfile
         yarn install-addons
+    - name: Wait for build job
+      uses: NathanFirmo/wait-for-other-job@v1.1.1
+      with:
+        token: ${{ secrets.GITHUB_TOKEN }}
+        job: build
     - uses: actions/download-artifact@v3
       with:
         name: build-artifacts

addons/addon-canvas/src/CanvasAddon.ts

@@ -37,7 +37,7 @@ export class CanvasAddon extends Disposable implements ITerminalAddon , ICanvasA
     const coreService = core.coreService;
     const optionsService = core.optionsService;
     const screenElement = core.screenElement!;
-    const linkifier = core.linkifier2;
+    const linkifier = core.linkifier!;
 
     const unsafeCore = core as any;
     const bufferService: IBufferService = unsafeCore._bufferService;

addons/addon-webgl/src/WebglRenderer.ts

@@ -81,7 +81,7 @@ export class WebglRenderer extends Disposable implements IRenderer {
     this._core = (this._terminal as any)._core;
 
     this._renderLayers = [
-      new LinkRenderLayer(this._core.screenElement!, 2, this._terminal, this._core.linkifier2, this._coreBrowserService, _optionsService, this._themeService)
+      new LinkRenderLayer(this._core.screenElement!, 2, this._terminal, this._core.linkifier!, this._coreBrowserService, _optionsService, this._themeService)
     ];
     this.dimensions = createRenderDimensions();
     this._devicePixelRatio = this._coreBrowserService.dpr;

src/browser/Linkifier.test.ts

@@ -5,11 +5,13 @@
 
 import { assert } from 'chai';
 import { IBufferService } from 'common/services/Services';
-import { Linkifier2 } from 'browser/Linkifier2';
+import { Linkifier } from './Linkifier';
 import { MockBufferService } from 'common/TestUtils.test';
 import { ILink } from 'browser/Types';
+import { LinkProviderService } from 'browser/services/LinkProviderService';
+import jsdom = require('jsdom');
 
-class TestLinkifier2 extends Linkifier2 {
+class TestLinkifier2 extends Linkifier {
   public set currentLink(link: any) {
     this._currentLink = link;
   }
@@ -43,8 +45,9 @@ describe('Linkifier2', () => {
   };
 
   beforeEach(() => {
+    const dom = new jsdom.JSDOM();
     bufferService = new MockBufferService(100, 10);
-    linkifier = new TestLinkifier2(bufferService);
+    linkifier = new TestLinkifier2(dom.window.document.createElement('div'), null!, null!, bufferService, new LinkProviderService());
     linkifier.currentLink = {
       link,
       state: {

src/browser/Linkifier.ts

@@ -4,18 +4,14 @@
  */
 
 import { addDisposableDomListener } from 'browser/Lifecycle';
-import { IBufferCellPosition, ILink, ILinkDecorations, ILinkProvider, ILinkWithState, ILinkifier2, ILinkifierEvent } from 'browser/Types';
+import { IBufferCellPosition, ILink, ILinkDecorations, ILinkWithState, ILinkifier2, ILinkifierEvent } from 'browser/Types';
 import { EventEmitter } from 'common/EventEmitter';
 import { Disposable, disposeArray, getDisposeArrayDisposable, toDisposable } from 'common/Lifecycle';
 import { IDisposable } from 'common/Types';
 import { IBufferService } from 'common/services/Services';
-import { IMouseService, IRenderService } from './services/Services';
+import { ILinkProviderService, IMouseService, IRenderService } from './services/Services';
 
-export class Linkifier2 extends Disposable implements ILinkifier2 {
-  private _element: HTMLElement | undefined;
-  private _mouseService: IMouseService | undefined;
-  private _renderService: IRenderService | undefined;
-  private _linkProviders: ILinkProvider[] = [];
+export class Linkifier extends Disposable implements ILinkifier2 {
   public get currentLink(): ILinkWithState | undefined { return this._currentLink; }
   protected _currentLink: ILinkWithState | undefined;
   private _mouseDownLink: ILinkWithState | undefined;
@@ -33,14 +29,17 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
   public readonly onHideLinkUnderline = this._onHideLinkUnderline.event;
 
   constructor(
-    @IBufferService private readonly _bufferService: IBufferService
+    private readonly _element: HTMLElement,
+    @IMouseService private readonly _mouseService: IMouseService,
+    @IRenderService private readonly _renderService: IRenderService,
+    @IBufferService private readonly _bufferService: IBufferService,
+    @ILinkProviderService private readonly _linkProviderService: ILinkProviderService
   ) {
     super();
     this.register(getDisposeArrayDisposable(this._linkCacheDisposables));
     this.register(toDisposable(() => {
       this._lastMouseEvent = undefined;
       // Clear out link providers as they could easily cause an embedder memory leak
-      this._linkProviders.length = 0;
       this._activeProviderReplies?.clear();
     }));
     // Listen to resize to catch the case where it's resized and the cursor is out of the viewport.
@@ -48,27 +47,6 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
       this._clearCurrentLink();
       this._wasResized = true;
     }));
-  }
-
-  public registerLinkProvider(linkProvider: ILinkProvider): IDisposable {
-    this._linkProviders.push(linkProvider);
-    return {
-      dispose: () => {
-        // Remove the link provider from the list
-        const providerIndex = this._linkProviders.indexOf(linkProvider);
-
-        if (providerIndex !== -1) {
-          this._linkProviders.splice(providerIndex, 1);
-        }
-      }
-    };
-  }
-
-  public attachToDom(element: HTMLElement, mouseService: IMouseService, renderService: IRenderService): void {
-    this._element = element;
-    this._mouseService = mouseService;
-    this._renderService = renderService;
-
     this.register(addDisposableDomListener(this._element, 'mouseleave', () => {
       this._isMouseOut = true;
       this._clearCurrentLink();
@@ -81,10 +59,6 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
   private _handleMouseMove(event: MouseEvent): void {
     this._lastMouseEvent = event;
 
-    if (!this._element || !this._mouseService) {
-      return;
-    }
-
     const position = this._positionFromMouseEvent(event, this._element, this._mouseService);
     if (!position) {
       return;
@@ -145,7 +119,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
     let linkProvided = false;
 
     // There is no link cached, so ask for one
-    for (const [i, linkProvider] of this._linkProviders.entries()) {
+    for (const [i, linkProvider] of this._linkProviderService.linkProviders.entries()) {
       if (useLineCache) {
         const existingReply = this._activeProviderReplies?.get(i);
         // If there isn't a reply, the provider hasn't responded yet.
@@ -167,7 +141,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
 
           // If all providers have responded, remove lower priority links that intersect ranges of
           // higher priority links
-          if (this._activeProviderReplies?.size === this._linkProviders.length) {
+          if (this._activeProviderReplies?.size === this._linkProviderService.linkProviders.length) {
             this._removeIntersectingLinks(position.y, this._activeProviderReplies);
           }
         });
@@ -223,7 +197,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
     }
 
     // Check if all the providers have responded
-    if (this._activeProviderReplies.size === this._linkProviders.length && !linkProvided) {
+    if (this._activeProviderReplies.size === this._linkProviderService.linkProviders.length && !linkProvided) {
       // Respect the order of the link providers
       for (let j = 0; j < this._activeProviderReplies.size; j++) {
         const currentLink = this._activeProviderReplies.get(j)?.find(link => this._linkAtPosition(link.link, position));
@@ -243,7 +217,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
   }
 
   private _handleMouseUp(event: MouseEvent): void {
-    if (!this._element || !this._mouseService || !this._currentLink) {
+    if (!this._currentLink) {
       return;
     }
 
@@ -258,7 +232,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
   }
 
   private _clearCurrentLink(startRow?: number, endRow?: number): void {
-    if (!this._element || !this._currentLink || !this._lastMouseEvent) {
+    if (!this._currentLink || !this._lastMouseEvent) {
       return;
     }
 
@@ -271,7 +245,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
   }
 
   private _handleNewLink(linkWithState: ILinkWithState): void {
-    if (!this._element || !this._lastMouseEvent || !this._mouseService) {
+    if (!this._lastMouseEvent) {
       return;
     }
 
@@ -302,7 +276,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
             if (this._currentLink?.state && this._currentLink.state.decorations.pointerCursor !== v) {
               this._currentLink.state.decorations.pointerCursor = v;
               if (this._currentLink.state.isHovered) {
-                this._element?.classList.toggle('xterm-cursor-pointer', v);
+                this._element.classList.toggle('xterm-cursor-pointer', v);
               }
             }
           }
@@ -322,29 +296,27 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
 
       // Listen to viewport changes to re-render the link under the cursor (only when the line the
       // link is on changes)
-      if (this._renderService) {
-        this._linkCacheDisposables.push(this._renderService.onRenderedViewportChange(e => {
-          // Sanity check, this shouldn't happen in practice as this listener would be disposed
-          if (!this._currentLink) {
-            return;
-          }
-          // When start is 0 a scroll most likely occurred, make sure links above the fold also get
-          // cleared.
-          const start = e.start === 0 ? 0 : e.start + 1 + this._bufferService.buffer.ydisp;
-          const end = this._bufferService.buffer.ydisp + 1 + e.end;
-          // Only clear the link if the viewport change happened on this line
-          if (this._currentLink.link.range.start.y >= start && this._currentLink.link.range.end.y <= end) {
-            this._clearCurrentLink(start, end);
-            if (this._lastMouseEvent && this._element) {
-              // re-eval previously active link after changes
-              const position = this._positionFromMouseEvent(this._lastMouseEvent, this._element, this._mouseService!);
-              if (position) {
-                this._askForLink(position, false);
-              }
+      this._linkCacheDisposables.push(this._renderService.onRenderedViewportChange(e => {
+        // Sanity check, this shouldn't happen in practice as this listener would be disposed
+        if (!this._currentLink) {
+          return;
+        }
+        // When start is 0 a scroll most likely occurred, make sure links above the fold also get
+        // cleared.
+        const start = e.start === 0 ? 0 : e.start + 1 + this._bufferService.buffer.ydisp;
+        const end = this._bufferService.buffer.ydisp + 1 + e.end;
+        // Only clear the link if the viewport change happened on this line
+        if (this._currentLink.link.range.start.y >= start && this._currentLink.link.range.end.y <= end) {
+          this._clearCurrentLink(start, end);
+          if (this._lastMouseEvent) {
+            // re-eval previously active link after changes
+            const position = this._positionFromMouseEvent(this._lastMouseEvent, this._element, this._mouseService!);
+            if (position) {
+              this._askForLink(position, false);
             }
           }
-        }));
-      }
+        }
+      }));
     }
   }
 

src/browser/OscLinkProvider.ts

@@ -3,7 +3,8 @@
  * @license MIT
  */
 
-import { IBufferRange, ILink, ILinkProvider } from 'browser/Types';
+import { IBufferRange, ILink } from 'browser/Types';
+import { ILinkProvider } from 'browser/services/Services';
 import { CellData } from 'common/buffer/CellData';
 import { IBufferService, IOptionsService, IOscLinkService } from 'common/services/Services';
 

src/browser/Terminal.ts

@@ -23,7 +23,7 @@
 
 import { copyHandler, handlePasteEvent, moveTextAreaUnderMouseCursor, paste, rightClickHandler } from 'browser/Clipboard';
 import { addDisposableDomListener } from 'browser/Lifecycle';
-import { Linkifier2 } from 'browser/Linkifier2';
+import { Linkifier } from './Linkifier';
 import * as Strings from 'browser/LocalizableStrings';
 import { OscLinkProvider } from 'browser/OscLinkProvider';
 import { CharacterJoinerHandler, CustomKeyEventHandler, CustomWheelEventHandler, IBrowser, IBufferRange, ICompositionHelper, ILinkifier2, ITerminal, IViewport } from 'browser/Types';
@@ -39,7 +39,7 @@ import { CoreBrowserService } from 'browser/services/CoreBrowserService';
 import { MouseService } from 'browser/services/MouseService';
 import { RenderService } from 'browser/services/RenderService';
 import { SelectionService } from 'browser/services/SelectionService';
-import { ICharSizeService, ICharacterJoinerService, ICoreBrowserService, IMouseService, IRenderService, ISelectionService, IThemeService } from 'browser/services/Services';
+import { ICharSizeService, ICharacterJoinerService, ICoreBrowserService, ILinkProviderService, IMouseService, IRenderService, ISelectionService, IThemeService } from 'browser/services/Services';
 import { ThemeService } from 'browser/services/ThemeService';
 import { color, rgba } from 'common/Color';
 import { CoreTerminal } from 'common/CoreTerminal';
@@ -57,6 +57,7 @@ import { IDecorationService } from 'common/services/Services';
 import { IDecoration, IDecorationOptions, IDisposable, ILinkProvider, IMarker } from '@xterm/xterm';
 import { WindowsOptionsReportType } from '../common/InputHandler';
 import { AccessibilityManager } from './AccessibilityManager';
+import { LinkProviderService } from 'browser/services/LinkProviderService';
 
 export class Terminal extends CoreTerminal implements ITerminal {
   public textarea: HTMLTextAreaElement | undefined;
@@ -69,6 +70,7 @@ export class Terminal extends CoreTerminal implements ITerminal {
   private _helperContainer: HTMLElement | undefined;
   private _compositionView: HTMLElement | undefined;
 
+  public linkifier: ILinkifier2 | undefined;
   private _overviewRulerRenderer: OverviewRulerRenderer | undefined;
 
   public browser: IBrowser = Browser as any;
@@ -76,8 +78,11 @@ export class Terminal extends CoreTerminal implements ITerminal {
   private _customKeyEventHandler: CustomKeyEventHandler | undefined;
   private _customWheelEventHandler: CustomWheelEventHandler | undefined;
 
-  // browser services
+  // Browser services
   private _decorationService: DecorationService;
+  private _linkProviderService: ILinkProviderService;
+
+  // Optional browser services
   private _charSizeService: ICharSizeService | undefined;
   private _coreBrowserService: ICoreBrowserService | undefined;
   private _mouseService: IMouseService | undefined;
@@ -113,7 +118,6 @@ export class Terminal extends CoreTerminal implements ITerminal {
    */
   private _unprocessedDeadKey: boolean = false;
 
-  public linkifier2: ILinkifier2;
   public viewport: IViewport | undefined;
   private _compositionHelper: ICompositionHelper | undefined;
   private _accessibilityManager: MutableDisposable<AccessibilityManager> = this.register(new MutableDisposable());
@@ -149,10 +153,11 @@ export class Terminal extends CoreTerminal implements ITerminal {
 
     this._setup();
 
-    this.linkifier2 = this.register(this._instantiationService.createInstance(Linkifier2));
-    this.linkifier2.registerLinkProvider(this._instantiationService.createInstance(OscLinkProvider));
     this._decorationService = this._instantiationService.createInstance(DecorationService);
     this._instantiationService.setService(IDecorationService, this._decorationService);
+    this._linkProviderService = this._instantiationService.createInstance(LinkProviderService);
+    this._instantiationService.setService(ILinkProviderService, this._linkProviderService);
+    this._linkProviderService.registerLinkProvider(this._instantiationService.createInstance(OscLinkProvider));
 
     // Setup InputHandler listeners
     this.register(this._inputHandler.onRequestBell(() => this._onBell.fire()));
@@ -482,6 +487,11 @@ export class Terminal extends CoreTerminal implements ITerminal {
     this._compositionHelper = this._instantiationService.createInstance(CompositionHelper, this.textarea, this._compositionView);
     this._helperContainer.appendChild(this._compositionView);
 
+    this._mouseService = this._instantiationService.createInstance(MouseService);
+    this._instantiationService.setService(IMouseService, this._mouseService);
+
+    this.linkifier = this.register(this._instantiationService.createInstance(Linkifier, this.screenElement));
+
     // Performance: Add viewport and helper elements from the fragment
     this.element.appendChild(fragment);
 
@@ -493,9 +503,6 @@ export class Terminal extends CoreTerminal implements ITerminal {
       this._renderService.setRenderer(this._createRenderer());
     }
 
-    this._mouseService = this._instantiationService.createInstance(MouseService);
-    this._instantiationService.setService(IMouseService, this._mouseService);
-
     this.viewport = this._instantiationService.createInstance(Viewport, this._viewportElement, this._viewportScrollArea);
     this.viewport.onRequestScrollLines(e => this.scrollLines(e.amount, e.suppressScrollEvent, ScrollSource.VIEWPORT)),
     this.register(this._inputHandler.onRequestSyncScrollBar(() => this.viewport!.syncScrollArea()));
@@ -513,7 +520,7 @@ export class Terminal extends CoreTerminal implements ITerminal {
     this._selectionService = this.register(this._instantiationService.createInstance(SelectionService,
       this.element,
       this.screenElement,
-      this.linkifier2
+      this.linkifier
     ));
     this._instantiationService.setService(ISelectionService, this._selectionService);
     this.register(this._selectionService.onRequestScrollLines(e => this.scrollLines(e.amount, e.suppressScrollEvent)));
@@ -533,7 +540,6 @@ export class Terminal extends CoreTerminal implements ITerminal {
     }));
     this.register(addDisposableDomListener(this._viewportElement, 'scroll', () => this._selectionService!.refresh()));
 
-    this.linkifier2.attachToDom(this.screenElement, this._mouseService, this._renderService);
     this.register(this._instantiationService.createInstance(BufferDecorationRenderer, this.screenElement));
     this.register(addDisposableDomListener(this.element, 'mousedown', (e: MouseEvent) => this._selectionService!.handleMouseDown(e)));
 
@@ -575,7 +581,7 @@ export class Terminal extends CoreTerminal implements ITerminal {
   }
 
   private _createRenderer(): IRenderer {
-    return this._instantiationService.createInstance(DomRenderer, this, this._document!, this.element!, this.screenElement!, this._viewportElement!, this._helperContainer!, this.linkifier2);
+    return this._instantiationService.createInstance(DomRenderer, this, this._document!, this.element!, this.screenElement!, this._viewportElement!, this._helperContainer!, this.linkifier!);
   }
 
   /**
@@ -894,7 +900,7 @@ export class Terminal extends CoreTerminal implements ITerminal {
   }
 
   public registerLinkProvider(linkProvider: ILinkProvider): IDisposable {
-    return this.linkifier2.registerLinkProvider(linkProvider);
+    return this._linkProviderService.registerLinkProvider(linkProvider);
   }
 
   public registerCharacterJoiner(handler: CharacterJoinerHandler): number {

src/browser/TestUtils.test.ts

@@ -48,6 +48,7 @@ export class MockTerminal implements ITerminal {
   public onRender!: IEvent<{ start: number, end: number }>;
   public onResize!: IEvent<{ cols: number, rows: number }>;
   public markers!: IMarker[];
+  public linkifier: ILinkifier2 | undefined;
   public coreMouseService!: ICoreMouseService;
   public coreService!: ICoreService;
   public optionsService!: IOptionsService;
@@ -151,7 +152,6 @@ export class MockTerminal implements ITerminal {
   }
   public bracketedPasteMode!: boolean;
   public renderer!: IRenderer;
-  public linkifier2!: ILinkifier2;
   public isFocused!: boolean;
   public options!: Required<ITerminalOptions>;
   public element!: HTMLElement;

src/browser/Types.d.ts

@@ -7,7 +7,6 @@ import { IEvent } from 'common/EventEmitter';
 import { CharData, IColor, ICoreTerminal, ITerminalOptions } from 'common/Types';
 import { IBuffer } from 'common/buffer/Types';
 import { IDisposable, Terminal as ITerminalApi } from '@xterm/xterm';
-import { IMouseService, IRenderService } from './services/Services';
 
 /**
  * A portion of the public API that are implemented identially internally and simply passed through.
@@ -18,9 +17,9 @@ export interface ITerminal extends InternalPassthroughApis, ICoreTerminal {
   screenElement: HTMLElement | undefined;
   browser: IBrowser;
   buffer: IBuffer;
+  linkifier: ILinkifier2 | undefined;
   viewport: IViewport | undefined;
   options: Required<ITerminalOptions>;
-  linkifier2: ILinkifier2;
 
   onBlur: IEvent<void>;
   onFocus: IEvent<void>;
@@ -128,13 +127,6 @@ export interface ILinkifier2 extends IDisposable {
   onShowLinkUnderline: IEvent<ILinkifierEvent>;
   onHideLinkUnderline: IEvent<ILinkifierEvent>;
   readonly currentLink: ILinkWithState | undefined;
-
-  attachToDom(element: HTMLElement, mouseService: IMouseService, renderService: IRenderService): void;
-  registerLinkProvider(linkProvider: ILinkProvider): IDisposable;
-}
-
-interface ILinkProvider {
-  provideLinks(y: number, callback: (links: ILink[] | undefined) => void): void;
 }
 
 interface ILink {

src/browser/services/LinkProviderService.ts

@@ -0,0 +1,28 @@
+import { ILinkProvider, ILinkProviderService } from 'browser/services/Services';
+import { Disposable, toDisposable } from 'common/Lifecycle';
+import { IDisposable } from 'common/Types';
+
+export class LinkProviderService extends Disposable implements ILinkProviderService {
+  declare public serviceBrand: undefined;
+
+  public readonly linkProviders: ILinkProvider[] = [];
+
+  constructor() {
+    super();
+    this.register(toDisposable(() => this.linkProviders.length = 0));
+  }
+
+  public registerLinkProvider(linkProvider: ILinkProvider): IDisposable {
+    this.linkProviders.push(linkProvider);
+    return {
+      dispose: () => {
+        // Remove the link provider from the list
+        const providerIndex = this.linkProviders.indexOf(linkProvider);
+
+        if (providerIndex !== -1) {
+          this.linkProviders.splice(providerIndex, 1);
+        }
+      }
+    };
+  }
+}

src/browser/services/Services.ts

@@ -5,7 +5,7 @@
 
 import { IEvent } from 'common/EventEmitter';
 import { IRenderDimensions, IRenderer } from 'browser/renderer/shared/Types';
-import { IColorSet, ReadonlyColorSet } from 'browser/Types';
+import { IColorSet, ILink, ReadonlyColorSet } from 'browser/Types';
 import { ISelectionRedrawRequestEvent as ISelectionRequestRedrawEvent, ISelectionRequestScrollLinesEvent } from 'browser/selection/Types';
 import { createDecorator } from 'common/services/ServiceRegistry';
 import { AllColorIndex, IDisposable } from 'common/Types';
@@ -145,3 +145,14 @@ export interface IThemeService {
    */
   modifyColors(callback: (colors: IColorSet) => void): void;
 }
+
+
+export const ILinkProviderService = createDecorator<ILinkProviderService>('LinkProviderService');
+export interface ILinkProviderService extends IDisposable {
+  serviceBrand: undefined;
+  readonly linkProviders: ReadonlyArray<ILinkProvider>;
+  registerLinkProvider(linkProvider: ILinkProvider): IDisposable;
+}
+export interface ILinkProvider {
+  provideLinks(y: number, callback: (links: ILink[] | undefined) => void): void;
+}

src/common/CoreTerminal.ts

@@ -120,6 +120,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
     this._oscLinkService = this._instantiationService.createInstance(OscLinkService);
     this._instantiationService.setService(IOscLinkService, this._oscLinkService);
 
+
     // Register input handler and handle/forward events
     this._inputHandler = this.register(new InputHandler(this._bufferService, this._charsetService, this.coreService, this._logService, this.optionsService, this._oscLinkService, this.coreMouseService, this.unicodeService));
     this.register(forwardEvent(this._inputHandler.onLineFeed, this._onLineFeed));