Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 6 commits into from
Dec 23, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8733d2c
Separate link provider service from linkifier
Tyriar Dec 23, 2023
de41c9f
Make linkifier always attached to dom
Tyriar Dec 23, 2023
d7c8d06
Get linkifier test to work
Tyriar Dec 23, 2023
dbcb43f
Rename Linkifier2 -> Linkifier
Tyriar Dec 23, 2023
1b52c97
Add wait for builld job to test-unit
Tyriar Dec 23, 2023
53604e4
Update test-unit node to 18
Tyriar Dec 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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