fix linkifier after shrinking

src/Buffer.test.ts

@@ -500,19 +500,18 @@ describe('Buffer', () => {
       assert(terminal.buffer.lines.get(bufferIndex[0]).get(bufferIndex[1])[CHAR_DATA_CHAR_INDEX], '😃');
     });
 
-    it('multiline fullwidth chars with offset 1 (currently tests for broken behavior)', () => {
+    it('multiline fullwidth chars with offset 1', () => {
       const input = 'a１２３４５６７８９０１２３４５６７８９０';
       // the 'a' at the beginning moves all fullwidth chars one to the right
       // now the end of the line contains a dangling empty cell since
       // the next fullwidth char has to wrap early
       // the dangling last cell is wrongly added in the string
-      // --> fixable after resolving #1685
+      // --> fixable after resolving #1685, fixed in ....
       terminal.writeSync(input);
-      // TODO: reenable after fix
-      // const s = terminal.buffer.contents(true).toArray()[0];
-      // assert.equal(input, s);
+      const s = terminal.buffer.iterator(true).next().content;
+      assert.equal(input, s);
       for (let i = 10; i < input.length; ++i) {
-        const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i + 1); // TODO: remove +1 after fix
+        const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i);
         const j = (i - 0) << 1;
         assert.deepEqual([(j / terminal.cols) | 0, j % terminal.cols], bufferIndex);
       }

src/Buffer.ts

@@ -234,7 +234,6 @@ export class Buffer implements IBuffer {
    * The method operates on the CharData string length, there are no
    * additional content or boundary checks. Therefore the string and the buffer
    * should not be altered in between.
-   * TODO: respect trim flag after fixing #1685
    * @param lineIndex line index the string was retrieved from
    * @param stringIndex index within the string
    * @param startCol column offset the string was retrieved from
@@ -483,7 +482,6 @@ export class BufferStringIterator implements IBufferStringIterator {
     range.last = Math.min(range.last, this._buffer.lines.length);
     let result = '';
     for (let i = range.first; i <= range.last; ++i) {
-      // TODO: always apply trimRight after fixing #1685
       result += this._buffer.translateBufferLineToString(i, (this._trimRight) ? i === range.last : false);
     }
     this._current = range.last + 1;

src/InputHandler.ts

@@ -390,6 +390,14 @@ export class InputHandler extends Disposable implements IInputHandler {
         // autowrap - DECAWM
         // automatically wraps to the beginning of the next line
         if (wraparoundMode) {
+          // nullify all cells behind
+          // we iterate here to line.length which can be bigger
+          // after shrinking than cols to get correct offset handling
+          // in Buffer.stringIndexToBufferIndex, also partially fixes
+          // trimming in Buffer.translateBufferLineToString
+          for (let i = buffer.x; i < bufferRow.length; ++i) {
+            bufferRow.set(buffer.x++, [curAttr, '', 0, undefined]);
+          }
           buffer.x = 0;
           buffer.y++;
           if (buffer.y > buffer.scrollBottom) {

src/Linkifier.test.ts

@@ -5,11 +5,10 @@
 
 import { assert } from 'chai';
 import { IMouseZoneManager, IMouseZone } from './ui/Types';
-import { ILinkMatcher, ITerminal, IBufferLine } from './Types';
+import { ILinkMatcher, ITerminal } from './Types';
 import { Linkifier } from './Linkifier';
-import { MockBuffer, MockTerminal, TestTerminal } from './utils/TestUtils.test';
-import { CircularList } from './common/CircularList';
-import { BufferLine } from './BufferLine';
+import { TestTerminal } from './utils/TestUtils.test';
+import { getStringCellWidth } from './CharWidth';
 
 class TestLinkifier extends Linkifier {
   constructor(terminal: ITerminal) {
@@ -35,63 +34,29 @@ class TestMouseZoneManager implements IMouseZoneManager {
 }
 
 describe('Linkifier', () => {
-  let terminal: ITerminal;
+  let terminal: TestTerminal;
   let linkifier: TestLinkifier;
   let mouseZoneManager: TestMouseZoneManager;
 
   beforeEach(() => {
-    terminal = new MockTerminal();
-    terminal.cols = 100;
-    terminal.rows = 10;
-    terminal.buffer = new MockBuffer();
-    (<MockBuffer>terminal.buffer).setLines(new CircularList<IBufferLine>(20));
-    terminal.buffer.ydisp = 0;
+    terminal = new TestTerminal({cols: 100, rows: 10});
     linkifier = new TestLinkifier(terminal);
     mouseZoneManager = new TestMouseZoneManager();
+    linkifier.attachToDom(mouseZoneManager);
   });
 
-  function stringToRow(text: string): IBufferLine {
-    const result = new BufferLine(text.length);
-    for (let i = 0; i < text.length; i++) {
-      result.set(i, [0, text.charAt(i), 1, text.charCodeAt(i)]);
-    }
-    return result;
-  }
-
-  function addRow(text: string): void {
-    terminal.buffer.lines.push(stringToRow(text));
-  }
-
-  function assertLinkifiesRow(rowText: string, linkMatcherRegex: RegExp, links: {x: number, length: number}[], done: MochaDone): void {
-    addRow(rowText);
+  function assertLinkifiesInTerminalRow(rowText: string, linkMatcherRegex: RegExp, links: {x: number, length: number}[], done: MochaDone): void {
+    terminal.writeSync(rowText);
     linkifier.registerLinkMatcher(linkMatcherRegex, () => {});
-    terminal.rows = terminal.buffer.lines.length - 1;
     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.x + 1);
         assert.equal(mouseZoneManager.zones[i].x2, l.x + l.length + 1);
-        assert.equal(mouseZoneManager.zones[i].y1, terminal.buffer.lines.length);
-        assert.equal(mouseZoneManager.zones[i].y2, terminal.buffer.lines.length);
-      });
-      done();
-    }, 0);
-  }
-
-  function assertLinkifiesMultiLineLink(rowText: string, linkMatcherRegex: RegExp, links: {x1: number, y1: number, x2: number, y2: number}[], done: MochaDone): void {
-    addRow(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);
+        assert.equal(mouseZoneManager.zones[i].y1, 1);
+        assert.equal(mouseZoneManager.zones[i].y2, 1);
       });
       done();
     }, 0);
@@ -114,55 +79,76 @@ describe('Linkifier', () => {
 
     describe('link matcher', () => {
       it('should match a single link', done => {
-        assertLinkifiesRow('foo', /foo/, [{x: 0, length: 3}], done);
+        assertLinkifiesInTerminalRow('foo', /foo/, [{x: 0, length: 3}], done);
       });
       it('should match a single link at the start of a text node', done => {
-        assertLinkifiesRow('foo bar', /foo/, [{x: 0, length: 3}], done);
+        assertLinkifiesInTerminalRow('foo bar', /foo/, [{x: 0, length: 3}], done);
       });
       it('should match a single link in the middle of a text node', done => {
-        assertLinkifiesRow('foo bar baz', /bar/, [{x: 4, length: 3}], done);
+        assertLinkifiesInTerminalRow('foo bar baz', /bar/, [{x: 4, length: 3}], done);
       });
       it('should match a single link at the end of a text node', done => {
-        assertLinkifiesRow('foo bar', /bar/, [{x: 4, length: 3}], done);
+        assertLinkifiesInTerminalRow('foo bar', /bar/, [{x: 4, length: 3}], done);
       });
       it('should match a link after a link at the start of a text node', done => {
-        assertLinkifiesRow('foo bar', /foo|bar/, [{x: 0, length: 3}, {x: 4, length: 3}], done);
+        assertLinkifiesInTerminalRow('foo bar', /foo|bar/, [{x: 0, length: 3}, {x: 4, length: 3}], done);
       });
       it('should match a link after a link in the middle of a text node', done => {
-        assertLinkifiesRow('foo bar baz', /bar|baz/, [{x: 4, length: 3}, {x: 8, length: 3}], done);
+        assertLinkifiesInTerminalRow('foo bar baz', /bar|baz/, [{x: 4, length: 3}, {x: 8, length: 3}], done);
       });
       it('should match a link immediately after a link at the end of a text node', done => {
-        assertLinkifiesRow('foo barbaz', /bar|baz/, [{x: 4, length: 3}, {x: 7, length: 3}], done);
+        assertLinkifiesInTerminalRow('foo barbaz', /bar|baz/, [{x: 4, length: 3}, {x: 7, length: 3}], done);
       });
       it('should not duplicate text after a unicode character (wrapped in a span)', done => {
         // This is a regression test for an issue that came about when using
         // an oh-my-zsh theme that added the large blue diamond unicode
         // character (U+1F537) which caused the path to be duplicated. See #642.
-        assertLinkifiesRow('echo \'🔷foo\'', /foo/, [{x: 8, length: 3}], done);
+        const charWidth = getStringCellWidth('🔷'); // FIXME: make unicode version dependent
+        assertLinkifiesInTerminalRow('echo \'🔷foo\'', /foo/, [{x: 6 + charWidth, length: 3}], done);
       });
       describe('multi-line links', () => {
+        let terminal: TestTerminal;
+        beforeEach(() => {
+          terminal = new TestTerminal({cols: 4, rows: 10});
+          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.writeSync(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('should match links that start on line 1/2 of a wrapped line and end on the last character of line 1/2', done => {
-          terminal.cols = 4;
-          assertLinkifiesMultiLineLink('12345', /1234/, [{x1: 0, x2: 4, y1: 0, y2: 0}], done);
+          assertLinkifiesInTerminal('12345', /1234/, [{x1: 0, x2: 4, y1: 0, y2: 0}], done);
         });
         it('should match links that start on line 1/2 of a wrapped line and wrap to line 2/2', done => {
-          terminal.cols = 4;
-          assertLinkifiesMultiLineLink('12345', /12345/, [{x1: 0, x2: 1, y1: 0, y2: 1}], done);
+          assertLinkifiesInTerminal('12345', /12345/, [{x1: 0, x2: 1, y1: 0, y2: 1}], done);
         });
         it('should match links that start and end on line 2/2 of a wrapped line', done => {
-          terminal.cols = 4;
-          assertLinkifiesMultiLineLink('12345678', /5678/, [{x1: 0, x2: 4, y1: 1, y2: 1}], done);
+          assertLinkifiesInTerminal('12345678', /5678/, [{x1: 0, x2: 4, y1: 1, y2: 1}], done);
         });
         it('should match links that start on line 2/3 of a wrapped line and wrap to line 3/3', done => {
-          terminal.cols = 4;
-          assertLinkifiesMultiLineLink('123456789', /56789/, [{x1: 0, x2: 1, y1: 1, y2: 2}], done);
+          assertLinkifiesInTerminal('123456789', /56789/, [{x1: 0, x2: 1, y1: 1, y2: 2}], done);
         });
       });
     });
 
     describe('validationCallback', () => {
       it('should enable link if true', done => {
-        addRow('test');
+        terminal.writeSync('test');
         linkifier.registerLinkMatcher(/test/, () => done(), {
           validationCallback: (url, cb) => {
             assert.equal(mouseZoneManager.zones.length, 0);
@@ -180,7 +166,7 @@ describe('Linkifier', () => {
       });
 
       it('should validate the uri, not the row', done => {
-        addRow('abc test abc');
+        terminal.writeSync('abc test abc');
         linkifier.registerLinkMatcher(/test/, () => done(), {
           validationCallback: (uri, cb) => {
             assert.equal(uri, 'test');
@@ -191,7 +177,7 @@ describe('Linkifier', () => {
       });
 
       it('should disable link if false', done => {
-        addRow('test');
+        terminal.writeSync('test');
         linkifier.registerLinkMatcher(/test/, () => assert.fail(), {
           validationCallback: (url, cb) => {
             assert.equal(mouseZoneManager.zones.length, 0);
@@ -205,7 +191,7 @@ describe('Linkifier', () => {
       });
 
       it('should trigger for multiple link matches on one row', done => {
-        addRow('test test');
+        terminal.writeSync('test test');
         let count = 0;
         linkifier.registerLinkMatcher(/test/, () => assert.fail(), {
           validationCallback: (url, cb) => {

src/Linkifier.ts

@@ -7,8 +7,7 @@ import { IMouseZoneManager } from './ui/Types';
 import { ILinkHoverEvent, ILinkMatcher, LinkMatcherHandler, LinkHoverEventTypes, ILinkMatcherOptions, ILinkifier, ITerminal, IBufferStringIteratorResult } from './Types';
 import { MouseZone } from './ui/MouseZoneManager';
 import { EventEmitter } from './common/EventEmitter';
-import { CHAR_DATA_ATTR_INDEX } from './Buffer';
-import { getStringCellWidth } from './CharWidth';
+import { CHAR_DATA_ATTR_INDEX, CHAR_DATA_WIDTH_INDEX } from './Buffer';
 
 /**
  * The Linkifier applies links to rows shortly after they have been refreshed.
@@ -222,6 +221,30 @@ export class Linkifier extends EventEmitter implements ILinkifier {
 
       // get the buffer index as [absolute row, col] for the match
       const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex);
+      // calculate buffer index of uri end
+      // we cannot directly use uri.length here since stringIndexToBufferIndex would
+      // skip empty cells and stop at the next cell with real content
+      // instead we fetch the index of the last char in uri and advance to the next cell
+      const endIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex + uri.length - 1);
+
+      // adjust start index to visible line length
+      if (bufferIndex[1] >= this._terminal.cols) {
+        bufferIndex[0]++;
+        bufferIndex[1] = 0;
+      }
+      // advance endIndex to next cell:
+      //    add actual length of the last char to x_offset
+      //    wrap to next buffer line if we overflow
+      endIndex[1] += this._terminal.buffer.lines.get(endIndex[0]).get(endIndex[1])[CHAR_DATA_WIDTH_INDEX];
+      if (endIndex[1] >= this._terminal.buffer.lines.get(endIndex[0]).length) {
+        endIndex[0]++;
+        endIndex[1] = 0;
+      }
+      // adjust end index to visible line length
+      if (endIndex[1] >= this._terminal.cols) {
+        endIndex[1] = this._terminal.cols - 1;
+      }
+      const visibleLength = (endIndex[0] - bufferIndex[0]) * this._terminal.cols - bufferIndex[1] + endIndex[1];
 
       const line = this._terminal.buffer.lines.get(bufferIndex[0]);
       const char = line.get(bufferIndex[1]);
@@ -238,11 +261,11 @@ export class Linkifier extends EventEmitter implements ILinkifier {
             return;
           }
           if (isValid) {
-            this._addLink(bufferIndex[1], bufferIndex[0] - this._terminal.buffer.ydisp, uri, matcher, fg);
+            this._addLink(bufferIndex[1], bufferIndex[0] - this._terminal.buffer.ydisp, uri, visibleLength, matcher, fg);
           }
         });
       } else {
-        this._addLink(bufferIndex[1], bufferIndex[0] - this._terminal.buffer.ydisp, uri, matcher, fg);
+        this._addLink(bufferIndex[1], bufferIndex[0] - this._terminal.buffer.ydisp, uri, visibleLength, matcher, fg);
       }
     }
   }
@@ -255,8 +278,8 @@ export class Linkifier extends EventEmitter implements ILinkifier {
    * @param matcher The link matcher for the link.
    * @param fg The link color for hover event.
    */
-  private _addLink(x: number, y: number, uri: string, matcher: ILinkMatcher, fg: number): void {
-    const width = getStringCellWidth(uri);
+  private _addLink(x: number, y: number, uri: string, length: number, matcher: ILinkMatcher, fg: number): void {
+    const width = length;
     const x1 = x % this._terminal.cols;
     const y1 = y + Math.floor(x / this._terminal.cols);
     let x2 = (x1 + width) % this._terminal.cols;

src/Terminal.test.ts

@@ -928,9 +928,9 @@ describe('term.js addons', () => {
         }
       }
       let tchar = term.buffer.lines.get(0).get(term.cols - 1);
-      expect(tchar[CHAR_DATA_CHAR_INDEX]).eql(' ');
-      expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(1);
-      expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1);
+      expect(tchar[CHAR_DATA_CHAR_INDEX]).eql('');
+      expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(0);
+      expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(0);
       tchar = term.buffer.lines.get(1).get(0);
       expect(tchar[CHAR_DATA_CHAR_INDEX]).eql('￥');
       expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(1);
@@ -953,9 +953,9 @@ describe('term.js addons', () => {
         }
       }
       let tchar = term.buffer.lines.get(0).get(term.cols - 1);
-      expect(tchar[CHAR_DATA_CHAR_INDEX]).eql(' ');
-      expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(1);
-      expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1);
+      expect(tchar[CHAR_DATA_CHAR_INDEX]).eql('');
+      expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(0);
+      expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(0);
       tchar = term.buffer.lines.get(1).get(0);
       expect(tchar[CHAR_DATA_CHAR_INDEX]).eql('￥\u0301');
       expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(2);
@@ -998,9 +998,9 @@ describe('term.js addons', () => {
         }
       }
       let tchar = term.buffer.lines.get(0).get(term.cols - 1);
-      expect(tchar[CHAR_DATA_CHAR_INDEX]).eql(' ');
-      expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(1);
-      expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1);
+      expect(tchar[CHAR_DATA_CHAR_INDEX]).eql('');
+      expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(0);
+      expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(0);
       tchar = term.buffer.lines.get(1).get(0);
       expect(tchar[CHAR_DATA_CHAR_INDEX]).eql('\ud843\ude6d\u0301');
       expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(3);