From 39c2e57ad326d39db9ff2022d350a3575a7b7dd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 4 Dec 2022 22:26:29 +0100 Subject: [PATCH] hack around DOM renderer issue, fix api tests --- .../src/WebLinkProvider.ts | 19 ++++++------------- .../src/WebLinksAddon.ts | 7 ++++--- .../test/WebLinksAddon.api.ts | 4 ++++ src/browser/renderer/dom/DomRenderer.ts | 2 ++ .../renderer/dom/DomRendererRowFactory.ts | 5 +++++ 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/addons/xterm-addon-web-links/src/WebLinkProvider.ts b/addons/xterm-addon-web-links/src/WebLinkProvider.ts index 501eba99..0dd4c9be 100644 --- a/addons/xterm-addon-web-links/src/WebLinkProvider.ts +++ b/addons/xterm-addon-web-links/src/WebLinkProvider.ts @@ -46,13 +46,12 @@ export class LinkComputer { const rex = new RegExp(regex.source, (regex.flags || '') + 'g'); const [lines, startLineIndex] = LinkComputer._getFullLineString(y - 1, terminal); - - // TODO: do locally limited search if string is too long const line = lines.join(''); // Don't try if the wrapped line if excessively large as the regex matching will block the main // thread. if (line.length > 1024) { + // TODO: more sophisticated handling with individual line introspection? return []; } @@ -62,12 +61,6 @@ export class LinkComputer { while ((match = rex.exec(line)) !== null) { const text = match[0]; - if (!text) { - // something matched but does not comply with the given matchIndex - // since this is most likely a bug the regex itself we simply do nothing here - console.log('match found without corresponding matchIndex'); - break; - } // Get index, match.index is for the outer match which includes negated chars // therefore we cannot use match.index directly, instead we search the position @@ -99,8 +92,8 @@ export class LinkComputer { } - const [startY, startX] = LinkComputer._mapStringIndexToBuffer(startLineIndex, stringIndex, terminal); - const [endY, endX] = LinkComputer._mapStringIndexToBuffer(startLineIndex, stringIndex + text.length, terminal); + const [startY, startX] = LinkComputer._mapStrIdx(startLineIndex, stringIndex, terminal); + const [endY, endX] = LinkComputer._mapStrIdx(startLineIndex, stringIndex + text.length, terminal); if (startY === -1 || startX === -1 || endY === -1 || endX === -1) { continue; @@ -112,8 +105,8 @@ export class LinkComputer { y: startY + 1 }, end: { - x: endX + 1, - y: endY + x: endX, + y: endY + 1 } }; @@ -155,7 +148,7 @@ export class LinkComputer { * Returns buffer position as [lineIndex, columnIndex] 0-based, * or [-1, -1] in case the lookup ran into a non-existing line. */ - private static _mapStringIndexToBuffer(lineIndex: number, stringIndex: number, terminal: Terminal): [number, number] { + private static _mapStrIdx(lineIndex: number, stringIndex: number, terminal: Terminal): [number, number] { const buf = terminal.buffer.active; const cell = buf.getNullCell(); while (stringIndex) { diff --git a/addons/xterm-addon-web-links/src/WebLinksAddon.ts b/addons/xterm-addon-web-links/src/WebLinksAddon.ts index c71753ff..ccf45e74 100644 --- a/addons/xterm-addon-web-links/src/WebLinksAddon.ts +++ b/addons/xterm-addon-web-links/src/WebLinksAddon.ts @@ -7,9 +7,10 @@ import { Terminal, ITerminalAddon, IDisposable } from 'xterm'; import { ILinkProviderOptions, WebLinkProvider } from './WebLinkProvider'; // consider everthing starting with http:// or https:// -// up to first whitespace as url -// gets further narrowed down with URL later on -const strictUrlRegex = /https?:[/]{2}\S*/; +// up to first whitespace, `"` or `'` as url +// NOTE: The repeated end clause is needed to not match a dangling `:` +// resembling the old (...)*([^:"\'\\s]) final path clause +const strictUrlRegex = /https?:[/]{2}[^\s^"^']*[^\s^"^'^:]/; function handleLink(event: MouseEvent, uri: string): void { const newWindow = window.open(); diff --git a/addons/xterm-addon-web-links/test/WebLinksAddon.api.ts b/addons/xterm-addon-web-links/test/WebLinksAddon.api.ts index bc978085..9bcda0a5 100644 --- a/addons/xterm-addon-web-links/test/WebLinksAddon.api.ts +++ b/addons/xterm-addon-web-links/test/WebLinksAddon.api.ts @@ -35,6 +35,10 @@ describe('WebLinksAddon', () => { it('.io', async function(): Promise { await testHostName('foo.io'); }); + + it.skip('correct buffer offsets', async () => { + // TODO: test strings in test_weblinks.sh automatically + }); }); async function testHostName(hostname: string): Promise { diff --git a/src/browser/renderer/dom/DomRenderer.ts b/src/browser/renderer/dom/DomRenderer.ts index 72f7e254..094e6a8e 100644 --- a/src/browser/renderer/dom/DomRenderer.ts +++ b/src/browser/renderer/dom/DomRenderer.ts @@ -376,6 +376,8 @@ export class DomRenderer extends Disposable implements IRenderer { } private _setCellUnderline(x: number, x2: number, y: number, y2: number, cols: number, enabled: boolean): void { + // FIXME: offset calculation is wrong (temp. fixed by adding empty spans for wide chars + // to fullfill column to element index identity assumption) while (x !== x2 || y !== y2) { const row = this._rowElements[y]; if (!row) { diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index 14b26c92..e4722186 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -32,6 +32,7 @@ export class DomRendererRowFactory { private _selectionStart: [number, number] | undefined; private _selectionEnd: [number, number] | undefined; private _columnSelectMode: boolean = false; + private _nullSpan: HTMLSpanElement; constructor( private readonly _document: Document, @@ -42,6 +43,8 @@ export class DomRendererRowFactory { @IDecorationService private readonly _decorationService: IDecorationService, @IThemeService private readonly _themeService: IThemeService ) { + this._nullSpan = this._document.createElement('span'); + this._nullSpan.style.width = `0`; } public handleSelectionChanged(start: [number, number] | undefined, end: [number, number] | undefined, columnSelectMode: boolean): void { @@ -75,6 +78,8 @@ export class DomRendererRowFactory { // The character to the left is a wide character, drawing is owned by the char at x-1 if (width === 0) { + // hack: fix underline bug for wide chars by appending an empty span + fragment.appendChild(this._nullSpan.cloneNode()); continue; }