From a8fe4107a3b9c2bd9190d97602be19c4d691169c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 9 Sep 2018 21:17:48 +0200 Subject: [PATCH 01/24] Buffer.stringIndexToBufferIndex method --- src/Buffer.test.ts | 152 ++++++++++++++++++++++++++++++++++++++++++++- src/Buffer.ts | 20 ++++++ src/Types.ts | 1 + 3 files changed, 172 insertions(+), 1 deletion(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 6f417f8eb5..5a43a5fefa 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -5,10 +5,11 @@ import { assert } from 'chai'; import { ITerminal } from './Types'; -import { Buffer, DEFAULT_ATTR } from './Buffer'; +import { Buffer, DEFAULT_ATTR, CHAR_DATA_CHAR_INDEX } from './Buffer'; import { CircularList } from './common/CircularList'; import { MockTerminal } from './utils/TestUtils.test'; import { BufferLine } from './BufferLine'; +import { Terminal } from './Terminal'; const INIT_COLS = 80; const INIT_ROWS = 24; @@ -347,4 +348,153 @@ describe('Buffer', () => { assert.equal(str3, '😁a'); }); }); + describe('stringIndexToBufferIndex', function(): void { + beforeEach(function(): void { + terminal = new Terminal({rows: 5, cols: 10}); + const oldWrite: any = terminal.write.bind(terminal); + terminal.write = (s: string): void => { + oldWrite(s); + (terminal as any)._innerWrite(); + } + }); + function wholeString(terminal: ITerminal, range : {first: number, last: number}): string { + let result = ''; + for (let i = range.first; i <= range.last; ++i) { + result += terminal.buffer.translateBufferLineToString(i, i == range.last); + } + return result; + } + it('multiline ascii', function(): void { + const input = 'This is ASCII text spanning multiple lines.'; + terminal.write(input); + const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + assert.equal(input, s); + for (let i = 0; i < input.length; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([(i/terminal.cols) | 0, i % terminal.cols], bufferIndex); + } + }); + it('combining e\u0301 in a sentence', function(): void { + const input = 'Sitting in the cafe\u0301 drinking coffee.'; + terminal.write(input); + const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + 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); + } + // string index 18 & 19 point to combining char e\u0301 ---> same buffer Index + assert.deepEqual( + terminal.buffer.stringIndexToBufferIndex(0, 18), + terminal.buffer.stringIndexToBufferIndex(0, 19)); + // after the combining char every string index has an offset of -1 + for (let i = 19; i < input.length; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([((i - 1)/terminal.cols) | 0, (i - 1) % terminal.cols], bufferIndex); + } + }); + it('multiline combining e\u0301', function(): void { + const input = 'e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301'; + terminal.write(input); + const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + assert.equal(input, s); + // every buffer cell index contains 2 string indices + for (let i = 0; i < input.length; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([((i >> 1)/terminal.cols) | 0, (i >> 1) % terminal.cols], bufferIndex); + } + }); + it('surrogate char in a sentence', function(): void { + const input = 'The 𝄞 is a clef widely used in modern notation.'; + terminal.write(input); + const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + assert.equal(input, s); + for (let i = 0; i < 5; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([(i/terminal.cols) | 0, i % terminal.cols], bufferIndex); + } + // string index 4 & 5 point to surrogate char 𝄞 ---> same buffer Index + assert.deepEqual( + terminal.buffer.stringIndexToBufferIndex(0, 4), + terminal.buffer.stringIndexToBufferIndex(0, 5)); + // after the combining char every string index has an offset of -1 + for (let i = 5; i < input.length; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([((i - 1)/terminal.cols) | 0, (i - 1) % terminal.cols], bufferIndex); + } + }); + it('multiline surrogate char', function(): void { + const input = '𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞'; + terminal.write(input); + const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + assert.equal(input, s); + // every buffer cell index contains 2 string indices + for (let i = 0; i < input.length; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([((i >> 1)/terminal.cols) | 0, (i >> 1) % terminal.cols], bufferIndex); + } + }); + it('surrogate char with combining', function(): void { + // eye of Ra with acute accent - string length of 3 + const input = '𓂀\u0301 - the eye hiroglyph with an acute accent.'; + terminal.write(input); + const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + 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)); + for (let i = 2; i < input.length; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([((i - 2)/terminal.cols) | 0, (i - 2) % terminal.cols], bufferIndex); + } + }); + it('multiline surrogate with combining', function(): void { + const input = '𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301'; + terminal.write(input); + const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + assert.equal(input, s); + // every buffer cell index contains 3 string indices + for (let i = 0; i < input.length; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([(((i / 3) | 0) /terminal.cols) | 0, ((i / 3) | 0) % terminal.cols], bufferIndex); + } + }); + it('fullwidth chars', function(): void { + const input = 'These 123 are some fat numbers.'; + terminal.write(input); + const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + assert.equal(input, s); + for (let i = 0; i < 6; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([(i/terminal.cols) | 0, i % terminal.cols], bufferIndex); + } + // string index 6, 7, 8 take 2 cells + assert.deepEqual([0, 8], terminal.buffer.stringIndexToBufferIndex(0, 7)); + assert.deepEqual([1, 0], terminal.buffer.stringIndexToBufferIndex(0, 8)); + // rest of the string has offset of +3 + for (let i = 9; i < input.length; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([((i + 3)/terminal.cols) | 0, (i + 3) % terminal.cols], bufferIndex); + } + }); + it('multiline fullwidth chars', function(): void { + const input = '12345678901234567890'; + terminal.write(input); + const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + assert.equal(input, s); + for (let i = 9; i < input.length; ++i) { + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); + assert.deepEqual([((i << 1)/terminal.cols) | 0, (i << 1) % terminal.cols], bufferIndex); + } + }); + it('fullwidth combining with emoji - match emoji cell', function(): void { + const input = 'Lots of ¥\u0301 make me 😃.'; + terminal.write(input); + const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + assert.equal(input, s); + const stringIndex = s.match(/😃/).index; + const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, stringIndex); + assert(terminal.buffer.lines.get(bufferIndex[0]).get(bufferIndex[1])[CHAR_DATA_CHAR_INDEX], '😃'); + }); + }); }); diff --git a/src/Buffer.ts b/src/Buffer.ts index 9487be4a67..cb7d3b8771 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -194,6 +194,26 @@ export class Buffer implements IBuffer { this.scrollBottom = newRows - 1; } + public stringIndexToBufferIndex(lineIndex: number, stringIndex: number): number[] { + if (!stringIndex) { + return [lineIndex, 0]; + } + while (stringIndex) { + let line = this.lines.get(lineIndex); + if (!line) { + [-1, -1]; + } + for (let i = 0; i < line.length; ++i) { + stringIndex -= line.get(i)[CHAR_DATA_CHAR_INDEX].length; + if (stringIndex < 0) { + return [lineIndex, i]; + } + } + lineIndex++; + } + return [lineIndex, 0]; + } + /** * Translates a buffer line to a string, with optional start and end columns. * Wide characters will count as two columns in the resulting string. This diff --git a/src/Types.ts b/src/Types.ts index df3f1a7984..820a8b89ee 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -282,6 +282,7 @@ export interface IBuffer { getWrappedRangeForLine(y: number): { first: number, last: number }; nextStop(x?: number): number; prevStop(x?: number): number; + stringIndexToBufferIndex(lineIndex: number, stringIndex: number): number[]; } export interface IBufferSet extends IEventEmitter { From a494e80a9f38ceb4506acccfba5df6df71cf5054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 9 Sep 2018 22:17:49 +0200 Subject: [PATCH 02/24] BufferStringIterator --- src/Buffer.test.ts | 59 ++++++++++++++++++++-------------------------- src/Buffer.ts | 41 +++++++++++++++++++++++++++++--- src/Types.ts | 10 ++++++++ 3 files changed, 74 insertions(+), 36 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 5a43a5fefa..1bd1ea380a 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -355,33 +355,26 @@ describe('Buffer', () => { terminal.write = (s: string): void => { oldWrite(s); (terminal as any)._innerWrite(); - } + }; }); - function wholeString(terminal: ITerminal, range : {first: number, last: number}): string { - let result = ''; - for (let i = range.first; i <= range.last; ++i) { - result += terminal.buffer.translateBufferLineToString(i, i == range.last); - } - return result; - } it('multiline ascii', function(): void { const input = 'This is ASCII text spanning multiple lines.'; terminal.write(input); - const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); for (let i = 0; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([(i/terminal.cols) | 0, i % terminal.cols], bufferIndex); + assert.deepEqual([(i / terminal.cols) | 0, i % terminal.cols], bufferIndex); } }); it('combining e\u0301 in a sentence', function(): void { const input = 'Sitting in the cafe\u0301 drinking coffee.'; terminal.write(input); - const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + const s = terminal.buffer.contents(true).toArray()[0]; 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); + assert.deepEqual([(i / terminal.cols) | 0, i % terminal.cols], bufferIndex); } // string index 18 & 19 point to combining char e\u0301 ---> same buffer Index assert.deepEqual( @@ -390,28 +383,28 @@ describe('Buffer', () => { // after the combining char every string index has an offset of -1 for (let i = 19; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([((i - 1)/terminal.cols) | 0, (i - 1) % terminal.cols], bufferIndex); + assert.deepEqual([((i - 1) / terminal.cols) | 0, (i - 1) % terminal.cols], bufferIndex); } }); it('multiline combining e\u0301', function(): void { const input = 'e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301'; terminal.write(input); - const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); - // every buffer cell index contains 2 string indices + // every buffer cell index contains 2 string indices for (let i = 0; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([((i >> 1)/terminal.cols) | 0, (i >> 1) % terminal.cols], bufferIndex); + assert.deepEqual([((i >> 1) / terminal.cols) | 0, (i >> 1) % terminal.cols], bufferIndex); } }); it('surrogate char in a sentence', function(): void { const input = 'The 𝄞 is a clef widely used in modern notation.'; terminal.write(input); - const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); for (let i = 0; i < 5; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([(i/terminal.cols) | 0, i % terminal.cols], bufferIndex); + assert.deepEqual([(i / terminal.cols) | 0, i % terminal.cols], bufferIndex); } // string index 4 & 5 point to surrogate char 𝄞 ---> same buffer Index assert.deepEqual( @@ -420,53 +413,53 @@ describe('Buffer', () => { // after the combining char every string index has an offset of -1 for (let i = 5; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([((i - 1)/terminal.cols) | 0, (i - 1) % terminal.cols], bufferIndex); + assert.deepEqual([((i - 1) / terminal.cols) | 0, (i - 1) % terminal.cols], bufferIndex); } }); it('multiline surrogate char', function(): void { const input = '𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞'; terminal.write(input); - const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); - // every buffer cell index contains 2 string indices + // every buffer cell index contains 2 string indices for (let i = 0; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([((i >> 1)/terminal.cols) | 0, (i >> 1) % terminal.cols], bufferIndex); + assert.deepEqual([((i >> 1) / terminal.cols) | 0, (i >> 1) % terminal.cols], bufferIndex); } }); it('surrogate char with combining', function(): void { // eye of Ra with acute accent - string length of 3 const input = '𓂀\u0301 - the eye hiroglyph with an acute accent.'; terminal.write(input); - const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + const s = terminal.buffer.contents(true).toArray()[0]; 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)); for (let i = 2; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([((i - 2)/terminal.cols) | 0, (i - 2) % terminal.cols], bufferIndex); + assert.deepEqual([((i - 2) / terminal.cols) | 0, (i - 2) % terminal.cols], bufferIndex); } }); it('multiline surrogate with combining', function(): void { const input = '𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301'; terminal.write(input); - const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); - // every buffer cell index contains 3 string indices + // every buffer cell index contains 3 string indices for (let i = 0; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([(((i / 3) | 0) /terminal.cols) | 0, ((i / 3) | 0) % terminal.cols], bufferIndex); + assert.deepEqual([(((i / 3) | 0) / terminal.cols) | 0, ((i / 3) | 0) % terminal.cols], bufferIndex); } }); it('fullwidth chars', function(): void { const input = 'These 123 are some fat numbers.'; terminal.write(input); - const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); for (let i = 0; i < 6; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([(i/terminal.cols) | 0, i % terminal.cols], bufferIndex); + assert.deepEqual([(i / terminal.cols) | 0, i % terminal.cols], bufferIndex); } // string index 6, 7, 8 take 2 cells assert.deepEqual([0, 8], terminal.buffer.stringIndexToBufferIndex(0, 7)); @@ -474,23 +467,23 @@ describe('Buffer', () => { // rest of the string has offset of +3 for (let i = 9; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([((i + 3)/terminal.cols) | 0, (i + 3) % terminal.cols], bufferIndex); + assert.deepEqual([((i + 3) / terminal.cols) | 0, (i + 3) % terminal.cols], bufferIndex); } }); it('multiline fullwidth chars', function(): void { const input = '12345678901234567890'; terminal.write(input); - const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); for (let i = 9; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); - assert.deepEqual([((i << 1)/terminal.cols) | 0, (i << 1) % terminal.cols], bufferIndex); + assert.deepEqual([((i << 1) / terminal.cols) | 0, (i << 1) % terminal.cols], bufferIndex); } }); it('fullwidth combining with emoji - match emoji cell', function(): void { const input = 'Lots of ¥\u0301 make me 😃.'; terminal.write(input); - const s = wholeString(terminal, terminal.buffer.getWrappedRangeForLine(0)); + const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); const stringIndex = s.match(/😃/).index; const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, stringIndex); diff --git a/src/Buffer.ts b/src/Buffer.ts index cb7d3b8771..bc146a15cd 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -4,7 +4,7 @@ */ import { CircularList } from './common/CircularList'; -import { CharData, ITerminal, IBuffer, IBufferLine } from './Types'; +import { CharData, ITerminal, IBuffer, IBufferLine, BufferIndex, IBufferStringIterator } from './Types'; import { EventEmitter } from './common/EventEmitter'; import { IMarker } from 'xterm'; import { BufferLine } from './BufferLine'; @@ -194,12 +194,12 @@ export class Buffer implements IBuffer { this.scrollBottom = newRows - 1; } - public stringIndexToBufferIndex(lineIndex: number, stringIndex: number): number[] { + public stringIndexToBufferIndex(lineIndex: number, stringIndex: number): BufferIndex { if (!stringIndex) { return [lineIndex, 0]; } while (stringIndex) { - let line = this.lines.get(lineIndex); + const line = this.lines.get(lineIndex); if (!line) { [-1, -1]; } @@ -360,6 +360,10 @@ export class Buffer implements IBuffer { // TODO: This could probably be optimized by relying on sort order and trimming the array using .length this.markers.splice(this.markers.indexOf(marker), 1); } + + public contents(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator { + return new BufferStringIterator(this, trimRight, startIndex, endIndex); + } } export class Marker extends EventEmitter implements IMarker { @@ -386,3 +390,34 @@ export class Marker extends EventEmitter implements IMarker { super.dispose(); } } + +export class BufferStringIterator implements IBufferStringIterator { + private _start: number; + private _end: number; + private _current: number; + constructor (private _buffer: IBuffer, private _trimRight: boolean, startIndex?: number, endIndex?: number) { + this._start = startIndex || 0; + this._end = endIndex || this._buffer.lines.length; + this._current = this._start; + } + public hasNext(): boolean { + return this._current < this._end; + } + public next(withRanges: boolean = false): string | [{first: number, last: number}, string] { + const range = this._buffer.getWrappedRangeForLine(this._current); + let result = ''; + for (let i = range.first; i <= range.last; ++i) { + result += this._buffer.translateBufferLineToString(i, (this._trimRight) ? i === range.last : false); + } + this._current = range.last; + this._current++; + return (withRanges) ? [range, result] : result; + } + toArray(): string[] { + const result: string[] = []; + while (this.hasNext()) { + result.push(this.next() as string); + } + return result; + } +} diff --git a/src/Types.ts b/src/Types.ts index 820a8b89ee..5782f2c9ad 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -19,6 +19,9 @@ export type LinkMatcherValidationCallback = (uri: string, callback: (isValid: bo export type CharacterJoinerHandler = (text: string) => [number, number][]; +// BufferIndex denotes a position in the buffer: [rowIndex, colIndex] +export type BufferIndex = [number, number]; + export const enum LinkHoverEventTypes { HOVER = 'linkhover', TOOLTIP = 'linktooltip', @@ -265,6 +268,12 @@ export interface ITerminalOptions extends IPublicTerminalOptions { useFlowControl?: boolean; } +export interface IBufferStringIterator { + hasNext(): boolean; + next(withRanges: boolean): string | [{first: number, last: number}, string]; + toArray(): string[]; +} + export interface IBuffer { readonly lines: ICircularList; ydisp: number; @@ -283,6 +292,7 @@ export interface IBuffer { nextStop(x?: number): number; prevStop(x?: number): number; stringIndexToBufferIndex(lineIndex: number, stringIndex: number): number[]; + contents(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator; } export interface IBufferSet extends IEventEmitter { From 4aa8e402a440ab93a1fda6b52ddd89884ba36940 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 9 Sep 2018 22:21:00 +0200 Subject: [PATCH 03/24] fix MockBuffer --- src/utils/TestUtils.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index 32a7e9bc03..a447c8e854 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -4,7 +4,7 @@ */ import { IColorSet, IRenderer, IRenderDimensions, IColorManager } from '../renderer/Types'; -import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminal, IBuffer, IBufferSet, IBrowser, ICharMeasure, ISelectionManager, ITerminalOptions, ILinkifier, IMouseHelper, ILinkMatcherOptions, CharacterJoinerHandler, IBufferLine } from '../Types'; +import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminal, IBuffer, IBufferSet, IBrowser, ICharMeasure, ISelectionManager, ITerminalOptions, ILinkifier, IMouseHelper, ILinkMatcherOptions, CharacterJoinerHandler, IBufferLine, IBufferStringIterator } from '../Types'; import { ICircularList, XtermListener } from '../common/Types'; import { Buffer } from '../Buffer'; import * as Browser from '../shared/utils/Browser'; @@ -311,6 +311,12 @@ export class MockBuffer implements IBuffer { setLines(lines: ICircularList): void { this.lines = lines; } + stringIndexToBufferIndex(lineIndex: number, stringIndex: number): number[] { + throw new Error('Method not implemented.'); + } + contents(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator { + throw new Error('Method not implemented.'); + } } export class MockRenderer implements IRenderer { From daa6fa0a667a510a56d28fe0d0f5a93e3e0b3f21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Mon, 10 Sep 2018 00:40:29 +0200 Subject: [PATCH 04/24] change Linkifier._doLinkifyRow --- src/Linkifier.ts | 105 ++++++++++++------------------------ src/utils/TestUtils.test.ts | 8 +-- 2 files changed, 38 insertions(+), 75 deletions(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 74fc72db8a..fe896f91d5 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -163,86 +163,49 @@ export class Linkifier extends EventEmitter implements ILinkifier { if (absoluteRowIndex >= this._terminal.buffer.lines.length) { return; } - - if (this._terminal.buffer.lines.get(absoluteRowIndex).isWrapped) { - // Only attempt to linkify rows that start in the viewport - if (rowIndex !== 0) { - 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]); } - // If the first row is wrapped, backtrack to find the origin row and linkify that - let line: IBufferLine; - - do { - rowIndex--; - absoluteRowIndex--; - line = this._terminal.buffer.lines.get(absoluteRowIndex); - - if (!line) { - break; - } - - } while (line.isWrapped); - } - - // Construct full unwrapped line text - let text = this._terminal.buffer.translateBufferLineToString(absoluteRowIndex, false); - let currentIndex = absoluteRowIndex + 1; - while (currentIndex < this._terminal.buffer.lines.length && - this._terminal.buffer.lines.get(currentIndex).isWrapped) { - text += this._terminal.buffer.translateBufferLineToString(currentIndex++, false); - } - - for (let i = 0; i < this._linkMatchers.length; i++) { - this._doLinkifyRow(rowIndex, text, this._linkMatchers[i]); } } /** * Linkifies a row given a specific handler. - * @param rowIndex The row index to linkify. - * @param text The text of the row (excludes text in the row that's already - * linkified). + * @param rowIndex The row index to linkify (absolute index). + * @param text string content of the unwrapped row. * @param matcher The link matcher for this line. - * @param offset The how much of the row has already been linkified. - * @return The link element(s) that were added. */ - private _doLinkifyRow(rowIndex: number, text: string, matcher: ILinkMatcher, offset: number = 0): void { - // Find the first match - const match = text.match(matcher.regex); - if (!match || match.length === 0) { - return; - } - const uri = match[typeof matcher.matchIndex !== 'number' ? 0 : matcher.matchIndex]; - - // Get index, match.index is for the outer match which includes negated chars - const index = text.indexOf(uri); - - // Get cell color - const line = this._terminal.buffer.lines.get(this._terminal.buffer.ydisp + rowIndex); - const char = line.get(index); - const attr: number = char[CHAR_DATA_ATTR_INDEX]; - const fg = (attr >> 9) & 0x1ff; - - // Ensure the link is valid before registering - if (matcher.validationCallback) { - matcher.validationCallback(uri, isValid => { - // Discard link if the line has already changed - if (this._rowsTimeoutId) { - return; - } - if (isValid) { - this._addLink(offset + index, rowIndex, uri, matcher, fg); - } - }); - } else { - this._addLink(offset + index, rowIndex, uri, matcher, fg); - } + private _doLinkifyRow(rowIndex: number, text: string, matcher: ILinkMatcher): void { + 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]; + stringIndex = text.indexOf(uri, stringIndex + 1); + rex.lastIndex = stringIndex + uri.length; + const bufferIndex = this._terminal.buffer.stringIndexToBufferIndex(rowIndex, stringIndex); + const line = this._terminal.buffer.lines.get(bufferIndex[0]); + const char = line.get(bufferIndex[1]); + const attr: number = char[CHAR_DATA_ATTR_INDEX]; + const fg = (attr >> 9) & 0x1ff; - // Recursively check for links in the rest of the text - const remainingStartIndex = index + uri.length; - const remainingText = text.substr(remainingStartIndex); - if (remainingText.length > 0) { - this._doLinkifyRow(rowIndex, remainingText, matcher, offset + remainingStartIndex); + if (matcher.validationCallback) { + matcher.validationCallback(uri, isValid => { + // Discard link if the line has already changed + if (this._rowsTimeoutId) { + return; + } + if (isValid) { + this._addLink(bufferIndex[1], bufferIndex[0] - this._terminal.buffer.ydisp, uri, matcher, fg); + } + }); + } else { + this._addLink(bufferIndex[1], bufferIndex[0] - this._terminal.buffer.ydisp, uri, matcher, fg); + } } } diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index a447c8e854..d454210f9f 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -6,7 +6,7 @@ import { IColorSet, IRenderer, IRenderDimensions, IColorManager } from '../renderer/Types'; import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminal, IBuffer, IBufferSet, IBrowser, ICharMeasure, ISelectionManager, ITerminalOptions, ILinkifier, IMouseHelper, ILinkMatcherOptions, CharacterJoinerHandler, IBufferLine, IBufferStringIterator } from '../Types'; import { ICircularList, XtermListener } from '../common/Types'; -import { Buffer } from '../Buffer'; +import { Buffer, BufferStringIterator } from '../Buffer'; import * as Browser from '../shared/utils/Browser'; import { ITheme, IDisposable, IMarker } from 'xterm'; @@ -300,7 +300,7 @@ export class MockBuffer implements IBuffer { return Buffer.prototype.translateBufferLineToString.apply(this, arguments); } getWrappedRangeForLine(y: number): { first: number; last: number; } { - throw new Error('Method not implemented.'); + return Buffer.prototype.getWrappedRangeForLine.apply(this, arguments); } nextStop(x?: number): number { throw new Error('Method not implemented.'); @@ -312,10 +312,10 @@ export class MockBuffer implements IBuffer { this.lines = lines; } stringIndexToBufferIndex(lineIndex: number, stringIndex: number): number[] { - throw new Error('Method not implemented.'); + return Buffer.prototype.stringIndexToBufferIndex.apply(this, arguments); } contents(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator { - throw new Error('Method not implemented.'); + return Buffer.prototype.contents.apply(this, arguments); } } From 08af72be1aa72a102c0ab85e0d165dd3d6a2c35a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Mon, 10 Sep 2018 01:13:21 +0200 Subject: [PATCH 05/24] linter errors --- src/Linkifier.ts | 5 ++--- src/utils/TestUtils.test.ts | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index fe896f91d5..70fa9b157f 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -4,7 +4,7 @@ */ import { IMouseZoneManager } from './ui/Types'; -import { ILinkHoverEvent, ILinkMatcher, LinkMatcherHandler, LinkHoverEventTypes, ILinkMatcherOptions, ILinkifier, ITerminal, IBufferLine } from './Types'; +import { ILinkHoverEvent, ILinkMatcher, LinkMatcherHandler, LinkHoverEventTypes, ILinkMatcherOptions, ILinkifier, ITerminal } from './Types'; import { MouseZone } from './ui/MouseZoneManager'; import { EventEmitter } from './common/EventEmitter'; import { CHAR_DATA_ATTR_INDEX } from './Buffer'; @@ -159,11 +159,10 @@ export class Linkifier extends EventEmitter implements ILinkifier { */ private _linkifyRow(rowIndex: number): void { // Ensure the row exists - let absoluteRowIndex = this._terminal.buffer.ydisp + rowIndex; + 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++) { diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index d454210f9f..6fa0c87a3b 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -6,7 +6,7 @@ import { IColorSet, IRenderer, IRenderDimensions, IColorManager } from '../renderer/Types'; import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminal, IBuffer, IBufferSet, IBrowser, ICharMeasure, ISelectionManager, ITerminalOptions, ILinkifier, IMouseHelper, ILinkMatcherOptions, CharacterJoinerHandler, IBufferLine, IBufferStringIterator } from '../Types'; import { ICircularList, XtermListener } from '../common/Types'; -import { Buffer, BufferStringIterator } from '../Buffer'; +import { Buffer } from '../Buffer'; import * as Browser from '../shared/utils/Browser'; import { ITheme, IDisposable, IMarker } from 'xterm'; From a76d4a6055d6b9ded964618a9f3d7059085e43a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Mon, 10 Sep 2018 16:10:41 +0200 Subject: [PATCH 06/24] fix multiple row linkifier attempts; tests for unicode before a match --- src/Linkifier.test.ts | 64 +++++++++++++++++++++++++++++++++++++++++++ src/Linkifier.ts | 44 +++++++++++++++-------------- 2 files changed, 87 insertions(+), 21 deletions(-) diff --git a/src/Linkifier.test.ts b/src/Linkifier.test.ts index eeaaeedcc1..6eb85b91a3 100644 --- a/src/Linkifier.test.ts +++ b/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('12 foo', /foo/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); + }); + it('fullwidth before - match over two lines', function(done: () => void): void { + assertLinkifiesInTerminal('12 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); + }); + }); }); diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 70fa9b157f..44a91a36ec 100644 --- a/src/Linkifier.ts +++ b/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]); From c35306aaa6aadd7ac965a91b3d1d62cd0d0a1e47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Mon, 10 Sep 2018 17:47:29 +0200 Subject: [PATCH 07/24] handle unicode within a match; test cases --- src/CharWidth.ts | 23 +++++++++++ src/Linkifier.test.ts | 90 +++++++++++++++++++++++++++++-------------- src/Linkifier.ts | 7 +++- 3 files changed, 90 insertions(+), 30 deletions(-) diff --git a/src/CharWidth.ts b/src/CharWidth.ts index 6e4aa37fbd..26aaba0234 100644 --- a/src/CharWidth.ts +++ b/src/CharWidth.ts @@ -169,3 +169,26 @@ export const wcwidth = (function(opts: {nul: number, control: number}): (ucs: nu return wcwidthHigh(num); }; })({nul: 0, control: 0}); // configurable options + +/** + * Get the terminal cell width for a string. + * @param s + */ +export function stringWidth(s: string): number { + let result = 0; + for (let i = 0; i < s.length; ++i) { + let code = s.charCodeAt(i); + if (0xD800 <= code && code <= 0xDBFF) { + const low = s.charCodeAt(i + 1); + if (isNaN(low)) { + return result; + } + code = ((code - 0xD800) * 0x400) + (low - 0xDC00) + 0x10000; + } + if (0xDC00 <= code && code <= 0xDFFF) { + continue; + } + result += wcwidth(code); + } + return result; +} diff --git a/src/Linkifier.test.ts b/src/Linkifier.test.ts index 6eb85b91a3..32b4ee3be9 100644 --- a/src/Linkifier.test.ts +++ b/src/Linkifier.test.ts @@ -271,35 +271,69 @@ describe('Linkifier', () => { }, 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('12 foo', /foo/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); - }); - it('fullwidth before - match over two lines', function(done: () => void): void { - assertLinkifiesInTerminal('12 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); + describe('unicode before the match', function(): void { + 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); + }); + 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); + }); + it('surrogate - match within one line', function(done: () => void): void { + assertLinkifiesInTerminal('𝄞𝄞𝄞 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); + }); + 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); + }); + 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); + }); + it('fullwidth - match within one line', function(done: () => void): void { + assertLinkifiesInTerminal('12 foo', /foo/, [{x1: 5, x2: 8, y1: 0, y2: 0}], done); + }); + it('fullwidth - match over two lines', function(done: () => void): void { + assertLinkifiesInTerminal('12 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); + }); + 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); + }); }); - 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); + describe('unicode within the match', function(): void { + 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); + }); + 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); + }); + 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); + }); + 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); + }); + 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); + }); + 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); + }); + it('fullwidth - match within one line', function(done: () => void): void { + assertLinkifiesInTerminal('test a1b', /a1b/, [{x1: 5, x2: 9, y1: 0, y2: 0}], done); + }); + it('fullwidth - match over two lines', function(done: () => void): void { + assertLinkifiesInTerminal('testtest a1b', /a1b/, [{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); + }); + 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); + }); }); }); }); diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 17e59f1cb6..16d2fd7ab5 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -8,6 +8,7 @@ import { ILinkHoverEvent, ILinkMatcher, LinkMatcherHandler, LinkHoverEventTypes, import { MouseZone } from './ui/MouseZoneManager'; import { EventEmitter } from './common/EventEmitter'; import { CHAR_DATA_ATTR_INDEX } from './Buffer'; +import { stringWidth } from './CharWidth'; /** * The Linkifier applies links to rows shortly after they have been refreshed. @@ -222,15 +223,17 @@ export class Linkifier extends EventEmitter implements ILinkifier { * @param fg The link color for hover event. */ private _addLink(x: number, y: number, uri: string, matcher: ILinkMatcher, fg: number): void { + const length = stringWidth(uri); const x1 = x % this._terminal.cols; const y1 = y + Math.floor(x / this._terminal.cols); - let x2 = (x1 + uri.length) % this._terminal.cols; - let y2 = y1 + Math.floor((x1 + uri.length) / this._terminal.cols); + let x2 = (x1 + length) % this._terminal.cols; + let y2 = y1 + Math.floor((x1 + length) / this._terminal.cols); if (x2 === 0) { x2 = this._terminal.cols; y2--; } + this._mouseZoneManager.add(new MouseZone( x1 + 1, y1 + 1, From 7e69c5d0dbdf5c66479c2800ced18b7c0ad8f238 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 12 Sep 2018 15:37:05 +0200 Subject: [PATCH 08/24] apply linkifier only to rows in viewport --- src/Linkifier.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 16d2fd7ab5..a213555033 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -89,6 +89,8 @@ export class Linkifier extends EventEmitter implements ILinkifier { // 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 + // for wrapped content over several rows the iterator might return rows outside the viewport + // we skip those later in _doLinkifyRow 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++) { @@ -174,7 +176,7 @@ 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 + // clone regex to do a global search on text const rex = new RegExp(matcher.regex.source, matcher.regex.flags + 'g'); let match; let stringIndex = -1; @@ -188,8 +190,16 @@ export class Linkifier extends EventEmitter implements ILinkifier { 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); + + // skip rows outside of the viewport + if (bufferIndex[0] - this._terminal.buffer.ydisp < 0) { + continue; + } + if (bufferIndex[0] - this._terminal.buffer.ydisp > this._terminal.rows) { + break; + } + const line = this._terminal.buffer.lines.get(bufferIndex[0]); const char = line.get(bufferIndex[1]); let fg: number | undefined; From c8c22e3ddfa4825f2682e9eebdc003f2d42c28f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 13 Sep 2018 14:30:11 +0200 Subject: [PATCH 09/24] docs --- src/Buffer.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/Buffer.ts b/src/Buffer.ts index bc146a15cd..e7a35d247c 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -194,6 +194,18 @@ export class Buffer implements IBuffer { this.scrollBottom = newRows - 1; } + /** + * Translates a string index back to a BufferIndex. + * To get the correct buffer position the string must start at `startCol` 0 + * (default in translateBufferLineToString). + * The method also works on wrapped line strings given rows were not trimmed. + * The method operates on the CharData width attribute, there are no + * additional content or boundary checks. Therefore the string and the buffer + * should not be altered in between. + * @param lineIndex line index the string was retrieved from + * @param stringIndex index within the string + * @param startCol column offset the string was retrieved from + */ public stringIndexToBufferIndex(lineIndex: number, stringIndex: number): BufferIndex { if (!stringIndex) { return [lineIndex, 0]; From 2708a76501348b940cc09ce7546b6b8e0c826c3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 13 Sep 2018 15:44:26 +0200 Subject: [PATCH 10/24] fix next() invocation; handle wild matches --- src/Linkifier.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index a213555033..9ff8b823e8 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -93,8 +93,8 @@ export class Linkifier extends EventEmitter implements ILinkifier { // we skip those later in _doLinkifyRow const linesIterator = this._terminal.buffer.contents(false, absoluteRowIndexStart, this._terminal.buffer.ydisp + this._rowsToLinkify.end + 1); while (linesIterator.hasNext()) { + const lineData: any = linesIterator.next(true); 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]); } } @@ -182,6 +182,12 @@ export class Linkifier extends EventEmitter implements ILinkifier { let stringIndex = -1; while ((match = rex.exec(text)) !== null) { const uri = match[typeof matcher.matchIndex !== 'number' ? 0 : matcher.matchIndex]; + if (!uri) { + // something matched but does not comply with the given matchIndex + // since this is most likely a bug the regex itself we simply do nothing here + // TODO: should this be logged for debugging? + break; + } // 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? From 30f74bf5f3a96904c70224b20cf042b43f24b9c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 13 Sep 2018 16:20:37 +0200 Subject: [PATCH 11/24] cover & comment right border trim behavior to be fixed --- src/Buffer.test.ts | 17 +++++++++++++++++ src/Buffer.ts | 2 ++ 2 files changed, 19 insertions(+) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 1bd1ea380a..aff9a57ac1 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -489,5 +489,22 @@ describe('Buffer', () => { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, stringIndex); 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)', function(): void { + const input = 'a12345678901234567890'; + // 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 + terminal.write(input); + // 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 + 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 e7a35d247c..b807f51eb6 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -202,6 +202,7 @@ export class Buffer implements IBuffer { * The method operates on the CharData width attribute, 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 @@ -419,6 +420,7 @@ export class BufferStringIterator implements IBufferStringIterator { const range = this._buffer.getWrappedRangeForLine(this._current); 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; From a8d9c92ae1c9628391751053aa2b04e8c1ba706f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 13 Sep 2018 19:03:57 +0200 Subject: [PATCH 12/24] make linter happy --- src/Buffer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index b807f51eb6..3a26fb79dd 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -420,7 +420,7 @@ export class BufferStringIterator implements IBufferStringIterator { const range = this._buffer.getWrappedRangeForLine(this._current); let result = ''; for (let i = range.first; i <= range.last; ++i) { - // TODO: always apply trimRight after fixing #1685 + // TODO: always apply trimRight after fixing #1685 result += this._buffer.translateBufferLineToString(i, (this._trimRight) ? i === range.last : false); } this._current = range.last; From 74015678569bb6807458def0df6b4557f7c67262 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 13 Sep 2018 19:34:44 -0700 Subject: [PATCH 13/24] Remove unsafe cast in Buffer.test.ts --- src/Buffer.test.ts | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index aff9a57ac1..2ad86b22e1 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -14,6 +14,13 @@ import { Terminal } from './Terminal'; const INIT_COLS = 80; const INIT_ROWS = 24; +class TestTerminal extends Terminal { + writeSync(data: string): void { + this.writeBuffer.push(data); + this._innerWrite(); + } +} + describe('Buffer', () => { let terminal: ITerminal; let buffer: Buffer; @@ -349,17 +356,13 @@ describe('Buffer', () => { }); }); describe('stringIndexToBufferIndex', function(): void { + let terminal: TestTerminal; beforeEach(function(): void { - terminal = new Terminal({rows: 5, cols: 10}); - const oldWrite: any = terminal.write.bind(terminal); - terminal.write = (s: string): void => { - oldWrite(s); - (terminal as any)._innerWrite(); - }; + terminal = new TestTerminal({rows: 5, cols: 10}); }); it('multiline ascii', function(): void { const input = 'This is ASCII text spanning multiple lines.'; - terminal.write(input); + terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); for (let i = 0; i < input.length; ++i) { @@ -369,7 +372,7 @@ describe('Buffer', () => { }); it('combining e\u0301 in a sentence', function(): void { const input = 'Sitting in the cafe\u0301 drinking coffee.'; - terminal.write(input); + terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); for (let i = 0; i < 19; ++i) { @@ -388,7 +391,7 @@ describe('Buffer', () => { }); it('multiline combining e\u0301', function(): void { const input = 'e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301'; - terminal.write(input); + terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); // every buffer cell index contains 2 string indices @@ -399,7 +402,7 @@ describe('Buffer', () => { }); it('surrogate char in a sentence', function(): void { const input = 'The 𝄞 is a clef widely used in modern notation.'; - terminal.write(input); + terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); for (let i = 0; i < 5; ++i) { @@ -418,7 +421,7 @@ describe('Buffer', () => { }); it('multiline surrogate char', function(): void { const input = '𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞'; - terminal.write(input); + terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); // every buffer cell index contains 2 string indices @@ -430,7 +433,7 @@ describe('Buffer', () => { it('surrogate char with combining', function(): void { // eye of Ra with acute accent - string length of 3 const input = '𓂀\u0301 - the eye hiroglyph with an acute accent.'; - terminal.write(input); + terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); // index 0..2 should map to 0 @@ -443,7 +446,7 @@ describe('Buffer', () => { }); it('multiline surrogate with combining', function(): void { const input = '𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301'; - terminal.write(input); + terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); // every buffer cell index contains 3 string indices @@ -454,7 +457,7 @@ describe('Buffer', () => { }); it('fullwidth chars', function(): void { const input = 'These 123 are some fat numbers.'; - terminal.write(input); + terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); for (let i = 0; i < 6; ++i) { @@ -472,7 +475,7 @@ describe('Buffer', () => { }); it('multiline fullwidth chars', function(): void { const input = '12345678901234567890'; - terminal.write(input); + terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); for (let i = 9; i < input.length; ++i) { @@ -482,7 +485,7 @@ describe('Buffer', () => { }); it('fullwidth combining with emoji - match emoji cell', function(): void { const input = 'Lots of ¥\u0301 make me 😃.'; - terminal.write(input); + terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; assert.equal(input, s); const stringIndex = s.match(/😃/).index; @@ -496,7 +499,7 @@ describe('Buffer', () => { // the next fullwidth char has to wrap early // the dangling last cell is wrongly added in the string // --> fixable after resolving #1685 - terminal.write(input); + terminal.writeSync(input); // TODO: reenable after fix // const s = terminal.buffer.contents(true).toArray()[0]; // assert.equal(input, s); From cd2ac929c3a1f05d28ca78a01e63b3548042ef45 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 13 Sep 2018 19:44:02 -0700 Subject: [PATCH 14/24] :lipstick: --- src/Buffer.ts | 11 +++++++---- src/Types.ts | 1 - 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 3a26fb79dd..9c4dcf71c2 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -408,14 +408,17 @@ export class BufferStringIterator implements IBufferStringIterator { private _start: number; private _end: number; private _current: number; + constructor (private _buffer: IBuffer, private _trimRight: boolean, startIndex?: number, endIndex?: number) { this._start = startIndex || 0; this._end = endIndex || this._buffer.lines.length; this._current = this._start; } + public hasNext(): boolean { return this._current < this._end; } + public next(withRanges: boolean = false): string | [{first: number, last: number}, string] { const range = this._buffer.getWrappedRangeForLine(this._current); let result = ''; @@ -423,12 +426,12 @@ export class BufferStringIterator implements IBufferStringIterator { // TODO: always apply trimRight after fixing #1685 result += this._buffer.translateBufferLineToString(i, (this._trimRight) ? i === range.last : false); } - this._current = range.last; - this._current++; + this._current = range.last + 1; return (withRanges) ? [range, result] : result; } - toArray(): string[] { - const result: string[] = []; + + public toArray(): string[] { + const result = []; while (this.hasNext()) { result.push(this.next() as string); } diff --git a/src/Types.ts b/src/Types.ts index 5782f2c9ad..5652f13545 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -271,7 +271,6 @@ export interface ITerminalOptions extends IPublicTerminalOptions { export interface IBufferStringIterator { hasNext(): boolean; next(withRanges: boolean): string | [{first: number, last: number}, string]; - toArray(): string[]; } export interface IBuffer { From 50d600877cf2f3c310c7a4c54c3d4f569db9fd35 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 13 Sep 2018 19:54:53 -0700 Subject: [PATCH 15/24] Use common TestTerminal for writeSync across Buffer/Linkifier test --- src/Buffer.test.ts | 48 ++++++++++++++++++++----------------- src/Linkifier.test.ts | 22 +++++++---------- src/Types.ts | 1 + src/utils/TestUtils.test.ts | 8 +++++++ 4 files changed, 44 insertions(+), 35 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 2ad86b22e1..153b6f15eb 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -7,20 +7,12 @@ import { assert } from 'chai'; import { ITerminal } from './Types'; import { Buffer, DEFAULT_ATTR, CHAR_DATA_CHAR_INDEX } from './Buffer'; import { CircularList } from './common/CircularList'; -import { MockTerminal } from './utils/TestUtils.test'; +import { MockTerminal, TestTerminal } from './utils/TestUtils.test'; import { BufferLine } from './BufferLine'; -import { Terminal } from './Terminal'; const INIT_COLS = 80; const INIT_ROWS = 24; -class TestTerminal extends Terminal { - writeSync(data: string): void { - this.writeBuffer.push(data); - this._innerWrite(); - } -} - describe('Buffer', () => { let terminal: ITerminal; let buffer: Buffer; @@ -355,12 +347,14 @@ describe('Buffer', () => { assert.equal(str3, '😁a'); }); }); - describe('stringIndexToBufferIndex', function(): void { + describe('stringIndexToBufferIndex', () => { let terminal: TestTerminal; - beforeEach(function(): void { + + beforeEach(() => { terminal = new TestTerminal({rows: 5, cols: 10}); }); - it('multiline ascii', function(): void { + + it('multiline ascii', () => { const input = 'This is ASCII text spanning multiple lines.'; terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; @@ -370,7 +364,8 @@ describe('Buffer', () => { assert.deepEqual([(i / terminal.cols) | 0, i % terminal.cols], bufferIndex); } }); - it('combining e\u0301 in a sentence', function(): void { + + it('combining e\u0301 in a sentence', () => { const input = 'Sitting in the cafe\u0301 drinking coffee.'; terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; @@ -389,7 +384,8 @@ describe('Buffer', () => { assert.deepEqual([((i - 1) / terminal.cols) | 0, (i - 1) % terminal.cols], bufferIndex); } }); - it('multiline combining e\u0301', function(): void { + + it('multiline combining e\u0301', () => { const input = 'e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301'; terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; @@ -400,7 +396,8 @@ describe('Buffer', () => { assert.deepEqual([((i >> 1) / terminal.cols) | 0, (i >> 1) % terminal.cols], bufferIndex); } }); - it('surrogate char in a sentence', function(): void { + + it('surrogate char in a sentence', () => { const input = 'The 𝄞 is a clef widely used in modern notation.'; terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; @@ -419,7 +416,8 @@ describe('Buffer', () => { assert.deepEqual([((i - 1) / terminal.cols) | 0, (i - 1) % terminal.cols], bufferIndex); } }); - it('multiline surrogate char', function(): void { + + it('multiline surrogate char', () => { const input = '𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞'; terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; @@ -430,7 +428,8 @@ describe('Buffer', () => { assert.deepEqual([((i >> 1) / terminal.cols) | 0, (i >> 1) % terminal.cols], bufferIndex); } }); - it('surrogate char with combining', function(): void { + + it('surrogate char with combining', () => { // eye of Ra with acute accent - string length of 3 const input = '𓂀\u0301 - the eye hiroglyph with an acute accent.'; terminal.writeSync(input); @@ -444,7 +443,8 @@ describe('Buffer', () => { assert.deepEqual([((i - 2) / terminal.cols) | 0, (i - 2) % terminal.cols], bufferIndex); } }); - it('multiline surrogate with combining', function(): void { + + it('multiline surrogate with combining', () => { const input = '𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301'; terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; @@ -455,7 +455,8 @@ describe('Buffer', () => { assert.deepEqual([(((i / 3) | 0) / terminal.cols) | 0, ((i / 3) | 0) % terminal.cols], bufferIndex); } }); - it('fullwidth chars', function(): void { + + it('fullwidth chars', () => { const input = 'These 123 are some fat numbers.'; terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; @@ -473,7 +474,8 @@ describe('Buffer', () => { assert.deepEqual([((i + 3) / terminal.cols) | 0, (i + 3) % terminal.cols], bufferIndex); } }); - it('multiline fullwidth chars', function(): void { + + it('multiline fullwidth chars', () => { const input = '12345678901234567890'; terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; @@ -483,7 +485,8 @@ describe('Buffer', () => { assert.deepEqual([((i << 1) / terminal.cols) | 0, (i << 1) % terminal.cols], bufferIndex); } }); - it('fullwidth combining with emoji - match emoji cell', function(): void { + + it('fullwidth combining with emoji - match emoji cell', () => { const input = 'Lots of ¥\u0301 make me 😃.'; terminal.writeSync(input); const s = terminal.buffer.contents(true).toArray()[0]; @@ -492,7 +495,8 @@ describe('Buffer', () => { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, stringIndex); 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)', function(): void { + + it('multiline fullwidth chars with offset 1 (currently tests for broken behavior)', () => { const input = 'a12345678901234567890'; // 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 diff --git a/src/Linkifier.test.ts b/src/Linkifier.test.ts index 32b4ee3be9..35610acb01 100644 --- a/src/Linkifier.test.ts +++ b/src/Linkifier.test.ts @@ -7,10 +7,9 @@ import { assert } from 'chai'; import { IMouseZoneManager, IMouseZone } from './ui/Types'; import { ILinkMatcher, ITerminal, IBufferLine } from './Types'; import { Linkifier } from './Linkifier'; -import { MockBuffer, MockTerminal } from './utils/TestUtils.test'; +import { MockBuffer, MockTerminal, TestTerminal } from './utils/TestUtils.test'; import { CircularList } from './common/CircularList'; import { BufferLine } from './BufferLine'; -import { Terminal } from './Terminal'; class TestLinkifier extends Linkifier { constructor(terminal: ITerminal) { @@ -239,23 +238,20 @@ describe('Linkifier', () => { }); }); }); - describe('unicode handling', function(): void { + 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(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(); - }; + beforeEach(() => { + terminal = new TestTerminal({cols: 10, rows: 5}); 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); + terminal.writeSync(rowText); linkifier.registerLinkMatcher(linkMatcherRegex, () => {}); linkifier.linkifyRows(); // Allow linkify to happen @@ -271,7 +267,7 @@ describe('Linkifier', () => { }, 0); } - describe('unicode before the match', function(): void { + 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); }); @@ -303,7 +299,7 @@ describe('Linkifier', () => { assertLinkifiesInTerminal('¥\u0301¥\u0301 foo', /foo/, [{x1: 8, x2: 1, y1: 0, y2: 1}], done); }); }); - describe('unicode within the match', function(): void { + 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); }); diff --git a/src/Types.ts b/src/Types.ts index 5652f13545..5782f2c9ad 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -271,6 +271,7 @@ export interface ITerminalOptions extends IPublicTerminalOptions { export interface IBufferStringIterator { hasNext(): boolean; next(withRanges: boolean): string | [{first: number, last: number}, string]; + toArray(): string[]; } export interface IBuffer { diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index 6fa0c87a3b..40a93ded41 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -9,6 +9,14 @@ import { ICircularList, XtermListener } from '../common/Types'; import { Buffer } from '../Buffer'; import * as Browser from '../shared/utils/Browser'; import { ITheme, IDisposable, IMarker } from 'xterm'; +import { Terminal } from '../Terminal'; + +export class TestTerminal extends Terminal { + writeSync(data: string): void { + this.writeBuffer.push(data); + this._innerWrite(); + } +} export class MockTerminal implements ITerminal { markers: IMarker[]; From 9ce28943536c77199d8af7e1d252291c980790e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 14 Sep 2018 12:59:56 +0200 Subject: [PATCH 16/24] remove toArray; interface type for iterator result --- src/Buffer.test.ts | 20 ++++++++++---------- src/Buffer.ts | 14 +++----------- src/Linkifier.ts | 6 +++--- src/Types.ts | 8 ++++++-- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 153b6f15eb..1157770761 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -357,7 +357,7 @@ describe('Buffer', () => { it('multiline ascii', () => { const input = 'This is ASCII text spanning multiple lines.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).toArray()[0]; + const s = terminal.buffer.contents(true).next().content; assert.equal(input, s); for (let i = 0; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); @@ -368,7 +368,7 @@ describe('Buffer', () => { it('combining e\u0301 in a sentence', () => { const input = 'Sitting in the cafe\u0301 drinking coffee.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).toArray()[0]; + const s = terminal.buffer.contents(true).next().content; assert.equal(input, s); for (let i = 0; i < 19; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); @@ -388,7 +388,7 @@ describe('Buffer', () => { it('multiline combining e\u0301', () => { const input = 'e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).toArray()[0]; + const s = terminal.buffer.contents(true).next().content; assert.equal(input, s); // every buffer cell index contains 2 string indices for (let i = 0; i < input.length; ++i) { @@ -400,7 +400,7 @@ describe('Buffer', () => { it('surrogate char in a sentence', () => { const input = 'The 𝄞 is a clef widely used in modern notation.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).toArray()[0]; + const s = terminal.buffer.contents(true).next().content; assert.equal(input, s); for (let i = 0; i < 5; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); @@ -420,7 +420,7 @@ describe('Buffer', () => { it('multiline surrogate char', () => { const input = '𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).toArray()[0]; + const s = terminal.buffer.contents(true).next().content; assert.equal(input, s); // every buffer cell index contains 2 string indices for (let i = 0; i < input.length; ++i) { @@ -433,7 +433,7 @@ describe('Buffer', () => { // eye of Ra with acute accent - string length of 3 const input = '𓂀\u0301 - the eye hiroglyph with an acute accent.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).toArray()[0]; + const s = terminal.buffer.contents(true).next().content; assert.equal(input, s); // index 0..2 should map to 0 assert.deepEqual([0, 0], terminal.buffer.stringIndexToBufferIndex(0, 1)); @@ -447,7 +447,7 @@ describe('Buffer', () => { it('multiline surrogate with combining', () => { const input = '𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).toArray()[0]; + const s = terminal.buffer.contents(true).next().content; assert.equal(input, s); // every buffer cell index contains 3 string indices for (let i = 0; i < input.length; ++i) { @@ -459,7 +459,7 @@ describe('Buffer', () => { it('fullwidth chars', () => { const input = 'These 123 are some fat numbers.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).toArray()[0]; + const s = terminal.buffer.contents(true).next().content; assert.equal(input, s); for (let i = 0; i < 6; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); @@ -478,7 +478,7 @@ describe('Buffer', () => { it('multiline fullwidth chars', () => { const input = '12345678901234567890'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).toArray()[0]; + const s = terminal.buffer.contents(true).next().content; assert.equal(input, s); for (let i = 9; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); @@ -489,7 +489,7 @@ describe('Buffer', () => { it('fullwidth combining with emoji - match emoji cell', () => { const input = 'Lots of ¥\u0301 make me 😃.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).toArray()[0]; + const s = terminal.buffer.contents(true).next().content; assert.equal(input, s); const stringIndex = s.match(/😃/).index; const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, stringIndex); diff --git a/src/Buffer.ts b/src/Buffer.ts index 9c4dcf71c2..4413b0a73a 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -4,7 +4,7 @@ */ import { CircularList } from './common/CircularList'; -import { CharData, ITerminal, IBuffer, IBufferLine, BufferIndex, IBufferStringIterator } from './Types'; +import { CharData, ITerminal, IBuffer, IBufferLine, BufferIndex, IBufferStringIterator, IBufferStringIteratorResult } from './Types'; import { EventEmitter } from './common/EventEmitter'; import { IMarker } from 'xterm'; import { BufferLine } from './BufferLine'; @@ -419,7 +419,7 @@ export class BufferStringIterator implements IBufferStringIterator { return this._current < this._end; } - public next(withRanges: boolean = false): string | [{first: number, last: number}, string] { + public next(): IBufferStringIteratorResult { const range = this._buffer.getWrappedRangeForLine(this._current); let result = ''; for (let i = range.first; i <= range.last; ++i) { @@ -427,14 +427,6 @@ export class BufferStringIterator implements IBufferStringIterator { result += this._buffer.translateBufferLineToString(i, (this._trimRight) ? i === range.last : false); } this._current = range.last + 1; - return (withRanges) ? [range, result] : result; - } - - public toArray(): string[] { - const result = []; - while (this.hasNext()) { - result.push(this.next() as string); - } - return result; + return {range: range, content: result}; } } diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 9ff8b823e8..cd5cba96ae 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -4,7 +4,7 @@ */ import { IMouseZoneManager } from './ui/Types'; -import { ILinkHoverEvent, ILinkMatcher, LinkMatcherHandler, LinkHoverEventTypes, ILinkMatcherOptions, ILinkifier, ITerminal } from './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'; @@ -93,9 +93,9 @@ export class Linkifier extends EventEmitter implements ILinkifier { // we skip those later in _doLinkifyRow const linesIterator = this._terminal.buffer.contents(false, absoluteRowIndexStart, this._terminal.buffer.ydisp + this._rowsToLinkify.end + 1); while (linesIterator.hasNext()) { - const lineData: any = linesIterator.next(true); + const lineData: IBufferStringIteratorResult = linesIterator.next(); for (let i = 0; i < this._linkMatchers.length; i++) { - this._doLinkifyRow(lineData[0].first, lineData[1], this._linkMatchers[i]); + this._doLinkifyRow(lineData.range.first, lineData.content, this._linkMatchers[i]); } } diff --git a/src/Types.ts b/src/Types.ts index 5782f2c9ad..063eba1743 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -268,10 +268,14 @@ export interface ITerminalOptions extends IPublicTerminalOptions { useFlowControl?: boolean; } +export interface IBufferStringIteratorResult { + range: {first: number, last: number}; + content: string; +} + export interface IBufferStringIterator { hasNext(): boolean; - next(withRanges: boolean): string | [{first: number, last: number}, string]; - toArray(): string[]; + next(): IBufferStringIteratorResult; } export interface IBuffer { From 9514bfd31c0cff23968d6370cb1da2a32770ab71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 14 Sep 2018 13:15:35 +0200 Subject: [PATCH 17/24] cleanup BufferStringIterator --- src/Buffer.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 4413b0a73a..6d8952dc0b 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -405,18 +405,19 @@ export class Marker extends EventEmitter implements IMarker { } export class BufferStringIterator implements IBufferStringIterator { - private _start: number; - private _end: number; private _current: number; - constructor (private _buffer: IBuffer, private _trimRight: boolean, startIndex?: number, endIndex?: number) { - this._start = startIndex || 0; - this._end = endIndex || this._buffer.lines.length; - this._current = this._start; + constructor ( + private _buffer: IBuffer, + private _trimRight: boolean, + private _startIndex: number = 0, + private _endIndex: number = _buffer.lines.length) + { + this._current = this._startIndex; } public hasNext(): boolean { - return this._current < this._end; + return this._current < this._endIndex; } public next(): IBufferStringIteratorResult { From 8664f54a7634a1673dfcf38dd666070112f0e2b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 14 Sep 2018 13:22:02 +0200 Subject: [PATCH 18/24] rename contents to iterator --- src/Buffer.test.ts | 20 ++++++++++---------- src/Buffer.ts | 2 +- src/Linkifier.ts | 6 +++--- src/Types.ts | 2 +- src/utils/TestUtils.test.ts | 4 ++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 1157770761..32492ee0a5 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -357,7 +357,7 @@ describe('Buffer', () => { it('multiline ascii', () => { const input = 'This is ASCII text spanning multiple lines.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).next().content; + const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); for (let i = 0; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); @@ -368,7 +368,7 @@ describe('Buffer', () => { it('combining e\u0301 in a sentence', () => { const input = 'Sitting in the cafe\u0301 drinking coffee.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).next().content; + const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); for (let i = 0; i < 19; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); @@ -388,7 +388,7 @@ describe('Buffer', () => { it('multiline combining e\u0301', () => { const input = 'e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).next().content; + const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); // every buffer cell index contains 2 string indices for (let i = 0; i < input.length; ++i) { @@ -400,7 +400,7 @@ describe('Buffer', () => { it('surrogate char in a sentence', () => { const input = 'The 𝄞 is a clef widely used in modern notation.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).next().content; + const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); for (let i = 0; i < 5; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); @@ -420,7 +420,7 @@ describe('Buffer', () => { it('multiline surrogate char', () => { const input = '𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).next().content; + const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); // every buffer cell index contains 2 string indices for (let i = 0; i < input.length; ++i) { @@ -433,7 +433,7 @@ describe('Buffer', () => { // eye of Ra with acute accent - string length of 3 const input = '𓂀\u0301 - the eye hiroglyph with an acute accent.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).next().content; + const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); // index 0..2 should map to 0 assert.deepEqual([0, 0], terminal.buffer.stringIndexToBufferIndex(0, 1)); @@ -447,7 +447,7 @@ describe('Buffer', () => { it('multiline surrogate with combining', () => { const input = '𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).next().content; + const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); // every buffer cell index contains 3 string indices for (let i = 0; i < input.length; ++i) { @@ -459,7 +459,7 @@ describe('Buffer', () => { it('fullwidth chars', () => { const input = 'These 123 are some fat numbers.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).next().content; + const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); for (let i = 0; i < 6; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); @@ -478,7 +478,7 @@ describe('Buffer', () => { it('multiline fullwidth chars', () => { const input = '12345678901234567890'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).next().content; + const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); for (let i = 9; i < input.length; ++i) { const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i); @@ -489,7 +489,7 @@ describe('Buffer', () => { it('fullwidth combining with emoji - match emoji cell', () => { const input = 'Lots of ¥\u0301 make me 😃.'; terminal.writeSync(input); - const s = terminal.buffer.contents(true).next().content; + const s = terminal.buffer.iterator(true).next().content; assert.equal(input, s); const stringIndex = s.match(/😃/).index; const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, stringIndex); diff --git a/src/Buffer.ts b/src/Buffer.ts index 6d8952dc0b..383a1f9087 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -374,7 +374,7 @@ export class Buffer implements IBuffer { this.markers.splice(this.markers.indexOf(marker), 1); } - public contents(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator { + public iterator(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator { return new BufferStringIterator(this, trimRight, startIndex, endIndex); } } diff --git a/src/Linkifier.ts b/src/Linkifier.ts index cd5cba96ae..728cb0c23d 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -91,9 +91,9 @@ export class Linkifier extends EventEmitter implements ILinkifier { // _doLinkifyRow gets full unwrapped lines with the start row as buffer offset for every matcher // for wrapped content over several rows the iterator might return rows outside the viewport // we skip those later in _doLinkifyRow - const linesIterator = this._terminal.buffer.contents(false, absoluteRowIndexStart, this._terminal.buffer.ydisp + this._rowsToLinkify.end + 1); - while (linesIterator.hasNext()) { - const lineData: IBufferStringIteratorResult = linesIterator.next(); + const iterator = this._terminal.buffer.iterator(false, absoluteRowIndexStart, this._terminal.buffer.ydisp + this._rowsToLinkify.end + 1); + while (iterator.hasNext()) { + const lineData: IBufferStringIteratorResult = iterator.next(); for (let i = 0; i < this._linkMatchers.length; i++) { this._doLinkifyRow(lineData.range.first, lineData.content, this._linkMatchers[i]); } diff --git a/src/Types.ts b/src/Types.ts index 063eba1743..69c4015143 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -296,7 +296,7 @@ export interface IBuffer { nextStop(x?: number): number; prevStop(x?: number): number; stringIndexToBufferIndex(lineIndex: number, stringIndex: number): number[]; - contents(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator; + iterator(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator; } export interface IBufferSet extends IEventEmitter { diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index 40a93ded41..b38f4b858f 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -322,8 +322,8 @@ export class MockBuffer implements IBuffer { stringIndexToBufferIndex(lineIndex: number, stringIndex: number): number[] { return Buffer.prototype.stringIndexToBufferIndex.apply(this, arguments); } - contents(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator { - return Buffer.prototype.contents.apply(this, arguments); + iterator(trimRight: boolean, startIndex?: number, endIndex?: number): IBufferStringIterator { + return Buffer.prototype.iterator.apply(this, arguments); } } From 57bbbda7e816c4aed03317047ebcaf48c8776112 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 14 Sep 2018 14:15:27 +0200 Subject: [PATCH 19/24] rename stringWidth; add some comments --- src/CharWidth.ts | 3 +-- src/Linkifier.ts | 16 +++++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/CharWidth.ts b/src/CharWidth.ts index 26aaba0234..d54e939242 100644 --- a/src/CharWidth.ts +++ b/src/CharWidth.ts @@ -172,9 +172,8 @@ export const wcwidth = (function(opts: {nul: number, control: number}): (ucs: nu /** * Get the terminal cell width for a string. - * @param s */ -export function stringWidth(s: string): number { +export function getStringCellWidth(s: string): number { let result = 0; for (let i = 0; i < s.length; ++i) { let code = s.charCodeAt(i); diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 728cb0c23d..88c848b833 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -8,7 +8,7 @@ import { ILinkHoverEvent, ILinkMatcher, LinkMatcherHandler, LinkHoverEventTypes, import { MouseZone } from './ui/MouseZoneManager'; import { EventEmitter } from './common/EventEmitter'; import { CHAR_DATA_ATTR_INDEX } from './Buffer'; -import { stringWidth } from './CharWidth'; +import { getStringCellWidth } from './CharWidth'; /** * The Linkifier applies links to rows shortly after they have been refreshed. @@ -82,12 +82,13 @@ export class Linkifier extends EventEmitter implements ILinkifier { private _linkifyRows(): void { this._rowsTimeoutId = null; + // Ensure the row exists 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 + // iterate over the range of unwrapped content strings within start..end (excluding) // _doLinkifyRow gets full unwrapped lines with the start row as buffer offset for every matcher // for wrapped content over several rows the iterator might return rows outside the viewport // we skip those later in _doLinkifyRow @@ -189,8 +190,9 @@ export class Linkifier extends EventEmitter implements ILinkifier { break; } - // 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? + // Get index, match.index is for the outer match which includes negated chars + // therefore we cannot use match.index directly, instead we search the position + // of the match group in text again // also correct regex and string search offsets for the next loop run stringIndex = text.indexOf(uri, stringIndex + 1); rex.lastIndex = stringIndex + uri.length; @@ -239,11 +241,11 @@ export class Linkifier extends EventEmitter implements ILinkifier { * @param fg The link color for hover event. */ private _addLink(x: number, y: number, uri: string, matcher: ILinkMatcher, fg: number): void { - const length = stringWidth(uri); + const width = getStringCellWidth(uri); const x1 = x % this._terminal.cols; const y1 = y + Math.floor(x / this._terminal.cols); - let x2 = (x1 + length) % this._terminal.cols; - let y2 = y1 + Math.floor((x1 + length) / this._terminal.cols); + let x2 = (x1 + width) % this._terminal.cols; + let y2 = y1 + Math.floor((x1 + width) / this._terminal.cols); if (x2 === 0) { x2 = this._terminal.cols; y2--; From 14c20c5f32f7076f1b369752807a403d4a3955f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 14 Sep 2018 14:19:25 +0200 Subject: [PATCH 20/24] simplify stringIndexToBufferIndex, fix docs --- src/Buffer.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 383a1f9087..d1028d0985 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -199,7 +199,7 @@ export class Buffer implements IBuffer { * To get the correct buffer position the string must start at `startCol` 0 * (default in translateBufferLineToString). * The method also works on wrapped line strings given rows were not trimmed. - * The method operates on the CharData width attribute, there are no + * 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 @@ -208,9 +208,6 @@ export class Buffer implements IBuffer { * @param startCol column offset the string was retrieved from */ public stringIndexToBufferIndex(lineIndex: number, stringIndex: number): BufferIndex { - if (!stringIndex) { - return [lineIndex, 0]; - } while (stringIndex) { const line = this.lines.get(lineIndex); if (!line) { From 70be55df7558f5cd3ee7b883fd1680f18d96e9f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 14 Sep 2018 14:46:57 +0200 Subject: [PATCH 21/24] test cases for getStringCellWidth --- src/CharWidth.test.ts | 79 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 src/CharWidth.test.ts diff --git a/src/CharWidth.test.ts b/src/CharWidth.test.ts new file mode 100644 index 0000000000..d4320ab484 --- /dev/null +++ b/src/CharWidth.test.ts @@ -0,0 +1,79 @@ +/** + * Copyright (c) 2017 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { TestTerminal } from './utils/TestUtils.test'; +import { assert } from 'chai'; +import { getStringCellWidth } from './CharWidth'; +import { IBuffer } from './Types'; +import { CHAR_DATA_WIDTH_INDEX, CHAR_DATA_CHAR_INDEX } from './Buffer'; + + +describe('getStringCellWidth', function(): void { + let terminal: TestTerminal; + + beforeEach(() => { + terminal = new TestTerminal({rows: 5, cols: 30}); + }); + + function sumWidths(buffer: IBuffer, start: number, end: number, sentinel: string): number { + let result = 0; + for (let i = start; i < end; ++i) { + const line = buffer.lines.get(i); + for (let j = 0; j < line.length; ++i) { // TODO: change to trimBorder with multiline + const ch = line.get(i); + result += ch[CHAR_DATA_WIDTH_INDEX]; + // return on sentinel + if (ch[CHAR_DATA_CHAR_INDEX] === sentinel) { + return result; + } + } + } + return result; + } + + it('ASCII chars', function(): void { + const input = 'This is just ASCII text.#'; + terminal.writeSync(input); + const s = terminal.buffer.iterator(true).next().content; + assert.equal(input, s); + assert.equal(getStringCellWidth(s), sumWidths(terminal.buffer, 0, 1, '#')); + }); + it('combining chars', function(): void { + const input = 'e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301e\u0301#'; + terminal.writeSync(input); + const s = terminal.buffer.iterator(true).next().content; + assert.equal(input, s); + assert.equal(getStringCellWidth(s), sumWidths(terminal.buffer, 0, 1, '#')); + }); + it('surrogate chars', function(): void { + const input = '𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞𝄞#'; + terminal.writeSync(input); + const s = terminal.buffer.iterator(true).next().content; + assert.equal(input, s); + assert.equal(getStringCellWidth(s), sumWidths(terminal.buffer, 0, 1, '#')); + }); + it('surrogate combining chars', function(): void { + const input = '𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301𓂀\u0301#'; + terminal.writeSync(input); + const s = terminal.buffer.iterator(true).next().content; + assert.equal(input, s); + assert.equal(getStringCellWidth(s), sumWidths(terminal.buffer, 0, 1, '#')); + }); + it('fullwidth chars', function(): void { + const input = '1234567890#'; + terminal.writeSync(input); + const s = terminal.buffer.iterator(true).next().content; + assert.equal(input, s); + assert.equal(getStringCellWidth(s), sumWidths(terminal.buffer, 0, 1, '#')); + }); + it('fullwidth chars offset 1', function(): void { + const input = 'a1234567890#'; + terminal.writeSync(input); + const s = terminal.buffer.iterator(true).next().content; + assert.equal(input, s); + assert.equal(getStringCellWidth(s), sumWidths(terminal.buffer, 0, 1, '#')); + }); + // TODO: multiline tests once #1685 is resolved +}); From 631e331077886fb03478fbdb5b67faabe2283fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 14 Sep 2018 14:48:41 +0200 Subject: [PATCH 22/24] fix test cases --- src/CharWidth.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CharWidth.test.ts b/src/CharWidth.test.ts index d4320ab484..3242dd7732 100644 --- a/src/CharWidth.test.ts +++ b/src/CharWidth.test.ts @@ -21,8 +21,8 @@ describe('getStringCellWidth', function(): void { let result = 0; for (let i = start; i < end; ++i) { const line = buffer.lines.get(i); - for (let j = 0; j < line.length; ++i) { // TODO: change to trimBorder with multiline - const ch = line.get(i); + for (let j = 0; j < line.length; ++j) { // TODO: change to trimBorder with multiline + const ch = line.get(j); result += ch[CHAR_DATA_WIDTH_INDEX]; // return on sentinel if (ch[CHAR_DATA_CHAR_INDEX] === sentinel) { From 4b68976dfb7722c2161f8492b29431e1ee73c51c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 14 Sep 2018 15:43:29 +0200 Subject: [PATCH 23/24] hide regex error behind debug flag --- src/Linkifier.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 88c848b833..001247fc7c 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -186,7 +186,11 @@ export class Linkifier extends EventEmitter implements ILinkifier { if (!uri) { // something matched but does not comply with the given matchIndex // since this is most likely a bug the regex itself we simply do nothing here - // TODO: should this be logged for debugging? + // DEBUG: print match and throw + if ((this._terminal).debug) { + console.log({match, matcher}); + throw new Error('match found without corresponding matchIndex'); + } break; } From 2f66ec452c08118ed4e26fd2313a3f59f1ee9e84 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 14 Sep 2018 08:35:14 -0700 Subject: [PATCH 24/24] :lipstick: --- src/Buffer.ts | 4 ++-- src/Linkifier.ts | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index d1028d0985..29b034ac6c 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -408,8 +408,8 @@ export class BufferStringIterator implements IBufferStringIterator { private _buffer: IBuffer, private _trimRight: boolean, private _startIndex: number = 0, - private _endIndex: number = _buffer.lines.length) - { + private _endIndex: number = _buffer.lines.length + ) { this._current = this._startIndex; } diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 001247fc7c..4d84b1a793 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -255,7 +255,6 @@ export class Linkifier extends EventEmitter implements ILinkifier { y2--; } - this._mouseZoneManager.add(new MouseZone( x1 + 1, y1 + 1,