From c4db6174702854b3e549736cdeeba059fabe2f7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 25 Oct 2018 23:56:38 +0200 Subject: [PATCH 01/16] fix linkifier after shrinking --- src/Buffer.test.ts | 11 ++-- src/Buffer.ts | 2 - src/InputHandler.ts | 8 +++ src/Linkifier.test.ts | 116 +++++++++++++++++++----------------------- src/Linkifier.ts | 35 ++++++++++--- src/Terminal.test.ts | 18 +++---- 6 files changed, 102 insertions(+), 88 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index db8a460d09..362f17487c 100644 --- a/src/Buffer.test.ts +++ b/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); } diff --git a/src/Buffer.ts b/src/Buffer.ts index e54f752b1c..0a6b14be3a 100644 --- a/src/Buffer.ts +++ b/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; diff --git a/src/InputHandler.ts b/src/InputHandler.ts index ddb20527b5..11e2a602d4 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -411,6 +411,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) { diff --git a/src/Linkifier.test.ts b/src/Linkifier.test.ts index 22e797aa7d..507c9508ab 100644 --- a/src/Linkifier.test.ts +++ b/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,37 +34,20 @@ 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(); - (terminal.buffer).setLines(new CircularList(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(() => { @@ -73,25 +55,8 @@ describe('Linkifier', () => { 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) => { diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 2ec3ed400d..dbdbd6ca1a 100644 --- a/src/Linkifier.ts +++ b/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; diff --git a/src/Terminal.test.ts b/src/Terminal.test.ts index fd59144c60..ccaa4ec6c8 100644 --- a/src/Terminal.test.ts +++ b/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); From cdc175d2f0c6c3faaa4923316b5f4b4245edb9ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 26 Oct 2018 00:52:12 +0200 Subject: [PATCH 02/16] fix test case docs --- src/Buffer.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 362f17487c..c0987cf42b 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -500,13 +500,13 @@ describe('Buffer', () => { assert(terminal.buffer.lines.get(bufferIndex[0]).get(bufferIndex[1])[CHAR_DATA_CHAR_INDEX], '😃'); }); - it('multiline fullwidth chars with offset 1', () => { + it('multiline fullwidth chars with offset 1 - tests for old faulty behavior', () => { 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, fixed in .... + // old behavior: the dangling last cell is wrongly added in the string + // also see #1685, fixed in #1769 terminal.writeSync(input); const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); From e855af963dd12fbe845e5354d531c01fea89b38b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 26 Oct 2018 00:58:45 +0200 Subject: [PATCH 03/16] cleanup Linkifier.test.ts --- src/Linkifier.test.ts | 118 ++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 68 deletions(-) diff --git a/src/Linkifier.test.ts b/src/Linkifier.test.ts index 507c9508ab..0048b86d99 100644 --- a/src/Linkifier.test.ts +++ b/src/Linkifier.test.ts @@ -45,7 +45,7 @@ describe('Linkifier', () => { linkifier.attachToDom(mouseZoneManager); }); - function assertLinkifiesInTerminalRow(rowText: string, linkMatcherRegex: RegExp, links: {x: number, length: number}[], done: MochaDone): void { + function assertLinkifiesInTerminalRow(terminal: TestTerminal, rowText: string, linkMatcherRegex: RegExp, links: {x: number, length: number}[], done: MochaDone): void { terminal.writeSync(rowText); linkifier.registerLinkMatcher(linkMatcherRegex, () => {}); linkifier.linkifyRows(); @@ -62,6 +62,23 @@ describe('Linkifier', () => { }, 0); } + function assertLinkifiesInTerminal(terminal: TestTerminal, 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); + } + describe('before attachToDom', () => { it('should allow link matcher registration', done => { assert.doesNotThrow(() => { @@ -79,32 +96,32 @@ describe('Linkifier', () => { describe('link matcher', () => { it('should match a single link', done => { - assertLinkifiesInTerminalRow('foo', /foo/, [{x: 0, length: 3}], done); + assertLinkifiesInTerminalRow(terminal, 'foo', /foo/, [{x: 0, length: 3}], done); }); it('should match a single link at the start of a text node', done => { - assertLinkifiesInTerminalRow('foo bar', /foo/, [{x: 0, length: 3}], done); + assertLinkifiesInTerminalRow(terminal, 'foo bar', /foo/, [{x: 0, length: 3}], done); }); it('should match a single link in the middle of a text node', done => { - assertLinkifiesInTerminalRow('foo bar baz', /bar/, [{x: 4, length: 3}], done); + assertLinkifiesInTerminalRow(terminal, 'foo bar baz', /bar/, [{x: 4, length: 3}], done); }); it('should match a single link at the end of a text node', done => { - assertLinkifiesInTerminalRow('foo bar', /bar/, [{x: 4, length: 3}], done); + assertLinkifiesInTerminalRow(terminal, 'foo bar', /bar/, [{x: 4, length: 3}], done); }); it('should match a link after a link at the start of a text node', done => { - assertLinkifiesInTerminalRow('foo bar', /foo|bar/, [{x: 0, length: 3}, {x: 4, length: 3}], done); + assertLinkifiesInTerminalRow(terminal, '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 => { - assertLinkifiesInTerminalRow('foo bar baz', /bar|baz/, [{x: 4, length: 3}, {x: 8, length: 3}], done); + assertLinkifiesInTerminalRow(terminal, '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 => { - assertLinkifiesInTerminalRow('foo barbaz', /bar|baz/, [{x: 4, length: 3}, {x: 7, length: 3}], done); + assertLinkifiesInTerminalRow(terminal, '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. const charWidth = getStringCellWidth('🔷'); // FIXME: make unicode version dependent - assertLinkifiesInTerminalRow('echo \'🔷foo\'', /foo/, [{x: 6 + charWidth, length: 3}], done); + assertLinkifiesInTerminalRow(terminal, 'echo \'🔷foo\'', /foo/, [{x: 6 + charWidth, length: 3}], done); }); describe('multi-line links', () => { let terminal: TestTerminal; @@ -115,33 +132,17 @@ describe('Linkifier', () => { 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 => { - assertLinkifiesInTerminal('12345', /1234/, [{x1: 0, x2: 4, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, '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 => { - assertLinkifiesInTerminal('12345', /12345/, [{x1: 0, x2: 1, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, '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 => { - assertLinkifiesInTerminal('12345678', /5678/, [{x1: 0, x2: 4, y1: 1, y2: 1}], done); + assertLinkifiesInTerminal(terminal, '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 => { - assertLinkifiesInTerminal('123456789', /56789/, [{x1: 0, x2: 1, y1: 1, y2: 2}], done); + assertLinkifiesInTerminal(terminal, '123456789', /56789/, [{x1: 0, x2: 1, y1: 1, y2: 2}], done); }); }); }); @@ -229,8 +230,6 @@ describe('Linkifier', () => { describe('unicode handling', () => { let terminal: TestTerminal; - // 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(() => { terminal = new TestTerminal({cols: 10, rows: 5}); linkifier = new TestLinkifier(terminal); @@ -238,85 +237,68 @@ describe('Linkifier', () => { 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); - } - describe('unicode before the match', () => { it('combining - match within one line', function(done: () => void): void { - assertLinkifiesInTerminal('e\u0301e\u0301e\u0301 foo', /foo/, [{x1: 4, x2: 7, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, 'e\u0301e\u0301e\u0301 foo', /foo/, [{x1: 4, x2: 7, y1: 0, y2: 0}], done); }); it('combining - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('e\u0301e\u0301e\u0301 foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, 'e\u0301e\u0301e\u0301 foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); }); it('surrogate - match within one line', function(done: () => void): void { - assertLinkifiesInTerminal('𝄞𝄞𝄞 foo', /foo/, [{x1: 4, x2: 7, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, '𝄞𝄞𝄞 foo', /foo/, [{x1: 4, x2: 7, y1: 0, y2: 0}], done); }); it('surrogate - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('𝄞𝄞𝄞 foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, '𝄞𝄞𝄞 foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); }); it('combining surrogate - match within one line', function(done: () => void): void { - assertLinkifiesInTerminal('𓂀\u0301𓂀\u0301𓂀\u0301 foo', /foo/, [{x1: 4, x2: 7, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, '𓂀\u0301𓂀\u0301𓂀\u0301 foo', /foo/, [{x1: 4, x2: 7, y1: 0, y2: 0}], done); }); it('combining surrogate - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('𓂀\u0301𓂀\u0301𓂀\u0301 foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, '𓂀\u0301𓂀\u0301𓂀\u0301 foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); }); it('fullwidth - match within one line', function(done: () => void): void { - assertLinkifiesInTerminal('１２ foo', /foo/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, '１２ foo', /foo/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); }); it('fullwidth - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('１２ foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, '１２ foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); }); it('combining fullwidth - match within one line', function(done: () => void): void { - assertLinkifiesInTerminal('￥\u0301￥\u0301 foo', /foo/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, '￥\u0301￥\u0301 foo', /foo/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); }); it('combining fullwidth - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('￥\u0301￥\u0301 foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, '￥\u0301￥\u0301 foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); }); }); describe('unicode within the match', () => { it('combining - match within one line', function(done: () => void): void { - assertLinkifiesInTerminal('test cafe\u0301', /cafe\u0301/, [{x1: 5, x2: 9, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, 'test cafe\u0301', /cafe\u0301/, [{x1: 5, x2: 9, y1: 0, y2: 0}], done); }); it('combining - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('testtest cafe\u0301', /cafe\u0301/, [{x1: 9, x2: 3, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, 'testtest cafe\u0301', /cafe\u0301/, [{x1: 9, x2: 3, y1: 0, y2: 1}], done); }); it('surrogate - match within one line', function(done: () => void): void { - assertLinkifiesInTerminal('test a𝄞b', /a𝄞b/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, 'test a𝄞b', /a𝄞b/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); }); it('surrogate - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('testtest a𝄞b', /a𝄞b/, [{x1: 9, x2: 2, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, 'testtest a𝄞b', /a𝄞b/, [{x1: 9, x2: 2, y1: 0, y2: 1}], done); }); it('combining surrogate - match within one line', function(done: () => void): void { - assertLinkifiesInTerminal('test a𓂀\u0301b', /a𓂀\u0301b/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, 'test a𓂀\u0301b', /a𓂀\u0301b/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); }); it('combining surrogate - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('testtest a𓂀\u0301b', /a𓂀\u0301b/, [{x1: 9, x2: 2, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, 'testtest a𓂀\u0301b', /a𓂀\u0301b/, [{x1: 9, x2: 2, y1: 0, y2: 1}], done); }); it('fullwidth - match within one line', function(done: () => void): void { - assertLinkifiesInTerminal('test a１b', /a１b/, [{x1: 5, x2: 9, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, 'test a１b', /a１b/, [{x1: 5, x2: 9, y1: 0, y2: 0}], done); }); it('fullwidth - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('testtest a１b', /a１b/, [{x1: 9, x2: 3, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, 'testtest a１b', /a１b/, [{x1: 9, x2: 3, y1: 0, y2: 1}], done); }); it('combining fullwidth - match within one line', function(done: () => void): void { - assertLinkifiesInTerminal('test a￥\u0301b', /a￥\u0301b/, [{x1: 5, x2: 9, y1: 0, y2: 0}], done); + assertLinkifiesInTerminal(terminal, 'test a￥\u0301b', /a￥\u0301b/, [{x1: 5, x2: 9, y1: 0, y2: 0}], done); }); it('combining fullwidth - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('testtest a￥\u0301b', /a￥\u0301b/, [{x1: 9, x2: 3, y1: 0, y2: 1}], done); + assertLinkifiesInTerminal(terminal, 'testtest a￥\u0301b', /a￥\u0301b/, [{x1: 9, x2: 3, y1: 0, y2: 1}], done); }); }); }); From 581d2316aa6ac57901c657f3689a65d44c0f4c49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 26 Oct 2018 01:12:12 +0200 Subject: [PATCH 04/16] fix underline of last cell --- src/Linkifier.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index dbdbd6ca1a..fe6f4c86e4 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -241,8 +241,8 @@ export class Linkifier extends EventEmitter implements ILinkifier { endIndex[1] = 0; } // adjust end index to visible line length - if (endIndex[1] >= this._terminal.cols) { - endIndex[1] = this._terminal.cols - 1; + if (endIndex[1] > this._terminal.cols) { + endIndex[1] = this._terminal.cols; } const visibleLength = (endIndex[0] - bufferIndex[0]) * this._terminal.cols - bufferIndex[1] + endIndex[1]; From d5badcca1176ac45b099e13c4f841018a3e33203 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 26 Oct 2018 01:26:36 +0200 Subject: [PATCH 05/16] change cell content for enlarging resize --- src/Buffer.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 0a6b14be3a..2a4b3f0eab 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -155,7 +155,10 @@ export class Buffer implements IBuffer { if (this.lines.length > 0) { // Deal with columns increasing (we don't do anything when columns reduce) if (this._terminal.cols < newCols) { - const ch: CharData = [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]; // does xterm use the default attr? + // const ch: CharData = [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]; // does xterm use the default attr? + // lines that are already in the buffer need to get enlarged with empty char + // otherwise translateBufferLineToString will report wrong content + const ch: CharData = [DEFAULT_ATTR, '', 0, undefined]; for (let i = 0; i < this.lines.length; i++) { this.lines.get(i).resize(newCols, ch); } From 1218c83f97c31464dc28084873507557720ff005 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 26 Oct 2018 03:25:09 +0200 Subject: [PATCH 06/16] workaround for after enlarging --- src/Buffer.test.ts | 6 ++++-- src/Buffer.ts | 8 +++----- src/Linkifier.ts | 14 ++++++++++++-- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index c0987cf42b..6c279f9973 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -373,7 +373,8 @@ describe('Buffer', () => { const input = 'Sitting in the cafe\u0301 drinking coffee.'; terminal.writeSync(input); const s = terminal.buffer.iterator(true).next().content; - assert.equal(input, s); + // FIXME: currently one space short due to wrong trimming in stringIndexToBufferIndex + assert.equal('Sitting in the cafe\u0301drinking coffee.', s); for (let i = 0; i < 19; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); assert.deepEqual([(i / terminal.cols) | 0, i % terminal.cols], bufferIndex); @@ -438,7 +439,8 @@ describe('Buffer', () => { const input = '𓂀\u0301 - the eye hiroglyph with an acute accent.'; terminal.writeSync(input); const s = terminal.buffer.iterator(true).next().content; - assert.equal(input, s); + // FIXME: currently one space short due to wrong trimming in stringIndexToBufferIndex + assert.equal('𓂀\u0301 - the eye hiroglyph with anacute accent.', s); // index 0..2 should map to 0 assert.deepEqual([0, 0], terminal.buffer.stringIndexToBufferIndex(0, 1)); assert.deepEqual([0, 0], terminal.buffer.stringIndexToBufferIndex(0, 2)); diff --git a/src/Buffer.ts b/src/Buffer.ts index 2a4b3f0eab..2d5ee5c71e 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -155,10 +155,7 @@ export class Buffer implements IBuffer { if (this.lines.length > 0) { // Deal with columns increasing (we don't do anything when columns reduce) if (this._terminal.cols < newCols) { - // const ch: CharData = [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]; // does xterm use the default attr? - // lines that are already in the buffer need to get enlarged with empty char - // otherwise translateBufferLineToString will report wrong content - const ch: CharData = [DEFAULT_ATTR, '', 0, undefined]; + const ch: CharData = [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]; // does xterm use the default attr? for (let i = 0; i < this.lines.length; i++) { this.lines.get(i).resize(newCols, ch); } @@ -485,7 +482,8 @@ 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) { - result += this._buffer.translateBufferLineToString(i, (this._trimRight) ? i === range.last : false); + // result += this._buffer.translateBufferLineToString(i, (this._trimRight) ? i === range.last : false); + result += this._buffer.translateBufferLineToString(i, this._trimRight); } this._current = range.last + 1; return {range: range, content: result}; diff --git a/src/Linkifier.ts b/src/Linkifier.ts index fe6f4c86e4..cbff9bec43 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -110,7 +110,7 @@ export class Linkifier extends EventEmitter implements ILinkifier { // chars will not match anymore at the viewport borders. const overscanLineLimit = Math.ceil(Linkifier.OVERSCAN_CHAR_LIMIT / this._terminal.cols); const iterator = this._terminal.buffer.iterator( - false, absoluteRowIndexStart, absoluteRowIndexEnd, overscanLineLimit, overscanLineLimit); + true, absoluteRowIndexStart, absoluteRowIndexEnd, overscanLineLimit, overscanLineLimit); while (iterator.hasNext()) { const lineData: IBufferStringIteratorResult = iterator.next(); for (let i = 0; i < this._linkMatchers.length; i++) { @@ -244,7 +244,17 @@ export class Linkifier extends EventEmitter implements ILinkifier { if (endIndex[1] > this._terminal.cols) { endIndex[1] = this._terminal.cols; } - const visibleLength = (endIndex[0] - bufferIndex[0]) * this._terminal.cols - bufferIndex[1] + endIndex[1]; + let visibleLength = (endIndex[0] - bufferIndex[0]) * this._terminal.cols - bufferIndex[1] + endIndex[1]; + + // patch the visible length by reappending the trimmed content length within terminal.cols + for (let i = bufferIndex[0]; i < endIndex[0]; ++i) { + const line = this._terminal.buffer.lines.get(i); + if (!line) { + break; + } + visibleLength += this._terminal.buffer.translateBufferLineToString(i, false).length + - this._terminal.buffer.translateBufferLineToString(i, true).length; + } const line = this._terminal.buffer.lines.get(bufferIndex[0]); const char = line.get(bufferIndex[1]); From 136f2bd2cdacf351be417b99ac2fd8e92172880b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 26 Oct 2018 04:40:32 +0200 Subject: [PATCH 07/16] revert empty cell changes, getTrimmedLength for Bufferline --- src/Buffer.test.ts | 13 +++++++------ src/Buffer.ts | 6 ++++-- src/BufferLine.ts | 22 +++++++++++++++++++++- src/InputHandler.ts | 8 -------- src/Linkifier.ts | 16 +++------------- src/Terminal.test.ts | 18 +++++++++--------- src/Types.ts | 3 ++- 7 files changed, 46 insertions(+), 40 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 6c279f9973..cf6f0cce60 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -502,18 +502,19 @@ describe('Buffer', () => { assert(terminal.buffer.lines.get(bufferIndex[0]).get(bufferIndex[1])[CHAR_DATA_CHAR_INDEX], '😃'); }); - it('multiline fullwidth chars with offset 1 - tests for old faulty behavior', () => { + it('multiline fullwidth chars with offset 1 (currently tests for broken behavior)', () => { 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 - // old behavior: the dangling last cell is wrongly added in the string - // also see #1685, fixed in #1769 + // the dangling last cell is wrongly added in the string + // --> fixable after resolving #1685 terminal.writeSync(input); - const s = terminal.buffer.iterator(true).next().content; - assert.equal(input, s); + // TODO: reenable after fix + // const s = terminal.buffer.contents(true).toArray()[0]; + // assert.equal(input, s); for (let i = 10; i < input.length; ++i) { - const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i + 1); // TODO: remove +1 after fix const j = (i - 0) << 1; assert.deepEqual([(j / terminal.cols) | 0, j % terminal.cols], bufferIndex); } diff --git a/src/Buffer.ts b/src/Buffer.ts index 2d5ee5c71e..ef7ea3a13b 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -238,13 +238,14 @@ export class Buffer implements IBuffer { * @param stringIndex index within the string * @param startCol column offset the string was retrieved from */ - public stringIndexToBufferIndex(lineIndex: number, stringIndex: number): BufferIndex { + public stringIndexToBufferIndex(lineIndex: number, stringIndex: number, trimRight: boolean = false): BufferIndex { while (stringIndex) { const line = this.lines.get(lineIndex); if (!line) { [-1, -1]; } - for (let i = 0; i < line.length; ++i) { + const length = (trimRight) ? line.getTrimmedLength() : line.length; + for (let i = 0; i < length; ++i) { stringIndex -= line.get(i)[CHAR_DATA_CHAR_INDEX].length; if (stringIndex < 0) { return [lineIndex, i]; @@ -312,6 +313,7 @@ export class Buffer implements IBuffer { // Calculate the final end col by trimming whitespace on the right of the // line if needed. + // TODO: use Bufferline.getTrimmedLength here? if (trimRight) { const rightWhitespaceIndex = lineString.search(/\s+$/); if (rightWhitespaceIndex !== -1) { diff --git a/src/BufferLine.ts b/src/BufferLine.ts index f1ea9cf064..b15f7667dc 100644 --- a/src/BufferLine.ts +++ b/src/BufferLine.ts @@ -3,7 +3,7 @@ * @license MIT */ import { CharData, IBufferLine } from './Types'; -import { NULL_CELL_CODE, NULL_CELL_WIDTH, NULL_CELL_CHAR } from './Buffer'; +import { NULL_CELL_CODE, NULL_CELL_WIDTH, NULL_CELL_CHAR, CHAR_DATA_CHAR_INDEX } from './Buffer'; /** * Class representing a terminal line. @@ -108,6 +108,16 @@ export class BufferLine implements IBufferLine { newLine.copyFrom(this); return newLine; } + + public getTrimmedLength(): number { + for (let i = this.length - 1; i >= 0; --i) { + const s = this.get(i)[CHAR_DATA_CHAR_INDEX]; + if (s !== ' ') { + return i + 1; + } + } + return 0; + } } /** typed array slots taken by one cell */ @@ -279,4 +289,14 @@ export class BufferLineTypedArray implements IBufferLine { newLine.isWrapped = this.isWrapped; return newLine; } + + public getTrimmedLength(): number { + for (let i = this.length - 1; i >= 0; --i) { + const s = this._data[i * CELL_SIZE + Cell.STRING]; + if (s !== 32) { // 32 ==> ' '.charCodeAt(0) + return i + 1; + } + } + return 0; + } } diff --git a/src/InputHandler.ts b/src/InputHandler.ts index b81208d19d..091f41cf7b 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -390,14 +390,6 @@ 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) { diff --git a/src/Linkifier.ts b/src/Linkifier.ts index cbff9bec43..7458b55d16 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -220,12 +220,12 @@ export class Linkifier extends EventEmitter implements ILinkifier { rex.lastIndex = stringIndex + uri.length; // get the buffer index as [absolute row, col] for the match - const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex); + const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex, true); // 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); + const endIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex + uri.length - 1, true); // adjust start index to visible line length if (bufferIndex[1] >= this._terminal.cols) { @@ -244,17 +244,7 @@ export class Linkifier extends EventEmitter implements ILinkifier { if (endIndex[1] > this._terminal.cols) { endIndex[1] = this._terminal.cols; } - let visibleLength = (endIndex[0] - bufferIndex[0]) * this._terminal.cols - bufferIndex[1] + endIndex[1]; - - // patch the visible length by reappending the trimmed content length within terminal.cols - for (let i = bufferIndex[0]; i < endIndex[0]; ++i) { - const line = this._terminal.buffer.lines.get(i); - if (!line) { - break; - } - visibleLength += this._terminal.buffer.translateBufferLineToString(i, false).length - - this._terminal.buffer.translateBufferLineToString(i, true).length; - } + 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]); diff --git a/src/Terminal.test.ts b/src/Terminal.test.ts index ccaa4ec6c8..fd59144c60 100644 --- a/src/Terminal.test.ts +++ b/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(0); - expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(0); + expect(tchar[CHAR_DATA_CHAR_INDEX]).eql(' '); + expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(1); + expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1); 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(0); - expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(0); + expect(tchar[CHAR_DATA_CHAR_INDEX]).eql(' '); + expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(1); + expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1); 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(0); - expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(0); + expect(tchar[CHAR_DATA_CHAR_INDEX]).eql(' '); + expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(1); + expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1); 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); diff --git a/src/Types.ts b/src/Types.ts index e857842616..5b964c08e8 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -296,7 +296,7 @@ export interface IBuffer { nextStop(x?: number): number; prevStop(x?: number): number; getBlankLine(attr: number, isWrapped?: boolean): IBufferLine; - stringIndexToBufferIndex(lineIndex: number, stringIndex: number): number[]; + stringIndexToBufferIndex(lineIndex: number, stringIndex: number, trimRight?: boolean): number[]; iterator(trimRight: boolean, startIndex?: number, endIndex?: number, startOverscan?: number, endOverscan?: number): IBufferStringIterator; } @@ -524,6 +524,7 @@ export interface IBufferLine { fill(fillCharData: CharData): void; copyFrom(line: IBufferLine): void; clone(): IBufferLine; + getTrimmedLength(): number; } export interface IBufferLineConstructor { From 0be98735001e42868921ab498c8078e0b6d3b6ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 26 Oct 2018 04:51:06 +0200 Subject: [PATCH 08/16] remove remnant --- src/Buffer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index ef7ea3a13b..ec4462c266 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -484,7 +484,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) { - // result += this._buffer.translateBufferLineToString(i, (this._trimRight) ? i === range.last : false); result += this._buffer.translateBufferLineToString(i, this._trimRight); } this._current = range.last + 1; From d1fcfd5b0187ffa4f3610f45c9927a0016627367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 26 Oct 2018 15:19:42 +0200 Subject: [PATCH 09/16] test cases, docs --- src/Buffer.test.ts | 20 ++++++++++++++++++++ src/Buffer.ts | 7 ++++++- src/BufferLine.test.ts | 13 +++++++++++++ src/BufferLine.ts | 6 ++---- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index cf6f0cce60..87efb0a472 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -519,6 +519,26 @@ describe('Buffer', () => { assert.deepEqual([(j / terminal.cols) | 0, j % terminal.cols], bufferIndex); } }); + it('should point to correct trimmed .translateBufferLineToString', function(): void { + terminal.writeSync([ + 'abc', // #1 + '１２３４５', // #2 + '￥cafe\u0301￥' // #3 + ].join('\r\n')); + let content = ''; + for (let i = 0; i < 3; ++i) { + const s = terminal.buffer.translateBufferLineToString(i, true); + content += s; + const endIndex = terminal.buffer.stringIndexToBufferIndex(i, s.length - 1, true); + const endChar = terminal.buffer.lines.get(i).get(endIndex[1])[CHAR_DATA_CHAR_INDEX]; + assert.equal(s[s.length - 1], endChar); + // with trim active the next stop after s should be [i + 1, 0] + assert.deepEqual(terminal.buffer.stringIndexToBufferIndex(i, s.length, true), [i + 1, 0]); + } + const lastIndex = terminal.buffer.stringIndexToBufferIndex(0, content.length - 1, true); + const lastChar = terminal.buffer.lines.get(lastIndex[0]).get(lastIndex[1])[CHAR_DATA_CHAR_INDEX]; + assert.equal(content[content.length - 1], lastChar); + }); }); describe('BufferStringIterator', function(): void { it('iterator does not ovrflow buffer limits', function(): void { diff --git a/src/Buffer.ts b/src/Buffer.ts index ec4462c266..f584af182a 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -234,15 +234,20 @@ 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. + * Note: Trimmed cells will be skipped until the next cell with real content. + * Thus it is not possible to get the end of a trimmed string with string.length + * directly. Instead use the last char index and add the char's width to the + * index. * @param lineIndex line index the string was retrieved from * @param stringIndex index within the string * @param startCol column offset the string was retrieved from + * @param trimRight whether to trim spaces from right, defaults to false */ public stringIndexToBufferIndex(lineIndex: number, stringIndex: number, trimRight: boolean = false): BufferIndex { while (stringIndex) { const line = this.lines.get(lineIndex); if (!line) { - [-1, -1]; + return [-1, -1]; } const length = (trimRight) ? line.getTrimmedLength() : line.length; for (let i = 0; i < length; ++i) { diff --git a/src/BufferLine.test.ts b/src/BufferLine.test.ts index b824cc38a2..b674b85a82 100644 --- a/src/BufferLine.test.ts +++ b/src/BufferLine.test.ts @@ -200,4 +200,17 @@ describe('BufferLine', function(): void { chai.expect(line.toArray()).eql(Array(7).fill([1, 'a', 0, 'a'.charCodeAt(0)])); }); }); + it('getTrimmedLength', function(): void { + // Note: trim is currently hardcoded to SP + const line = new TestBufferLine(10, [1, 'a', 0, 'a'.charCodeAt(0)], false); + chai.expect(line.getTrimmedLength()).equals(10); + line.replaceCells(8, 10, [0, ' ', 1, 32]); + chai.expect(line.getTrimmedLength()).equals(8); + line.replaceCells(2, 4, [0, ' ', 1, 32]); + chai.expect(line.getTrimmedLength()).equals(8); + line.replaceCells(4, 8, [0, ' ', 1, 32]); + chai.expect(line.getTrimmedLength()).equals(2); + line.replaceCells(0, 4, [0, ' ', 1, 32]); + chai.expect(line.getTrimmedLength()).equals(0); + }); }); diff --git a/src/BufferLine.ts b/src/BufferLine.ts index b15f7667dc..4782aca8bd 100644 --- a/src/BufferLine.ts +++ b/src/BufferLine.ts @@ -111,8 +111,7 @@ export class BufferLine implements IBufferLine { public getTrimmedLength(): number { for (let i = this.length - 1; i >= 0; --i) { - const s = this.get(i)[CHAR_DATA_CHAR_INDEX]; - if (s !== ' ') { + if (this.get(i)[CHAR_DATA_CHAR_INDEX] !== ' ') { return i + 1; } } @@ -292,8 +291,7 @@ export class BufferLineTypedArray implements IBufferLine { public getTrimmedLength(): number { for (let i = this.length - 1; i >= 0; --i) { - const s = this._data[i * CELL_SIZE + Cell.STRING]; - if (s !== 32) { // 32 ==> ' '.charCodeAt(0) + if (this._data[i * CELL_SIZE + Cell.STRING] !== 32) { // 32 ==> ' '.charCodeAt(0) return i + 1; } } From 36375c62a963f141dd948e01e4f6184de66bc671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 30 Oct 2018 14:05:33 +0100 Subject: [PATCH 10/16] workaround without empty cells --- src/Buffer.ts | 3 ++- src/InputHandler.ts | 3 +++ src/Linkifier.ts | 7 ++++--- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index f584af182a..19f0ccdf2b 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -155,7 +155,8 @@ export class Buffer implements IBuffer { if (this.lines.length > 0) { // Deal with columns increasing (we don't do anything when columns reduce) if (this._terminal.cols < newCols) { - const ch: CharData = [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]; // does xterm use the default attr? + //const ch: CharData = [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]; // does xterm use the default attr? + const ch: CharData = [DEFAULT_ATTR, '', 0, undefined]; for (let i = 0; i < this.lines.length; i++) { this.lines.get(i).resize(newCols, ch); } diff --git a/src/InputHandler.ts b/src/InputHandler.ts index 091f41cf7b..4210817a48 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -390,6 +390,9 @@ export class InputHandler extends Disposable implements IInputHandler { // autowrap - DECAWM // automatically wraps to the beginning of the next line if (wraparoundMode) { + for (let i = buffer.x; i < bufferRow.length; ++i) { + bufferRow.set(i, [curAttr, '', 0, undefined]); + } buffer.x = 0; buffer.y++; if (buffer.y > buffer.scrollBottom) { diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 7458b55d16..a5584ada86 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -110,7 +110,7 @@ export class Linkifier extends EventEmitter implements ILinkifier { // chars will not match anymore at the viewport borders. const overscanLineLimit = Math.ceil(Linkifier.OVERSCAN_CHAR_LIMIT / this._terminal.cols); const iterator = this._terminal.buffer.iterator( - true, absoluteRowIndexStart, absoluteRowIndexEnd, overscanLineLimit, overscanLineLimit); + false, absoluteRowIndexStart, absoluteRowIndexEnd, overscanLineLimit, overscanLineLimit); while (iterator.hasNext()) { const lineData: IBufferStringIteratorResult = iterator.next(); for (let i = 0; i < this._linkMatchers.length; i++) { @@ -220,12 +220,12 @@ export class Linkifier extends EventEmitter implements ILinkifier { rex.lastIndex = stringIndex + uri.length; // get the buffer index as [absolute row, col] for the match - const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex, true); + const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex, false); // 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, true); + const endIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex + uri.length - 1, false); // adjust start index to visible line length if (bufferIndex[1] >= this._terminal.cols) { @@ -245,6 +245,7 @@ export class Linkifier extends EventEmitter implements ILinkifier { endIndex[1] = this._terminal.cols; } const visibleLength = (endIndex[0] - bufferIndex[0]) * this._terminal.cols - bufferIndex[1] + endIndex[1]; + console.log(uri); const line = this._terminal.buffer.lines.get(bufferIndex[0]); const char = line.get(bufferIndex[1]); From fff33374f457629ee1df0daba0189b9d0a211563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 16 Dec 2018 01:04:30 +0100 Subject: [PATCH 11/16] fix tests --- src/Buffer.test.ts | 4 ++-- src/Buffer.ts | 3 +-- src/Terminal.test.ts | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index fdea1d45a8..8cc0067692 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -374,7 +374,7 @@ describe('Buffer', () => { terminal.writeSync(input); const s = terminal.buffer.iterator(true).next().content; // FIXME: currently one space short due to wrong trimming in stringIndexToBufferIndex - assert.equal('Sitting in the cafe\u0301drinking coffee.', s); + assert.equal('Sitting in the cafe\u0301 drinking coffee.', s); for (let i = 0; i < 19; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); assert.deepEqual([(i / terminal.cols) | 0, i % terminal.cols], bufferIndex); @@ -440,7 +440,7 @@ describe('Buffer', () => { terminal.writeSync(input); const s = terminal.buffer.iterator(true).next().content; // FIXME: currently one space short due to wrong trimming in stringIndexToBufferIndex - assert.equal('𓂀\u0301 - the eye hiroglyph with anacute accent.', s); + assert.equal('𓂀\u0301 - the eye hiroglyph with an acute accent.', s); // index 0..2 should map to 0 assert.deepEqual([0, 0], terminal.buffer.stringIndexToBufferIndex(0, 1)); assert.deepEqual([0, 0], terminal.buffer.stringIndexToBufferIndex(0, 2)); diff --git a/src/Buffer.ts b/src/Buffer.ts index 820c054ceb..a164a77f7d 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -164,8 +164,7 @@ export class Buffer implements IBuffer { if (this.lines.length > 0) { // Deal with columns increasing (we don't do anything when columns reduce) if (this._terminal.cols < newCols) { - //const ch: CharData = [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]; // does xterm use the default attr? - const ch: CharData = [DEFAULT_ATTR, '', 0, undefined]; + const ch: CharData = [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]; // does xterm use the default attr? for (let i = 0; i < this.lines.length; i++) { this.lines.get(i).resize(newCols, ch); } diff --git a/src/Terminal.test.ts b/src/Terminal.test.ts index fdc9678be0..9fbcc82ffc 100644 --- a/src/Terminal.test.ts +++ b/src/Terminal.test.ts @@ -930,7 +930,7 @@ 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(0); - expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1); + 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); @@ -955,7 +955,7 @@ 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(0); - expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1); + 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); @@ -1000,7 +1000,7 @@ 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(0); - expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1); + 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); From ae905ec89a387a3bd1355f2ba299192595679c6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 16 Dec 2018 01:11:37 +0100 Subject: [PATCH 12/16] remove readded stuff --- src/Buffer.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 8cc0067692..b9d3472cb5 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -373,8 +373,7 @@ describe('Buffer', () => { const input = 'Sitting in the cafe\u0301 drinking coffee.'; terminal.writeSync(input); const s = terminal.buffer.iterator(true).next().content; - // FIXME: currently one space short due to wrong trimming in stringIndexToBufferIndex - assert.equal('Sitting in the cafe\u0301 drinking coffee.', s); + assert.equal(input, s); for (let i = 0; i < 19; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); assert.deepEqual([(i / terminal.cols) | 0, i % terminal.cols], bufferIndex); @@ -439,8 +438,7 @@ describe('Buffer', () => { const input = '𓂀\u0301 - the eye hiroglyph with an acute accent.'; terminal.writeSync(input); const s = terminal.buffer.iterator(true).next().content; - // FIXME: currently one space short due to wrong trimming in stringIndexToBufferIndex - assert.equal('𓂀\u0301 - the eye hiroglyph with an acute accent.', s); + assert.equal(input, s); // index 0..2 should map to 0 assert.deepEqual([0, 0], terminal.buffer.stringIndexToBufferIndex(0, 1)); assert.deepEqual([0, 0], terminal.buffer.stringIndexToBufferIndex(0, 2)); From fb92452979ccf57fac54f64fcf8e843124dc0c26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 16 Dec 2018 01:19:36 +0100 Subject: [PATCH 13/16] reset cell behind early wrap --- src/InputHandler.ts | 3 ++- src/Terminal.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/InputHandler.ts b/src/InputHandler.ts index 8922d61475..632a0fab33 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -415,8 +415,9 @@ export class InputHandler extends Disposable implements IInputHandler { // autowrap - DECAWM // automatically wraps to the beginning of the next line if (wraparoundMode) { + // nullify leftover cells to the right (happens only for wide chars) for (let i = buffer.x; i < bufferRow.length; ++i) { - bufferRow.set(i, [curAttr, '', 0, undefined]); + bufferRow.set(i, [curAttr, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]); } buffer.x = 0; buffer.y++; diff --git a/src/Terminal.test.ts b/src/Terminal.test.ts index 9fbcc82ffc..fdc9678be0 100644 --- a/src/Terminal.test.ts +++ b/src/Terminal.test.ts @@ -930,7 +930,7 @@ 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(0); - expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(0); + expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1); tchar = term.buffer.lines.get(1).get(0); expect(tchar[CHAR_DATA_CHAR_INDEX]).eql('￥'); expect(tchar[CHAR_DATA_CHAR_INDEX].length).eql(1); @@ -955,7 +955,7 @@ 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(0); - expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(0); + expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1); 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); @@ -1000,7 +1000,7 @@ 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(0); - expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(0); + expect(tchar[CHAR_DATA_WIDTH_INDEX]).eql(1); 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); From c7fa89da8e97e907cdfb72b23eabf5c3a5d1bb9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 16 Dec 2018 01:29:59 +0100 Subject: [PATCH 14/16] apply new trimming to linkifier --- src/Linkifier.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index a5584ada86..23beeeb882 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -110,7 +110,7 @@ export class Linkifier extends EventEmitter implements ILinkifier { // chars will not match anymore at the viewport borders. const overscanLineLimit = Math.ceil(Linkifier.OVERSCAN_CHAR_LIMIT / this._terminal.cols); const iterator = this._terminal.buffer.iterator( - false, absoluteRowIndexStart, absoluteRowIndexEnd, overscanLineLimit, overscanLineLimit); + true, absoluteRowIndexStart, absoluteRowIndexEnd, overscanLineLimit, overscanLineLimit); while (iterator.hasNext()) { const lineData: IBufferStringIteratorResult = iterator.next(); for (let i = 0; i < this._linkMatchers.length; i++) { @@ -220,12 +220,12 @@ export class Linkifier extends EventEmitter implements ILinkifier { rex.lastIndex = stringIndex + uri.length; // get the buffer index as [absolute row, col] for the match - const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex, false); + const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex, true); // 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, false); + const endIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex + uri.length - 1, true); // adjust start index to visible line length if (bufferIndex[1] >= this._terminal.cols) { From 635fc7224e97646211450bcf472f74726deb45a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 18 Dec 2018 00:08:41 +0100 Subject: [PATCH 15/16] handle invalid string buffer indices --- src/Linkifier.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 23beeeb882..3650e24570 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -218,14 +218,23 @@ export class Linkifier extends EventEmitter implements ILinkifier { // also correct regex and string search offsets for the next loop run stringIndex = text.indexOf(uri, stringIndex + 1); rex.lastIndex = stringIndex + uri.length; + if (stringIndex < -1) { + break; + } // get the buffer index as [absolute row, col] for the match const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex, true); + if (bufferIndex[0] < 0) { + break; + } // 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, true); + if (endIndex[0] < 0) { + break; + } // adjust start index to visible line length if (bufferIndex[1] >= this._terminal.cols) { From f91dde86fad7009ed5d77de0176324152c3ef233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 18 Dec 2018 00:14:42 +0100 Subject: [PATCH 16/16] add sanity check for visible length --- src/Linkifier.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 3650e24570..5299d38bb7 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -254,7 +254,9 @@ export class Linkifier extends EventEmitter implements ILinkifier { endIndex[1] = this._terminal.cols; } const visibleLength = (endIndex[0] - bufferIndex[0]) * this._terminal.cols - bufferIndex[1] + endIndex[1]; - console.log(uri); + if (visibleLength < 1) { + continue; + } const line = this._terminal.buffer.lines.get(bufferIndex[0]); const char = line.get(bufferIndex[1]);