fix multiple row linkifier attempts; tests for unicode before a match

src/Linkifier.test.ts

@@ -10,6 +10,7 @@ import { Linkifier } from './Linkifier';
 import { MockBuffer, MockTerminal } from './utils/TestUtils.test';
 import { CircularList } from './common/CircularList';
 import { BufferLine } from './BufferLine';
+import { Terminal } from './Terminal';
 
 class TestLinkifier extends Linkifier {
   constructor(terminal: ITerminal) {
@@ -238,4 +239,67 @@ describe('Linkifier', () => {
       });
     });
   });
+  describe('unicode handling', function(): void {
+    // other than the tests above unicode testing needs the full terminal instance
+    // to get the special handling of fullwidth, surrogate and combining chars in the input handler
+    beforeEach(function(): void {
+      terminal = new Terminal({cols: 10, rows: 5});
+      const oldWrite: any = terminal.write.bind(terminal);
+      terminal.write = (s: string): void => {
+        oldWrite(s);
+        (terminal as any)._innerWrite();
+      };
+      linkifier = new TestLinkifier(terminal);
+      mouseZoneManager = new TestMouseZoneManager();
+      linkifier.attachToDom(mouseZoneManager);
+    });
+
+    function assertLinkifiesInTerminal(rowText: string, linkMatcherRegex: RegExp, links: {x1: number, y1: number, x2: number, y2: number}[], done: MochaDone): void {
+      terminal.write(rowText);
+      linkifier.registerLinkMatcher(linkMatcherRegex, () => {});
+      linkifier.linkifyRows();
+      // Allow linkify to happen
+      setTimeout(() => {
+        assert.equal(mouseZoneManager.zones.length, links.length);
+        links.forEach((l, i) => {
+          assert.equal(mouseZoneManager.zones[i].x1, l.x1 + 1);
+          assert.equal(mouseZoneManager.zones[i].x2, l.x2 + 1);
+          assert.equal(mouseZoneManager.zones[i].y1, l.y1 + 1);
+          assert.equal(mouseZoneManager.zones[i].y2, l.y2 + 1);
+        });
+        done();
+      }, 0);
+    }
+
+    it('combining before - match within one line', function(done: () => void): void {
+      assertLinkifiesInTerminal('e\u0301e\u0301e\u0301 foo', /foo/, [{x1: 4, x2: 7, y1: 0, y2: 0}], done);
+    });
+    it('combining before - match over two lines', function(done: () => void): void {
+      assertLinkifiesInTerminal('e\u0301e\u0301e\u0301     foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done);
+    });
+    it('surrogate before - match within one line', function(done: () => void): void {
+      assertLinkifiesInTerminal('𝄞𝄞𝄞 foo', /foo/, [{x1: 4, x2: 7, y1: 0, y2: 0}], done);
+    });
+    it('surrogate before - match over two lines', function(done: () => void): void {
+      assertLinkifiesInTerminal('𝄞𝄞𝄞     foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done);
+    });
+    it('combining surrogate before - match within one line', function(done: () => void): void {
+      assertLinkifiesInTerminal('𓂀\u0301𓂀\u0301𓂀\u0301 foo', /foo/, [{x1: 4, x2: 7, y1: 0, y2: 0}], done);
+    });
+    it('combining surrogate before - match over two lines', function(done: () => void): void {
+      assertLinkifiesInTerminal('𓂀\u0301𓂀\u0301𓂀\u0301     foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done);
+    });
+    it('fullwidth before - match within one line', function(done: () => void): void {
+      assertLinkifiesInTerminal('１２ foo', /foo/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done);
+    });
+    it('fullwidth before - match over two lines', function(done: () => void): void {
+      assertLinkifiesInTerminal('１２    foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done);
+    });
+    it('combining fullwidth before - match within one line', function(done: () => void): void {
+      assertLinkifiesInTerminal('￥\u0301￥\u0301 foo', /foo/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done);
+    });
+    it('combining fullwidth before - match over two lines', function(done: () => void): void {
+      assertLinkifiesInTerminal('￥\u0301￥\u0301    foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done);
+    });
+  });
 });

src/Linkifier.ts

@@ -80,9 +80,22 @@ export class Linkifier extends EventEmitter implements ILinkifier {
    */
   private _linkifyRows(): void {
     this._rowsTimeoutId = null;
-    for (let i = this._rowsToLinkify.start; i <= this._rowsToLinkify.end; i++) {
-      this._linkifyRow(i);
+
+    const absoluteRowIndexStart = this._terminal.buffer.ydisp + this._rowsToLinkify.start;
+    if (absoluteRowIndexStart >= this._terminal.buffer.lines.length) {
+      return;
     }
+
+    // iterate over the range of unwrapped content strings within start..end
+    // _doLinkifyRow gets full unwrapped lines with the start row as buffer offset for every matcher
+    const linesIterator = this._terminal.buffer.contents(false, absoluteRowIndexStart, this._terminal.buffer.ydisp + this._rowsToLinkify.end + 1);
+    while (linesIterator.hasNext()) {
+      for (let i = 0; i < this._linkMatchers.length; i++) {
+        const lineData: any = linesIterator.next(true);
+        this._doLinkifyRow(lineData[0].first, lineData[1], this._linkMatchers[i]);
+      }
+    }
+
     this._rowsToLinkify.start = null;
     this._rowsToLinkify.end = null;
   }
@@ -153,25 +166,6 @@ export class Linkifier extends EventEmitter implements ILinkifier {
     return false;
   }
 
-  /**
-   * Linkifies a row.
-   * @param rowIndex The index of the row to linkify.
-   */
-  private _linkifyRow(rowIndex: number): void {
-    // Ensure the row exists
-    const absoluteRowIndex = this._terminal.buffer.ydisp + rowIndex;
-    if (absoluteRowIndex >= this._terminal.buffer.lines.length) {
-      return;
-    }
-    const unwrappedLinesIterator = this._terminal.buffer.contents(false, absoluteRowIndex, absoluteRowIndex + 1);
-    while (unwrappedLinesIterator.hasNext()) {
-      for (let i = 0; i < this._linkMatchers.length; i++) {
-        const lineData: any = unwrappedLinesIterator.next(true);
-        this._doLinkifyRow(lineData[0].first, lineData[1], this._linkMatchers[i]);
-      }
-    }
-  }
-
   /**
    * Linkifies a row given a specific handler.
    * @param rowIndex The row index to linkify (absolute index).
@@ -179,13 +173,21 @@ export class Linkifier extends EventEmitter implements ILinkifier {
    * @param matcher The link matcher for this line.
    */
   private _doLinkifyRow(rowIndex: number, text: string, matcher: ILinkMatcher): void {
+    // clone regex do a global search on text
     const rex = new RegExp(matcher.regex.source, matcher.regex.flags + 'g');
     let match;
     let stringIndex = -1;
     while ((match = rex.exec(text)) !== null) {
       const uri = match[typeof matcher.matchIndex !== 'number' ? 0 : matcher.matchIndex];
+
+      // due to complex regexes we cannot use match.index directly
+      // instead we search the position of the match group in text again TODO: Can this be avoided?
+      // also correct regex and string search offsets for the next loop run
       stringIndex = text.indexOf(uri, stringIndex + 1);
       rex.lastIndex = stringIndex + uri.length;
+
+      // get the buffer index as [absolute row, col] for the match
+      // load the attrs at that pos and underline
       const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex);
       const line = this._terminal.buffer.lines.get(bufferIndex[0]);
       const char = line.get(bufferIndex[1]);