Fix weblinks

addons/xterm-addon-web-links/src/WebLinkProvider.ts

@@ -45,26 +45,15 @@ export class LinkComputer {
   public static computeLink(y: number, regex: RegExp, terminal: Terminal, activate: (event: MouseEvent, uri: string) => void): ILink[] {
     const rex = new RegExp(regex.source, (regex.flags || '') + 'g');
 
-    const [line, startLineIndex] = LinkComputer._translateBufferLineToStringWithWrap(y - 1, false, terminal);
-
-    // Don't try if the wrapped line if excessively large as the regex matching will block the main
-    // thread.
-    if (line.length > 1024) {
-      return [];
-    }
+    const [lines, startLineIndex] = LinkComputer._getWindowedLineStrings(y - 1, terminal);
+    const line = lines.join('');
 
     let match;
     let stringIndex = -1;
     const result: ILink[] = [];
 
-    while ((match = rex.exec(line)) !== null) {
-      const text = match[1];
-      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;
-      }
+    while (match = rex.exec(line)) {
+      const text = match[0];
 
       // 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
@@ -77,29 +66,40 @@ export class LinkComputer {
         break;
       }
 
-      let endX = stringIndex + text.length;
-      let endY = startLineIndex + 1;
-
-      while (endX > terminal.cols) {
-        endX -= terminal.cols;
-        endY++;
+      // check via URL if the matched text would form a proper url
+      // NOTE: This outsources the ugly url parsing to the browser.
+      // To avoid surprising auto expansion from URL we additionally
+      // check afterwards if the provided string resembles the parsed
+      // one close enough:
+      // - decodeURI  decode path segement back to byte repr
+      //              to detect unicode auto conversion correctly
+      // - append /   also match domain urls w'o any path notion
+      try {
+        const url = new URL(text);
+        const urlText = decodeURI(url.toString());
+        if (text !== urlText && text + '/' !== urlText) {
+          continue;
+        }
+      } catch (e) {
+        continue;
       }
 
-      let startX = stringIndex + 1;
-      let startY = startLineIndex + 1;
-      while (startX > terminal.cols) {
-        startX -= terminal.cols;
-        startY++;
+
+      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;
       }
 
       const range = {
         start: {
-          x: startX,
-          y: startY
+          x: startX + 1,
+          y: startY + 1
         },
         end: {
           x: endX,
-          y: endY
+          y: endY + 1
         }
       };
 
@@ -110,41 +110,88 @@ export class LinkComputer {
   }
 
   /**
-   * Gets the entire line for the buffer line
-   * @param lineIndex The index of the line being translated.
-   * @param trimRight Whether to trim whitespace to the right.
+   * Get wrapped content lines for the current line index.
+   * The top/bottom line expansion stops at whitespaces or length > 2048.
+   * Returns an array with line strings and the top line index.
    */
-  private static _translateBufferLineToStringWithWrap(lineIndex: number, trimRight: boolean, terminal: Terminal): [string, number] {
-    let lineString = '';
-    let lineWrapsToNext: boolean;
-    let prevLinesToWrap: boolean;
-
-    do {
-      const line = terminal.buffer.active.getLine(lineIndex);
-      if (!line) {
-        break;
+  private static _getWindowedLineStrings(lineIndex: number, terminal: Terminal): [string[], number] {
+    let line: any;
+    let topIdx = lineIndex;
+    let bottomIdx = lineIndex;
+    let length = 0;
+    let content = '';
+    const lines: string[] = [];
+
+    if ((line = terminal.buffer.active.getLine(lineIndex))) {
+      const currentContent = line.translateToString(true);
+
+      // expand top, stop on whitespaces or length > 2048
+      if (line.isWrapped && currentContent[0] !== ' ') {
+        length = 0;
+        while ((line = terminal.buffer.active.getLine(--topIdx)) && length < 2048) {
+          content = line.translateToString(true);
+          length += content.length;
+          lines.push(content);
+          if (!line.isWrapped || content.indexOf(' ') !== -1) {
+            break;
+          }
+        }
+        lines.reverse();
       }
 
-      if (line.isWrapped) {
-        lineIndex--;
+      // append current line
+      lines.push(currentContent);
+
+      // expand bottom, stop on whitespaces or length > 2048
+      length = 0;
+      while ((line = terminal.buffer.active.getLine(++bottomIdx)) && line.isWrapped && length < 2048) {
+        content = line.translateToString(true);
+        length += content.length;
+        lines.push(content);
+        if (content.indexOf(' ') !== -1) {
+          break;
+        }
       }
+    }
+    return [lines, topIdx];
+  }
 
-      prevLinesToWrap = line.isWrapped;
-    } while (prevLinesToWrap);
-
-    const startLineIndex = lineIndex;
-
-    do {
-      const nextLine = terminal.buffer.active.getLine(lineIndex + 1);
-      lineWrapsToNext = nextLine ? nextLine.isWrapped : false;
-      const line = terminal.buffer.active.getLine(lineIndex);
+  /**
+   * Map a string index back to buffer positions.
+   * Returns buffer position as [lineIndex, columnIndex] 0-based,
+   * or [-1, -1] in case the lookup ran into a non-existing line.
+   */
+  private static _mapStrIdx(lineIndex: number, stringIndex: number, terminal: Terminal): [number, number] {
+    const buf = terminal.buffer.active;
+    const cell = buf.getNullCell();
+    while (stringIndex) {
+      const line = buf.getLine(lineIndex);
       if (!line) {
-        break;
+        return [-1, -1];
+      }
+      for (let i = 0; i < line.length; ++i) {
+        line.getCell(i, cell);
+        const chars = cell.getChars();
+        const width = cell.getWidth();
+        if (width) {
+          stringIndex -= chars.length || 1;
+        }
+        // look ahead for early wrap around of wide chars
+        if (i === line.length - 1 && chars === '' && width) {
+          const line = buf.getLine(lineIndex + 1);
+          if (line && line.isWrapped) {
+            line.getCell(0, cell);
+            if (cell.getWidth() === 2) {
+              stringIndex += 1;
+            }
+          }
+        }
+        if (stringIndex < 0) {
+          return [lineIndex, i];
+        }
       }
-      lineString += line.translateToString(!lineWrapsToNext && trimRight).substring(0, terminal.cols);
       lineIndex++;
-    } while (lineWrapsToNext);
-
-    return [lineString, startLineIndex];
+    }
+    return [lineIndex, 0];
   }
 }

addons/xterm-addon-web-links/src/WebLinksAddon.ts

@@ -6,25 +6,12 @@
 import { Terminal, ITerminalAddon, IDisposable } from 'xterm';
 import { ILinkProviderOptions, WebLinkProvider } from './WebLinkProvider';
 
-const protocolClause = '(https?:\\/\\/)';
-const domainCharacterSet = '[\\da-z\\.-]+';
-const negatedDomainCharacterSet = '[^\\da-z\\.-]+';
-const domainBodyClause = '(' + domainCharacterSet + ')';
-const tldClause = '([a-z\\.]{2,18})';
-const ipClause = '((\\d{1,3}\\.){3}\\d{1,3})';
-const localHostClause = '(localhost)';
-const portClause = '(:\\d{1,5})';
-const hostClause = '((' + domainBodyClause + '\\.' + tldClause + ')|' + ipClause + '|' + localHostClause + ')' + portClause + '?';
-const pathCharacterSet = '(\\/[\\/\\w\\.\\-%~:+@]*)*([^:"\'\\s])';
-const pathClause = '(' + pathCharacterSet + ')?';
-const queryStringHashFragmentCharacterSet = '[0-9\\w\\[\\]\\(\\)\\/\\?\\!#@$%&\'*+,:;~\\=\\.\\-]*';
-const queryStringClause = '(\\?' + queryStringHashFragmentCharacterSet + ')?';
-const hashFragmentClause = '(#' + queryStringHashFragmentCharacterSet + ')?';
-const negatedPathCharacterSet = '[^\\/\\w\\.\\-%]+';
-const bodyClause = hostClause + pathClause + queryStringClause + hashFragmentClause;
-const start = '(?:^|' + negatedDomainCharacterSet + ')(';
-const end = ')($|' + negatedPathCharacterSet + ')';
-const strictUrlRegex = new RegExp(start + protocolClause + bodyClause + end);
+// consider everthing starting with http:// or https://
+// 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
+// also exclude final interpunction like ,.!?
+const strictUrlRegex = /https?:[/]{2}[^\s^"^']*[^\s^"^'^:^,^.^!^?]/;
 
 function handleLink(event: MouseEvent, uri: string): void {
   const newWindow = window.open();

addons/xterm-addon-web-links/test/WebLinksAddon.api.ts

@@ -35,6 +35,10 @@ describe('WebLinksAddon', () => {
   it('.io', async function(): Promise<any> {
     await testHostName('foo.io');
   });
+
+  it.skip('correct buffer offsets', async () => {
+    // TODO: test strings in test_weblinks.sh automatically
+  });
 });
 
 async function testHostName(hostname: string): Promise<void> {

bin/test_weblinks.sh

@@ -0,0 +1,25 @@
+#!/bin/bash
+
+# all half width - only good case
+echo "aaa http://example.com aaa http://example.com aaa"
+
+# full width before - wrong offset
+echo "￥￥￥ http://example.com aaa http://example.com aaa"
+
+# full width between - wrong offset
+echo "aaa http://example.com ￥￥￥ http://example.com aaa"
+
+# full width before and between - error in offsets adding up
+echo "￥￥￥ http://example.com ￥￥￥ http://example.com aaa"
+
+# full width within url - partial wrong match
+echo "aaa https://ko.wikipedia.org/wiki/위키백과:대문 aaa https://ko.wikipedia.org/wiki/위키백과:대문 aaa"
+
+# full width within and before - partial wrong match + wrong offsets
+echo "￥￥￥ https://ko.wikipedia.org/wiki/위키백과:대문 aaa https://ko.wikipedia.org/wiki/위키백과:대문 ￥￥￥"
+
+# not matching at all
+echo "http://test:password@example.com/some_path aaa"
+
+# overly long text with urls with final interpunction
+echo "Lorem ipsum dolor sit amet, consetetur sadipscing elitr https://ko.wikipedia.org/wiki/위키백과:대문, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Duis autem vel eum iriure dolor in hendrerit in vulputate velit esse molestie consequat, vel illum dolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio dignissim qui blandit praesent luptatum zzril delenit augue duis dolore te feugait nulla facilisi. Lorem ipsum dolor sit amet, consectetuer adipiscing elit, sed diam nonummy nibh euismod tincidunt ut laoreet dolore magna aliquam erat volutpat. Ut wisi enim ad minim veniam, quis nostrud exerci tation ullamcorper suscipit lobortis nisl ut aliquip ex ea commodo consequat. Duis autem vel eum iriure dolor in hendrerit in vulputate velit esse molestie consequat, vel illum dolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio dignissim qui blandit praesent luptatum zzril delenit augue duis dolore te feugait nulla facilisi. Nam liber tempor cum soluta nobis eleifend option congue nihil imperdiet doming id quod mazim placerat facer possim assum. Lorem ipsum dolor sit amet, consectetuer adipiscing elit, sed diam nonummy nibh euismod tincidunt ut laoreet dolore magna aliquam erat volutpat: http://test:password@example.com/some_path."

src/browser/Linkifier2.ts

@@ -157,13 +157,15 @@ 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) {
-            this._removeIntersectingLinks(position.y, this._activeProviderReplies);
+            // FIXME: commented out due to bug below
+            // this._removeIntersectingLinks(position.y, this._activeProviderReplies);
           }
         });
       }
     }
   }
 
+  // FIXME: What is this supposed to do? Currently it removes wrongly a second link on a wrapped line...
   private _removeIntersectingLinks(y: number, replies: Map<Number, ILinkWithState[] | undefined>): void {
     const occupiedCells = new Set<number>();
     for (let i = 0; i < replies.size; i++) {
@@ -367,18 +369,10 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
    * @param position
    */
   private _linkAtPosition(link: ILink, position: IBufferCellPosition): boolean {
-    const sameLine = link.range.start.y === link.range.end.y;
-    const wrappedFromLeft = link.range.start.y < position.y;
-    const wrappedToRight = link.range.end.y > position.y;
-
-    // If the start and end have the same y, then the position must be between start and end x
-    // If not, then handle each case seperately, depending on which way it wraps
-    return ((sameLine && link.range.start.x <= position.x && link.range.end.x >= position.x) ||
-      (wrappedFromLeft && link.range.end.x >= position.x) ||
-      (wrappedToRight && link.range.start.x <= position.x) ||
-      (wrappedFromLeft && wrappedToRight)) &&
-      link.range.start.y <= position.y &&
-      link.range.end.y >= position.y;
+    const lower = link.range.start.y * this._bufferService.cols + link.range.start.x;
+    const upper = link.range.end.y * this._bufferService.cols + link.range.end.x;
+    const current = position.y * this._bufferService.cols + position.x;
+    return (lower <= current && current <= upper);
   }
 
   /**

src/browser/renderer/dom/DomRenderer.ts

@@ -38,6 +38,7 @@ export class DomRenderer extends Disposable implements IRenderer {
   private _rowContainer: HTMLElement;
   private _rowElements: HTMLElement[] = [];
   private _selectionContainer: HTMLElement;
+  private _cellToRowElements: Uint16Array[] = [];
 
   public dimensions: IRenderDimensions;
 
@@ -359,7 +360,10 @@ export class DomRenderer extends Disposable implements IRenderer {
       const row = y + this._bufferService.buffer.ydisp;
       const lineData = this._bufferService.buffer.lines.get(row);
       const cursorStyle = this._optionsService.rawOptions.cursorStyle;
-      rowElement.replaceChildren(this._rowFactory.createRow(lineData!, row, row === cursorAbsoluteY, cursorStyle, cursorX, cursorBlink, this.dimensions.css.cell.width, this._bufferService.cols));
+      if (!this._cellToRowElements[y] || this._cellToRowElements[y].length !== this._bufferService.cols) {
+        this._cellToRowElements[y] = new Uint16Array(this._bufferService.cols);
+      }
+      rowElement.replaceChildren(this._rowFactory.createRow(lineData!, row, row === cursorAbsoluteY, cursorStyle, cursorX, cursorBlink, this.dimensions.css.cell.width, this._bufferService.cols, this._cellToRowElements[y]));
     }
   }
 
@@ -376,6 +380,13 @@ export class DomRenderer extends Disposable implements IRenderer {
   }
 
   private _setCellUnderline(x: number, x2: number, y: number, y2: number, cols: number, enabled: boolean): void {
+    x = this._cellToRowElements[y][x];
+    x2 = this._cellToRowElements[y2][x2];
+
+    if (x === -1 || x2 === -1) {
+      return;
+    }
+
     while (x !== x2 || y !== y2) {
       const row = this._rowElements[y];
       if (!row) {