Skip to content

Commit

Permalink
Merge pull request #4924 from Tyriar/link_refactor
Browse files Browse the repository at this point in the history 
Split Linkifier2 (always active) into Linkifier (after attach) and LinkProviderService (always active)
  • Loading branch information
@Tyriar
Tyriar committed Dec 23, 2023
2 parents b3737d6 + 53604e4 commit 6e351dd
Show file tree
Hide file tree
Showing 12 changed files with 112 additions and 93 deletions.
7 changes: 6 additions & 1 deletion .github/workflows/ci.yml
View file
Expand Up @@ -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:
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion addons/addon-canvas/src/CanvasAddon.ts
View file
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion addons/addon-webgl/src/WebglRenderer.ts
View file
Expand Up @@ -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;
Expand Down
9 changes: 6 additions & 3 deletions src/browser/Linkifier2.test.ts → src/browser/Linkifier.test.ts
View file
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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: {
Expand Down
96 changes: 34 additions & 62 deletions src/browser/Linkifier2.ts → src/browser/Linkifier.ts
View file
Expand Up @@ -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;
Expand All @@ -33,42 +29,24 @@ 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.
this.register(this._bufferService.onResize(() => {
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();
Expand All @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}
});
Expand Down Expand Up @@ -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));
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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);
}
}
}));
}
}
}));
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/browser/OscLinkProvider.ts
View file
Expand Up @@ -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';

Expand Down
32 changes: 19 additions & 13 deletions src/browser/Terminal.ts
View file
Expand Up @@ -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';
Expand All @@ -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';
Expand All @@ -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;
Expand All @@ -69,15 +70,19 @@ 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;

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;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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);

Expand All @@ -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()));
Expand All @@ -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)));
Expand All @@ -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)));

Expand Down Expand Up @@ -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!);
}

/**
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 6e351dd

Please sign in to comment.