From 8733d2cfa24d5d81d3f3b398d413be54537cbe1a Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 23 Dec 2023 08:21:35 -0800 Subject: [PATCH 1/6] Separate link provider service from linkifier --- addons/addon-canvas/src/CanvasAddon.ts | 2 +- addons/addon-webgl/src/WebglRenderer.ts | 2 +- src/browser/Linkifier2.test.ts | 3 +- src/browser/Linkifier2.ts | 29 +++++-------------- src/browser/OscLinkProvider.ts | 3 +- src/browser/Terminal.ts | 32 +++++++++++++-------- src/browser/TestUtils.test.ts | 2 +- src/browser/Types.d.ts | 9 ++---- src/browser/services/LinkProviderService.ts | 28 ++++++++++++++++++ src/browser/services/Services.ts | 13 ++++++++- src/common/CoreTerminal.ts | 1 + 11 files changed, 77 insertions(+), 47 deletions(-) create mode 100644 src/browser/services/LinkProviderService.ts diff --git a/addons/addon-canvas/src/CanvasAddon.ts b/addons/addon-canvas/src/CanvasAddon.ts index 7f7f679bc1..4d32b02146 100644 --- a/addons/addon-canvas/src/CanvasAddon.ts +++ b/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; diff --git a/addons/addon-webgl/src/WebglRenderer.ts b/addons/addon-webgl/src/WebglRenderer.ts index f2f2c83e10..3a01e244b9 100644 --- a/addons/addon-webgl/src/WebglRenderer.ts +++ b/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; diff --git a/src/browser/Linkifier2.test.ts b/src/browser/Linkifier2.test.ts index 0af74c283f..569aba84e2 100644 --- a/src/browser/Linkifier2.test.ts +++ b/src/browser/Linkifier2.test.ts @@ -8,6 +8,7 @@ import { IBufferService } from 'common/services/Services'; import { Linkifier2 } from 'browser/Linkifier2'; import { MockBufferService } from 'common/TestUtils.test'; import { ILink } from 'browser/Types'; +import { LinkProviderService } from 'browser/services/LinkProviderService'; class TestLinkifier2 extends Linkifier2 { public set currentLink(link: any) { @@ -44,7 +45,7 @@ describe('Linkifier2', () => { beforeEach(() => { bufferService = new MockBufferService(100, 10); - linkifier = new TestLinkifier2(bufferService); + linkifier = new TestLinkifier2(bufferService, new LinkProviderService()); linkifier.currentLink = { link, state: { diff --git a/src/browser/Linkifier2.ts b/src/browser/Linkifier2.ts index f7b1f605eb..3bc38f8be5 100644 --- a/src/browser/Linkifier2.ts +++ b/src/browser/Linkifier2.ts @@ -4,18 +4,17 @@ */ 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[] = []; public get currentLink(): ILinkWithState | undefined { return this._currentLink; } protected _currentLink: ILinkWithState | undefined; private _mouseDownLink: ILinkWithState | undefined; @@ -33,14 +32,14 @@ export class Linkifier2 extends Disposable implements ILinkifier2 { public readonly onHideLinkUnderline = this._onHideLinkUnderline.event; constructor( - @IBufferService private readonly _bufferService: IBufferService + @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. @@ -50,20 +49,6 @@ export class Linkifier2 extends Disposable implements ILinkifier2 { })); } - 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; @@ -145,7 +130,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 +152,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 +208,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)); diff --git a/src/browser/OscLinkProvider.ts b/src/browser/OscLinkProvider.ts index fee1ae7c04..a079fe67e6 100644 --- a/src/browser/OscLinkProvider.ts +++ b/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'; diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 01dfbc89cd..246001318c 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -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 = 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,13 @@ 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(Linkifier2)); + // TODO: Move into ctor + this.linkifier.attachToDom(this.screenElement, this._mouseService, this._renderService); + // Performance: Add viewport and helper elements from the fragment this.element.appendChild(fragment); @@ -493,9 +505,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 +522,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 +542,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 +583,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 +902,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 { diff --git a/src/browser/TestUtils.test.ts b/src/browser/TestUtils.test.ts index a969ec2cad..59cc773a47 100644 --- a/src/browser/TestUtils.test.ts +++ b/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; public element!: HTMLElement; diff --git a/src/browser/Types.d.ts b/src/browser/Types.d.ts index d1ad5f87b7..d27c0ee796 100644 --- a/src/browser/Types.d.ts +++ b/src/browser/Types.d.ts @@ -7,7 +7,7 @@ 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'; +import { IMouseService, IRenderService } from 'browser/services/Services'; /** * A portion of the public API that are implemented identially internally and simply passed through. @@ -18,9 +18,9 @@ export interface ITerminal extends InternalPassthroughApis, ICoreTerminal { screenElement: HTMLElement | undefined; browser: IBrowser; buffer: IBuffer; + linkifier: ILinkifier2 | undefined; viewport: IViewport | undefined; options: Required; - linkifier2: ILinkifier2; onBlur: IEvent; onFocus: IEvent; @@ -130,11 +130,6 @@ export interface ILinkifier2 extends IDisposable { 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 { diff --git a/src/browser/services/LinkProviderService.ts b/src/browser/services/LinkProviderService.ts new file mode 100644 index 0000000000..2590f24bdc --- /dev/null +++ b/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); + } + } + }; + } +} diff --git a/src/browser/services/Services.ts b/src/browser/services/Services.ts index 5c14fa8af6..a82eabd098 100644 --- a/src/browser/services/Services.ts +++ b/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('LinkProviderService'); +export interface ILinkProviderService extends IDisposable { + serviceBrand: undefined; + readonly linkProviders: ReadonlyArray; + registerLinkProvider(linkProvider: ILinkProvider): IDisposable; +} +export interface ILinkProvider { + provideLinks(y: number, callback: (links: ILink[] | undefined) => void): void; +} diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 47f77406c5..1789daf8bd 100644 --- a/src/common/CoreTerminal.ts +++ b/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)); From de41c9f306ffcc4180855764c31e7086b90879b8 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 23 Dec 2023 08:29:28 -0800 Subject: [PATCH 2/6] Make linkifier always attached to dom --- src/browser/Linkifier2.test.ts | 4 +-- src/browser/Linkifier2.ts | 65 ++++++++++++++-------------------- src/browser/Terminal.ts | 6 ++-- src/browser/Types.d.ts | 3 -- 4 files changed, 30 insertions(+), 48 deletions(-) diff --git a/src/browser/Linkifier2.test.ts b/src/browser/Linkifier2.test.ts index 569aba84e2..07680a23d7 100644 --- a/src/browser/Linkifier2.test.ts +++ b/src/browser/Linkifier2.test.ts @@ -5,7 +5,7 @@ import { assert } from 'chai'; import { IBufferService } from 'common/services/Services'; -import { Linkifier2 } from 'browser/Linkifier2'; +import { Linkifier2 } from './Linkifier2'; import { MockBufferService } from 'common/TestUtils.test'; import { ILink } from 'browser/Types'; import { LinkProviderService } from 'browser/services/LinkProviderService'; @@ -45,7 +45,7 @@ describe('Linkifier2', () => { beforeEach(() => { bufferService = new MockBufferService(100, 10); - linkifier = new TestLinkifier2(bufferService, new LinkProviderService()); + linkifier = new TestLinkifier2(null!, null!, null!, bufferService, new LinkProviderService()); linkifier.currentLink = { link, state: { diff --git a/src/browser/Linkifier2.ts b/src/browser/Linkifier2.ts index 3bc38f8be5..4c0fc9dce2 100644 --- a/src/browser/Linkifier2.ts +++ b/src/browser/Linkifier2.ts @@ -12,9 +12,6 @@ import { IBufferService } from 'common/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; public get currentLink(): ILinkWithState | undefined { return this._currentLink; } protected _currentLink: ILinkWithState | undefined; private _mouseDownLink: ILinkWithState | undefined; @@ -32,6 +29,9 @@ export class Linkifier2 extends Disposable implements ILinkifier2 { public readonly onHideLinkUnderline = this._onHideLinkUnderline.event; constructor( + private readonly _element: HTMLElement, + @IMouseService private readonly _mouseService: IMouseService, + @IRenderService private readonly _renderService: IRenderService, @IBufferService private readonly _bufferService: IBufferService, @ILinkProviderService private readonly _linkProviderService: ILinkProviderService ) { @@ -47,13 +47,6 @@ export class Linkifier2 extends Disposable implements ILinkifier2 { this._clearCurrentLink(); this._wasResized = true; })); - } - - 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(); @@ -66,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; @@ -228,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; } @@ -243,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; } @@ -256,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; } @@ -287,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); } } } @@ -307,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); } } - })); - } + } + })); } } diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 246001318c..a92a424440 100644 --- a/src/browser/Terminal.ts +++ b/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 { Linkifier2 } from './Linkifier2'; import * as Strings from 'browser/LocalizableStrings'; import { OscLinkProvider } from 'browser/OscLinkProvider'; import { CharacterJoinerHandler, CustomKeyEventHandler, CustomWheelEventHandler, IBrowser, IBufferRange, ICompositionHelper, ILinkifier2, ITerminal, IViewport } from 'browser/Types'; @@ -490,9 +490,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this._mouseService = this._instantiationService.createInstance(MouseService); this._instantiationService.setService(IMouseService, this._mouseService); - this.linkifier = this.register(this._instantiationService.createInstance(Linkifier2)); - // TODO: Move into ctor - this.linkifier.attachToDom(this.screenElement, this._mouseService, this._renderService); + this.linkifier = this.register(this._instantiationService.createInstance(Linkifier2, this.screenElement)); // Performance: Add viewport and helper elements from the fragment this.element.appendChild(fragment); diff --git a/src/browser/Types.d.ts b/src/browser/Types.d.ts index d27c0ee796..9ebc55d96f 100644 --- a/src/browser/Types.d.ts +++ b/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 'browser/services/Services'; /** * A portion of the public API that are implemented identially internally and simply passed through. @@ -128,8 +127,6 @@ export interface ILinkifier2 extends IDisposable { onShowLinkUnderline: IEvent; onHideLinkUnderline: IEvent; readonly currentLink: ILinkWithState | undefined; - - attachToDom(element: HTMLElement, mouseService: IMouseService, renderService: IRenderService): void; } interface ILink { From d7c8d063b6f6fc1a316868a134ad601dbe2a13b3 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 23 Dec 2023 08:31:50 -0800 Subject: [PATCH 3/6] Get linkifier test to work --- src/browser/Linkifier2.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/browser/Linkifier2.test.ts b/src/browser/Linkifier2.test.ts index 07680a23d7..9176170cf4 100644 --- a/src/browser/Linkifier2.test.ts +++ b/src/browser/Linkifier2.test.ts @@ -9,6 +9,7 @@ import { Linkifier2 } from './Linkifier2'; 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 { public set currentLink(link: any) { @@ -44,8 +45,9 @@ describe('Linkifier2', () => { }; beforeEach(() => { + const dom = new jsdom.JSDOM(); bufferService = new MockBufferService(100, 10); - linkifier = new TestLinkifier2(null!, null!, null!, bufferService, new LinkProviderService()); + linkifier = new TestLinkifier2(dom.window.document.createElement('div'), null!, null!, bufferService, new LinkProviderService()); linkifier.currentLink = { link, state: { From dbcb43f1d36cc2dd0000232d97cd0a9884a9c4e8 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 23 Dec 2023 08:32:39 -0800 Subject: [PATCH 4/6] Rename Linkifier2 -> Linkifier --- src/browser/{Linkifier2.test.ts => Linkifier.test.ts} | 4 ++-- src/browser/{Linkifier2.ts => Linkifier.ts} | 2 +- src/browser/Terminal.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) rename src/browser/{Linkifier2.test.ts => Linkifier.test.ts} (96%) rename src/browser/{Linkifier2.ts => Linkifier.ts} (99%) diff --git a/src/browser/Linkifier2.test.ts b/src/browser/Linkifier.test.ts similarity index 96% rename from src/browser/Linkifier2.test.ts rename to src/browser/Linkifier.test.ts index 9176170cf4..4294ef105d 100644 --- a/src/browser/Linkifier2.test.ts +++ b/src/browser/Linkifier.test.ts @@ -5,13 +5,13 @@ import { assert } from 'chai'; import { IBufferService } from 'common/services/Services'; -import { Linkifier2 } from './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; } diff --git a/src/browser/Linkifier2.ts b/src/browser/Linkifier.ts similarity index 99% rename from src/browser/Linkifier2.ts rename to src/browser/Linkifier.ts index 4c0fc9dce2..ac37e42f18 100644 --- a/src/browser/Linkifier2.ts +++ b/src/browser/Linkifier.ts @@ -11,7 +11,7 @@ import { IDisposable } from 'common/Types'; import { IBufferService } from 'common/services/Services'; import { ILinkProviderService, IMouseService, IRenderService } from './services/Services'; -export class Linkifier2 extends Disposable implements ILinkifier2 { +export class Linkifier extends Disposable implements ILinkifier2 { public get currentLink(): ILinkWithState | undefined { return this._currentLink; } protected _currentLink: ILinkWithState | undefined; private _mouseDownLink: ILinkWithState | undefined; diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index a92a424440..de8278c66d 100644 --- a/src/browser/Terminal.ts +++ b/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 './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'; @@ -490,7 +490,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this._mouseService = this._instantiationService.createInstance(MouseService); this._instantiationService.setService(IMouseService, this._mouseService); - this.linkifier = this.register(this._instantiationService.createInstance(Linkifier2, this.screenElement)); + this.linkifier = this.register(this._instantiationService.createInstance(Linkifier, this.screenElement)); // Performance: Add viewport and helper elements from the fragment this.element.appendChild(fragment); From 1b52c97fd01ebbae89995fc71ed019aba479cfe8 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 23 Dec 2023 08:46:09 -0800 Subject: [PATCH 5/6] Add wait for builld job to test-unit --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a64b6a7cc..045eb9e403 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 From 53604e427ba05af231c08cd1123d223bdf39e128 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 23 Dec 2023 08:49:37 -0800 Subject: [PATCH 6/6] Update test-unit node to 18 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 045eb9e403..c985c0d092 100644 --- a/.github/workflows/ci.yml +++ b/.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: