From 49b76baf3fbbebb0d4a5ecadf5b346328d825acb Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 26 Nov 2016 16:57:15 -0800 Subject: [PATCH 01/15] Add CircularList and basic tests Part of #361 --- src/utils/CircularList.test.ts | 81 +++++++++++++++++++++++++++++ src/utils/CircularList.ts | 95 ++++++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100644 src/utils/CircularList.test.ts create mode 100644 src/utils/CircularList.ts diff --git a/src/utils/CircularList.test.ts b/src/utils/CircularList.test.ts new file mode 100644 index 0000000..4a89aa3 --- /dev/null +++ b/src/utils/CircularList.test.ts @@ -0,0 +1,81 @@ +import { assert } from 'chai'; +import { CircularList } from './CircularList'; + +describe('CircularList', () => { + describe('push', () => { + it('should push values onto the array', () => { + const list = new CircularList(5); + list.push('1'); + list.push('2'); + list.push('3'); + list.push('4'); + list.push('5'); + assert.equal(list.get(0), '1'); + assert.equal(list.get(1), '2'); + assert.equal(list.get(2), '3'); + assert.equal(list.get(3), '4'); + assert.equal(list.get(4), '5'); + }); + + it('should push old values from the start out of the array when max length is reached', () => { + const list = new CircularList(2); + list.push('1'); + list.push('2'); + assert.equal(list.get(0), '1'); + assert.equal(list.get(1), '2'); + list.push('3'); + assert.equal(list.get(0), '2'); + assert.equal(list.get(1), '3'); + list.push('4'); + assert.equal(list.get(0), '3'); + assert.equal(list.get(1), '4'); + }); + }); + + describe('maxLength', () => { + it('should increase the size of the list', () => { + const list = new CircularList(2); + list.push('1'); + list.push('2'); + assert.equal(list.get(0), '1'); + assert.equal(list.get(1), '2'); + list.maxLength = 4; + list.push('3'); + list.push('4'); + assert.equal(list.get(0), '1'); + assert.equal(list.get(1), '2'); + assert.equal(list.get(2), '3'); + assert.equal(list.get(3), '4'); + list.push('wrapped'); + assert.equal(list.get(0), '2'); + assert.equal(list.get(1), '3'); + assert.equal(list.get(2), '4'); + assert.equal(list.get(3), 'wrapped'); + }); + + it('should return the maximum length of the list', () => { + const list = new CircularList(2); + assert.equal(list.maxLength, 2); + list.push('1'); + list.push('2'); + assert.equal(list.maxLength, 2); + list.push('3'); + assert.equal(list.maxLength, 2); + list.maxLength = 4; + assert.equal(list.maxLength, 4); + }); + }); + + describe('length', () => { + it('should return the current length of the list, capped at the maximum length', () => { + const list = new CircularList(2); + assert.equal(list.length, 0); + list.push('1'); + assert.equal(list.length, 1); + list.push('2'); + assert.equal(list.length, 2); + list.push('3'); + assert.equal(list.length, 2); + }); + }); +}); diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts new file mode 100644 index 0000000..51da0c6 --- /dev/null +++ b/src/utils/CircularList.ts @@ -0,0 +1,95 @@ +/** + * xterm.js: xterm, in the browser + * Copyright (c) 2016, SourceLair Private Company (MIT License) + */ + +/** + * Represents a circular list; a list with a maximum size that wraps around when push is called, + * overriding values at the start of the list. + */ +export class CircularList { + private _array: T[]; + private _startIndex: number; + private _length: number; + + constructor(maxLength: number) { + this._array = new Array(maxLength); + this._startIndex = 0; + this._length = 0; + } + + public get maxLength(): number { + return this._array.length; + } + + public set maxLength(newMaxLength: number) { + // Reconstruct array, starting at index 0. Only transfer values from the + // indexes 0 to length. + let newArray = new Array(newMaxLength); + for (let i = 0; i < Math.min(newMaxLength, this.length); i++) { + newArray[i] = this._array[this._getCyclicIndex(i)]; + } + this._array = newArray; + this._startIndex = 0; + } + + public get length(): number { + return this._length; + } + + public set length(newLength: number) { + // TODO: Is this auto fill is needed or can it be + if (newLength > this._length) { + for (let i = this._length; i < newLength; i++) { + this._array[i] = undefined; + } + } + this._length = newLength; + } + + public get forEach(): (callbackfn: (value: T, index: number, array: T[]) => void) => void { + return this._array.forEach; + } + + /** + * Gets the value at an index. + * + * Note that for performance reasons there is no bounds checking here, the index reference is + * circular so this should always return a value and never throw. + * @param index The index of the value to get. + * @return The value corresponding to the index. + */ + public get(index: number): T { + return this._array[this._getCyclicIndex(index)]; + } + + /** + * Sets the value at an index. + * + * Note that for performance reasons there is no bounds checking here, the index reference is + * circular so this should always return a value and never throw. + * @param index The index to set. + * @param value The value to set. + */ + public set(index: number, value: T): void { + this._array[this._getCyclicIndex(index)] = value; + } + + /** + * Pushes a new value onto the list, wrapping around to the start of the array, overriding index 0 + * if the maximum length is reached. + * @param value The value to push onto the list. + */ + public push(value: T): void { + this._array[this._getCyclicIndex(this._length)] = value; + if (this._length === this.maxLength) { + this._startIndex++; + } else { + this._length++; + } + } + + private _getCyclicIndex(index: number): number { + return (this._startIndex + index) % this.maxLength; + } +} From 61fbbb0054a868c31f5d94334b6245bb92c53eac Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 27 Nov 2016 02:20:53 -0800 Subject: [PATCH 02/15] Add CiruclarList pop and splice --- src/utils/CircularList.test.ts | 50 ++++++++++++++++++++++++++++++++++ src/utils/CircularList.ts | 29 ++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/src/utils/CircularList.test.ts b/src/utils/CircularList.test.ts index 4a89aa3..3b2520a 100644 --- a/src/utils/CircularList.test.ts +++ b/src/utils/CircularList.test.ts @@ -78,4 +78,54 @@ describe('CircularList', () => { assert.equal(list.length, 2); }); }); + + describe('splice', () => { + it('should delete items', () => { + const list = new CircularList(2); + list.push('1'); + list.push('2'); + list.splice(0, 1); + assert.equal(list.length, 1); + assert.equal(list.get(0), '2'); + list.push('3'); + list.splice(1, 1); + assert.equal(list.length, 1); + assert.equal(list.get(0), '2'); + }); + + it('should insert items', () => { + const list = new CircularList(2); + list.push('1'); + list.splice(0, 0, '2'); + assert.equal(list.length, 2); + assert.equal(list.get(0), '2'); + assert.equal(list.get(1), '1'); + list.splice(1, 0, '3'); + assert.equal(list.length, 2); + assert.equal(list.get(0), '3'); + assert.equal(list.get(1), '1'); + }); + + it('should delete items then insert items', () => { + const list = new CircularList(3); + list.push('1'); + list.push('2'); + list.splice(0, 1, '3', '4'); + assert.equal(list.length, 3); + assert.equal(list.get(0), '3'); + assert.equal(list.get(1), '4'); + assert.equal(list.get(2), '2'); + }); + + it('should wrap the array correctly when more items are inserted than deleted', () => { + const list = new CircularList(3); + list.push('1'); + list.push('2'); + list.splice(1, 0, '3', '4'); + assert.equal(list.length, 3); + assert.equal(list.get(0), '3'); + assert.equal(list.get(1), '4'); + assert.equal(list.get(2), '2'); + }); + }); }); diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts index 51da0c6..a131bcb 100644 --- a/src/utils/CircularList.ts +++ b/src/utils/CircularList.ts @@ -89,6 +89,35 @@ export class CircularList { } } + public pop(): T { + return this._array[this._getCyclicIndex(this._length-- - 1)]; + } + + // TODO: Warn there's no error handling and that this is a slow operation + public splice(start: number, deleteCount: number, ...items: T[]) { + if (deleteCount) { + for (let i = start; i < this._length - deleteCount; i++) { + this._array[this._getCyclicIndex(i)] = this._array[this._getCyclicIndex(i + deleteCount)]; + } + this._length -= deleteCount; + } + if (items && items.length) { + for (let i = this._length - 1; i >= start; i--) { + this._array[this._getCyclicIndex(i + items.length)] = this._array[this._getCyclicIndex(i)]; + } + for (let i = 0; i < items.length; i++) { + this._array[this._getCyclicIndex(start + i)] = items[i]; + } + + if (this._length + items.length > this.maxLength) { + this._startIndex += (this._length + items.length) - this.maxLength; + this._length = this.maxLength; + } else { + this._length += items.length; + } + } + } + private _getCyclicIndex(index: number): number { return (this._startIndex + index) % this.maxLength; } From 607c8191095da14bde0c60c8dae838e1e6d87156 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 27 Nov 2016 03:27:36 -0800 Subject: [PATCH 03/15] Progress --- src/test/escape-sequences-test.js | 10 ++- src/test/test.js | 140 +++++++++++++++--------------- src/utils/CircularList.ts | 5 ++ src/xterm.js | 65 +++++++------- 4 files changed, 115 insertions(+), 105 deletions(-) diff --git a/src/test/escape-sequences-test.js b/src/test/escape-sequences-test.js index 2eb6016..05fb75f 100644 --- a/src/test/escape-sequences-test.js +++ b/src/test/escape-sequences-test.js @@ -59,7 +59,7 @@ function terminalToString(term) { for (var line=0; line { return this._array[this._getCyclicIndex(this._length-- - 1)]; } + public removeItemsFromStart(amount: number): void { + this._startIndex += this._length -amount; + this._length = amount; + } + // TODO: Warn there's no error handling and that this is a slow operation public splice(start: number, deleteCount: number, ...items: T[]) { if (deleteCount) { diff --git a/src/xterm.js b/src/xterm.js index c88423d..7239414 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -35,6 +35,7 @@ import { CompositionHelper } from './CompositionHelper.js'; import { EventEmitter } from './EventEmitter.js'; import { Viewport } from './Viewport.js'; import { rightClickHandler, pasteHandler, copyHandler } from './handlers/Clipboard.js'; +import { CircularList } from './utils/CircularList.js'; import * as Browser from './utils/Browser'; import * as Keyboard from './utils/Keyboard'; @@ -229,7 +230,7 @@ function Terminal(options) { * An array of all lines in the entire buffer, including the prompt. The lines are array of * characters which are 2-length arrays where [0] is an attribute and [1] is the character. */ - this.lines = []; + this.lines = new CircularList(this.scrollback); var i = this.rows; while (i--) { this.lines.push(this.blankLine()); @@ -1096,7 +1097,7 @@ Terminal.prototype.refresh = function(start, end, queue) { for (; y <= end; y++) { row = y + this.ydisp; - line = this.lines[row]; + line = this.lines.get(row); out = ''; if (this.y === y - (this.ybase - this.ydisp) @@ -1252,8 +1253,9 @@ Terminal.prototype.scroll = function() { var row; if (++this.ybase === this.scrollback) { - this.ybase = this.ybase / 2 | 0; - this.lines = this.lines.slice(-(this.ybase + this.rows) + 1); + this.ybase = this.ybase / 2; + // TODO: Rely on the circular list instead of cutting it in half + this.lines.removeItemsFromStart(this.ybase + this.rows - 1); } if (!this.userScrolling) { @@ -1388,7 +1390,6 @@ Terminal.prototype.write = function(data) { // surrogate low - already handled above if (0xDC00 <= code && code <= 0xDFFF) continue; - switch (this.state) { case normal: switch (ch) { @@ -1458,17 +1459,16 @@ Terminal.prototype.write = function(data) { // insert combining char in last cell // FIXME: needs handling after cursor jumps if (!ch_width && this.x) { - // dont overflow left - if (this.lines[row][this.x-1]) { - if (!this.lines[row][this.x-1][2]) { + if (this.lines.get(row)[this.x-1]) { + if (!this.lines.get(row)[this.x-1][2]) { // found empty cell after fullwidth, need to go 2 cells back - if (this.lines[row][this.x-2]) - this.lines[row][this.x-2][1] += ch; + if (this.lines.get(row)[this.x-2]) + this.lines.get(row)[this.x-2][1] += ch; } else { - this.lines[row][this.x-1][1] += ch; + this.lines.get(row)[this.x-1][1] += ch; } this.updateRange(this.y); } @@ -1500,24 +1500,24 @@ Terminal.prototype.write = function(data) { for (var moves=0; moves x) i = this.lines.length; while (i--) { - while (this.lines[i].length > x) { - this.lines[i].pop(); + while (this.lines.get(i).length > x) { + this.lines.get(i).pop(); } } } @@ -3044,7 +3044,7 @@ Terminal.prototype.nextStop = function(x) { * @param {number} y The line in which to operate. */ Terminal.prototype.eraseRight = function(x, y) { - var line = this.lines[this.ybase + y] + var line = this.lines.get(this.ybase + y) , ch = [this.eraseAttr(), ' ', 1]; // xterm @@ -3063,7 +3063,7 @@ Terminal.prototype.eraseRight = function(x, y) { * @param {number} y The line in which to operate. */ Terminal.prototype.eraseLeft = function(x, y) { - var line = this.lines[this.ybase + y] + var line = this.lines.get(this.ybase + y) , ch = [this.eraseAttr(), ' ', 1]; // xterm x++; @@ -3080,7 +3080,8 @@ Terminal.prototype.clear = function() { // Don't clear if it's already clear return; } - this.lines = [this.lines[this.ybase + this.y]]; + this.lines.set(0, this.lines.get(this.ybase + this.y)); + this.lines.length = 1; this.ydisp = 0; this.ybase = 0; this.y = 0; @@ -3655,8 +3656,8 @@ Terminal.prototype.insertChars = function(params) { ch = [this.eraseAttr(), ' ', 1]; // xterm while (param-- && j < this.cols) { - this.lines[row].splice(j++, 0, ch); - this.lines[row].pop(); + this.lines.get(row).splice(j++, 0, ch); + this.lines.get(row).pop(); } }; @@ -3789,7 +3790,7 @@ Terminal.prototype.eraseChars = function(params) { ch = [this.eraseAttr(), ' ', 1]; // xterm while (param-- && j < this.cols) { - this.lines[row][j++] = ch; + this.lines.get(row)[j++] = ch; } }; @@ -4453,7 +4454,7 @@ Terminal.prototype.cursorBackwardTab = function(params) { */ Terminal.prototype.repeatPrecedingCharacter = function(params) { var param = params[0] || 1 - , line = this.lines[this.ybase + this.y] + , line = this.lines.get(this.ybase + this.y) , ch = line[this.x - 1] || [this.defAttr, ' ', 1]; while (param--) line[this.x++] = ch; @@ -4688,7 +4689,7 @@ Terminal.prototype.setAttrInRectangle = function(params) { , i; for (; t < b + 1; t++) { - line = this.lines[this.ybase + t]; + line = this.lines.get(this.ybase + t); for (i = l; i < r; i++) { line[i] = [attr, line[i][1]]; } @@ -4718,7 +4719,7 @@ Terminal.prototype.fillRectangle = function(params) { , i; for (; t < b + 1; t++) { - line = this.lines[this.ybase + t]; + line = this.lines.get(this.ybase + t); for (i = l; i < r; i++) { line[i] = [line[i][0], String.fromCharCode(ch)]; } @@ -4770,7 +4771,7 @@ Terminal.prototype.eraseRectangle = function(params) { ch = [this.eraseAttr(), ' ', 1]; // xterm? for (; t < b + 1; t++) { - line = this.lines[this.ybase + t]; + line = this.lines.get(this.ybase + t); for (i = l; i < r; i++) { line[i] = ch; } From be56c72b5f05426f8e573c4135fa5e2af30fa77d Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 21 Dec 2016 15:43:04 -0800 Subject: [PATCH 04/15] Fix when scrollback limit is reached --- src/utils/CircularList.ts | 5 ----- src/xterm.js | 19 ++++--------------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts index 9065954..a131bcb 100644 --- a/src/utils/CircularList.ts +++ b/src/utils/CircularList.ts @@ -93,11 +93,6 @@ export class CircularList { return this._array[this._getCyclicIndex(this._length-- - 1)]; } - public removeItemsFromStart(amount: number): void { - this._startIndex += this._length -amount; - this._length = amount; - } - // TODO: Warn there's no error handling and that this is a slow operation public splice(start: number, deleteCount: number, ...items: T[]) { if (deleteCount) { diff --git a/src/xterm.js b/src/xterm.js index 813c720..2cf0651 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -1226,15 +1226,13 @@ Terminal.prototype.showCursor = function() { }; /** - * Scroll the terminal + * Scroll the terminal down 1 row, creating a blank line. */ Terminal.prototype.scroll = function() { var row; - if (++this.ybase === this.scrollback) { - this.ybase = this.ybase / 2; - // TODO: Rely on the circular list instead of cutting it in half - this.lines.removeItemsFromStart(this.ybase + this.rows - 1); + if (this.lines.length < this.lines.maxLength) { + this.ybase++; } if (!this.userScrolling) { @@ -1247,16 +1245,7 @@ Terminal.prototype.scroll = function() { // subtract the bottom scroll region row -= this.rows - 1 - this.scrollBottom; - if (row === this.lines.length) { - // potential optimization: - // pushing is faster than splicing - // when they amount to the same - // behavior. - this.lines.push(this.blankLine()); - } else { - // add our new line - this.lines.splice(row, 0, this.blankLine()); - } + this.lines.push(this.blankLine()); if (this.scrollTop !== 0) { if (this.ybase !== 0) { From 3b35d12e603842361fd08a77dcac2646a24af34c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 22 Dec 2016 02:02:03 -0800 Subject: [PATCH 05/15] Fix issue with the git log The old code was assuming that the buffer was not going to change, this is not true with the current impl though where the list is shifted and ybase and ydisp need to be compensated for that. --- src/utils/CircularList.ts | 11 ++++++++++ src/xterm.js | 46 ++++++++++++++++++++++++++++++--------- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts index a131bcb..699de65 100644 --- a/src/utils/CircularList.ts +++ b/src/utils/CircularList.ts @@ -84,12 +84,16 @@ export class CircularList { this._array[this._getCyclicIndex(this._length)] = value; if (this._length === this.maxLength) { this._startIndex++; + if (this._startIndex === this.maxLength) { + this._startIndex = 0; + } } else { this._length++; } } public pop(): T { + // TODO: This isn't popping from the array, only returning return this._array[this._getCyclicIndex(this._length-- - 1)]; } @@ -118,6 +122,13 @@ export class CircularList { } } + public trimStart(count: number): void { + // TODO: Error handling + // TODO: Testing (if we need this) + this._startIndex += count; + this._length--; + } + private _getCyclicIndex(index: number): number { return (this._startIndex + index) % this.maxLength; } diff --git a/src/xterm.js b/src/xterm.js index f07fb4e..dbf03ea 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -1238,6 +1238,7 @@ Terminal.prototype.scroll = function() { this.ybase++; } + // TODO: Why is this done twice? if (!this.userScrolling) { this.ydisp = this.ybase; } @@ -3164,21 +3165,30 @@ Terminal.prototype.index = function() { /** * ESC M Reverse Index (RI is 0x8d). + * + * Move the cursor up one row, inserting a new blank line if necessary. */ Terminal.prototype.reverseIndex = function() { var j; - this.y--; - if (this.y < this.scrollTop) { - this.y++; + if (this.y === this.scrollTop) { // possibly move the code below to term.reverseScroll(); // test: echo -ne '\e[1;1H\e[44m\eM\e[0m' // blankLine(true) is xterm/linux behavior + if (this.lines.length === this.lines.maxLength) { + // Trim the start of lines to make room for the new temporary row + // TODO: This section could be optimized by introducing a CircularList function that inserts, + // deletes and shifts elements to accomplish this task. + this.lines.trimStart(1); + this.ybase -= 1; + this.ydisp -= 1; + } this.lines.splice(this.y + this.ybase, 0, this.blankLine(true)); j = this.rows - 1 - this.scrollBottom; this.lines.splice(this.rows - 1 + this.ybase - j + 1, 1); - // this.maxRange(); this.updateRange(this.scrollTop); this.updateRange(this.scrollBottom); + } else { + this.y--; } this.state = normal; }; @@ -3695,6 +3705,14 @@ Terminal.prototype.insertLines = function(params) { j = this.rows - 1 + this.ybase - j + 1; while (param--) { + if (this.lines.length === this.lines.maxLength) { + // Trim the start of lines to make room for the new temporary row + // TODO: This section could be optimized by introducing a CircularList function that inserts, + // deletes and shifts elements to accomplish this task. + this.lines.trimStart(1); + this.ybase -= 1; + this.ydisp -= 1; + } // test: echo -e '\e[44m\e[1L\e[0m' // blankLine(true) - xterm/linux behavior this.lines.splice(row, 0, this.blankLine(true)); @@ -3722,6 +3740,14 @@ Terminal.prototype.deleteLines = function(params) { j = this.rows - 1 + this.ybase - j; while (param--) { + if (this.lines.length === this.lines.maxLength) { + // Trim the start of lines to make room for the new temporary row + // TODO: This section could be optimized by introducing a CircularList function that inserts, + // deletes and shifts elements to accomplish this task. + this.lines.trimStart(1); + this.ybase -= 1; + this.ydisp -= 1; + } // test: echo -e '\e[44m\e[1M\e[0m' // blankLine(true) - xterm/linux behavior this.lines.splice(j + 1, 0, this.blankLine(true)); @@ -3748,8 +3774,8 @@ Terminal.prototype.deleteChars = function(params) { ch = [this.eraseAttr(), ' ', 1]; // xterm while (param--) { - this.lines[row].splice(this.x, 1); - this.lines[row].push(ch); + this.lines.get(row).splice(this.x, 1); + this.lines.get(row).push(ch); } }; @@ -4774,8 +4800,8 @@ Terminal.prototype.insertColumns = function() { while (param--) { for (i = this.ybase; i < l; i++) { - this.lines[i].splice(this.x + 1, 0, ch); - this.lines[i].pop(); + this.lines.get(i).splice(this.x + 1, 0, ch); + this.lines.get(i).pop(); } } @@ -4796,8 +4822,8 @@ Terminal.prototype.deleteColumns = function() { while (param--) { for (i = this.ybase; i < l; i++) { - this.lines[i].splice(this.x, 1); - this.lines[i].push(ch); + this.lines.get(i).splice(this.x, 1); + this.lines.get(i).push(ch); } } From cc0fc953cf2bb92978d3c35875313ddd56f12937 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 22 Dec 2016 02:04:31 -0800 Subject: [PATCH 06/15] Remove logs --- src/test/escape-sequences-test.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/test/escape-sequences-test.js b/src/test/escape-sequences-test.js index 05fb75f..ace2c3c 100644 --- a/src/test/escape-sequences-test.js +++ b/src/test/escape-sequences-test.js @@ -87,21 +87,16 @@ describe('xterm output comparison', function() { var i = successful[a]; (function(filename){ it(filename.split('/').slice(-1)[0], function () { - console.log(filename); pty_reset(); - console.log(1); var in_file = fs.readFileSync(filename, 'utf8'); var from_pty = pty_write_read(in_file); - console.log(2); // uncomment this to get log from terminal //console.log = function(){}; xterm.write(from_pty); var from_emulator = terminalToString(xterm); - console.log(3); console.log = CONSOLE_LOG; var expected = fs.readFileSync(filename.split('.')[0] + '.text', 'utf8'); if (from_emulator != expected) { - console.log(4); // uncomment to get noisy output //throw new Error(formatError(in_file, from_emulator, expected)); throw new Error('mismatch'); From be2e4b5d858ac89f8b00fe8dfd9b11b12645fb1b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 21 Dec 2016 02:11:14 -0800 Subject: [PATCH 07/15] Add missing return type --- src/utils/CircularList.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts index 699de65..ce79e35 100644 --- a/src/utils/CircularList.ts +++ b/src/utils/CircularList.ts @@ -98,7 +98,7 @@ export class CircularList { } // TODO: Warn there's no error handling and that this is a slow operation - public splice(start: number, deleteCount: number, ...items: T[]) { + public splice(start: number, deleteCount: number, ...items: T[]): void { if (deleteCount) { for (let i = start; i < this._length - deleteCount; i++) { this._array[this._getCyclicIndex(i)] = this._array[this._getCyclicIndex(i + deleteCount)]; From bfaf9cd6ebc48d931162774ae3b12703af963b72 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 21 Dec 2016 02:39:21 -0800 Subject: [PATCH 08/15] Doc and testing --- src/utils/CircularList.test.ts | 28 ++++++++++++++++++++++++++++ src/utils/CircularList.ts | 32 +++++++++++++++++++++++++++----- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/utils/CircularList.test.ts b/src/utils/CircularList.test.ts index 3b2520a..41bb829 100644 --- a/src/utils/CircularList.test.ts +++ b/src/utils/CircularList.test.ts @@ -128,4 +128,32 @@ describe('CircularList', () => { assert.equal(list.get(2), '2'); }); }); + + describe('trimStart', () => { + it('should remove items from the beginning of the list', () => { + const list = new CircularList(5); + list.push('1'); + list.push('2'); + list.push('3'); + list.push('4'); + list.push('5'); + list.trimStart(1); + assert.equal(list.length, 4); + assert.deepEqual(list.get(0), '2'); + assert.deepEqual(list.get(1), '3'); + assert.deepEqual(list.get(2), '4'); + assert.deepEqual(list.get(3), '5'); + list.trimStart(2); + assert.equal(list.length, 2); + assert.deepEqual(list.get(0), '4'); + assert.deepEqual(list.get(1), '5'); + }); + + it('should remove all items if the requested trim amount is larger than the list\'s length', () => { + const list = new CircularList(5); + list.push('1'); + list.trimStart(2); + assert.equal(list.length, 0); + }); + }); }); diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts index ce79e35..1056c21 100644 --- a/src/utils/CircularList.ts +++ b/src/utils/CircularList.ts @@ -92,12 +92,23 @@ export class CircularList { } } + /** + * Removes and returns the last value on the list. + * @return The popped value. + */ public pop(): T { - // TODO: This isn't popping from the array, only returning return this._array[this._getCyclicIndex(this._length-- - 1)]; } - // TODO: Warn there's no error handling and that this is a slow operation + /** + * Deletes and/or inserts items at a particular index (in that order). Unlike + * Array.prototype.splice, this operation does not return the deleted items as a new array in + * order to save creating a new array. Note that this operation may shift all values in the list + * in the worst case. + * @param start The index to delete and/or insert. + * @param deleteCount The number of elements to delete. + * @param items The items to insert. + */ public splice(start: number, deleteCount: number, ...items: T[]): void { if (deleteCount) { for (let i = start; i < this._length - deleteCount; i++) { @@ -122,13 +133,24 @@ export class CircularList { } } + /** + * Trims a number of items from the start of the list. + * @param count The number of items to remove. + */ public trimStart(count: number): void { - // TODO: Error handling - // TODO: Testing (if we need this) + if (count > this._length) { + count = this._length; + } this._startIndex += count; - this._length--; + this._length -= count; } + /** + * Gets the cyclic index for the specified regular index. The cyclic index can then be used on the + * backing array to get the element associated with the regular index. + * @param index The regular index. + * @returns The cyclic index. + */ private _getCyclicIndex(index: number): number { return (this._startIndex + index) % this.maxLength; } From 969a2e953c976178af177c71d9900940933cec0c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 21 Dec 2016 03:18:30 -0800 Subject: [PATCH 09/15] Implement CircularList.shiftElements --- src/utils/CircularList.test.ts | 96 ++++++++++++++++++++++++++++++++++ src/utils/CircularList.ts | 30 +++++++++++ 2 files changed, 126 insertions(+) diff --git a/src/utils/CircularList.test.ts b/src/utils/CircularList.test.ts index 41bb829..aaa4a0d 100644 --- a/src/utils/CircularList.test.ts +++ b/src/utils/CircularList.test.ts @@ -156,4 +156,100 @@ describe('CircularList', () => { assert.equal(list.length, 0); }); }); + + describe('shiftElements', () => { + it('should not mutate the list when count is 0', () => { + const list = new CircularList(5); + list.push(1); + list.push(2); + list.shiftElements(0, 0, 1); + assert.equal(list.length, 2); + assert.equal(list.get(0), 1); + assert.equal(list.get(1), 2); + }); + + it('should throw for invalid args', () => { + const list = new CircularList(5); + list.push(1); + assert.throws(() => list.shiftElements(-1, 1, 1), 'start argument out of range'); + assert.throws(() => list.shiftElements(1, 1, 1), 'start argument out of range'); + assert.throws(() => list.shiftElements(0, 1, -1), 'Cannot shift elements in list beyond index 0'); + }); + + it('should shift an element forward', () => { + const list = new CircularList(5); + list.push(1); + list.push(2); + list.shiftElements(0, 1, 1); + assert.equal(list.length, 2); + assert.equal(list.get(0), 1); + assert.equal(list.get(1), 1); + }); + + it('should shift elements forward', () => { + const list = new CircularList(5); + list.push(1); + list.push(2); + list.push(3); + list.push(4); + list.shiftElements(0, 2, 2); + assert.equal(list.length, 4); + assert.equal(list.get(0), 1); + assert.equal(list.get(1), 2); + assert.equal(list.get(2), 1); + assert.equal(list.get(3), 2); + }); + + it('should shift elements forward, expanding the list if needed', () => { + const list = new CircularList(5); + list.push(1); + list.push(2); + list.shiftElements(0, 2, 2); + assert.equal(list.length, 4); + assert.equal(list.get(0), 1); + assert.equal(list.get(1), 2); + assert.equal(list.get(2), 1); + assert.equal(list.get(3), 2); + }); + + it('should shift elements forward, wrapping the list if needed', () => { + const list = new CircularList(5); + list.push(1); + list.push(2); + list.push(3); + list.push(4); + list.push(5); + list.shiftElements(2, 2, 3); + assert.equal(list.length, 5); + assert.equal(list.get(0), 3); + assert.equal(list.get(1), 4); + assert.equal(list.get(2), 5); + assert.equal(list.get(3), 3); + assert.equal(list.get(4), 4); + }); + + it('should shift an element backwards', () => { + const list = new CircularList(5); + list.push(1); + list.push(2); + list.shiftElements(1, 1, -1); + assert.equal(list.length, 2); + assert.equal(list.get(0), 2); + assert.equal(list.get(1), 2); + }); + + it('should shift elements backwards', () => { + const list = new CircularList(5); + list.push(1); + list.push(2); + list.push(3); + list.push(4); + list.shiftElements(2, 2, -2); + assert.equal(list.length, 4); + assert.equal(list.get(0), 3); + assert.equal(list.get(1), 4); + assert.equal(list.get(2), 3); + assert.equal(list.get(3), 4); + }); + }); }); diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts index 1056c21..a84a98e 100644 --- a/src/utils/CircularList.ts +++ b/src/utils/CircularList.ts @@ -145,6 +145,36 @@ export class CircularList { this._length -= count; } + public shiftElements(start: number, count: number, offset: number): void { + if (count <= 0) { + return; + } + if (start < 0 || start >= this._length) { + throw new Error('start argument out of range'); + } + if (start + offset < 0) { + throw new Error('Cannot shift elements in list beyond index 0'); + } + + if (offset > 0) { + for (let i = count - 1; i >= 0; i--) { + this.set(start + i + offset, this.get(start + i)); + } + const expandListBy = (start + count + offset) - this._length; + if (expandListBy > 0) { + this._length += expandListBy; + while (this._length > this.maxLength) { + this._length--; + this._startIndex++; + } + } + } else { + for (let i = 0; i < count; i++) { + this.set(start + i + offset, this.get(start + i)); + } + } + } + /** * Gets the cyclic index for the specified regular index. The cyclic index can then be used on the * backing array to get the element associated with the regular index. From bf16fdc01d1191e36e58eda6b285847a9fbc738f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 21 Dec 2016 03:26:40 -0800 Subject: [PATCH 10/15] Have reverseIndex use CircularList.shiftElements :tada: --- src/xterm.js | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/xterm.js b/src/xterm.js index dbf03ea..56cd4d3 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -3174,17 +3174,8 @@ Terminal.prototype.reverseIndex = function() { // possibly move the code below to term.reverseScroll(); // test: echo -ne '\e[1;1H\e[44m\eM\e[0m' // blankLine(true) is xterm/linux behavior - if (this.lines.length === this.lines.maxLength) { - // Trim the start of lines to make room for the new temporary row - // TODO: This section could be optimized by introducing a CircularList function that inserts, - // deletes and shifts elements to accomplish this task. - this.lines.trimStart(1); - this.ybase -= 1; - this.ydisp -= 1; - } - this.lines.splice(this.y + this.ybase, 0, this.blankLine(true)); - j = this.rows - 1 - this.scrollBottom; - this.lines.splice(this.rows - 1 + this.ybase - j + 1, 1); + this.lines.shiftElements(this.y + this.ybase, this.rows - 1, 1); + this.lines.set(this.y + this.ybase, this.blankLine(true)); this.updateRange(this.scrollTop); this.updateRange(this.scrollBottom); } else { From 6f7cb990ca19fbb11bb6c41c81b1bb38f66e0058 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 24 Dec 2016 00:04:31 -0800 Subject: [PATCH 11/15] Fix issue with vim scrolling --- src/xterm.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/xterm.js b/src/xterm.js index 56cd4d3..5886126 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -1249,7 +1249,13 @@ Terminal.prototype.scroll = function() { // subtract the bottom scroll region row -= this.rows - 1 - this.scrollBottom; - this.lines.push(this.blankLine()); + if (row === this.lines.length) { + // Optimization: pushing is faster than splicing when they amount to the same behavior + this.lines.push(this.blankLine()); + } else { + // add our new line + this.lines.splice(row, 0, this.blankLine()); + } if (this.scrollTop !== 0) { if (this.ybase !== 0) { From 30a1f1ccb50cc8295aef6db0b1306d6f2289f6db Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 24 Dec 2016 00:14:30 -0800 Subject: [PATCH 12/15] Fix issue with max buffer size after vim fix --- src/xterm.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/xterm.js b/src/xterm.js index 5886126..933a9e1 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -1234,9 +1234,7 @@ Terminal.prototype.showCursor = function() { Terminal.prototype.scroll = function() { var row; - if (this.lines.length < this.lines.maxLength) { - this.ybase++; - } + this.ybase++; // TODO: Why is this done twice? if (!this.userScrolling) { @@ -1250,6 +1248,11 @@ Terminal.prototype.scroll = function() { row -= this.rows - 1 - this.scrollBottom; if (row === this.lines.length) { + // Compensate ybase and ydisp if lines has hit the maximum buffer size + if (this.lines.length === this.lines.maxLength) { + this.ybase--; + this.ydisp--; + } // Optimization: pushing is faster than splicing when they amount to the same behavior this.lines.push(this.blankLine()); } else { From cc5ae8193b14171375e27e5b8cb320ed7d031cdc Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 24 Dec 2016 00:26:43 -0800 Subject: [PATCH 13/15] Fix insertLines as max buffer size --- src/xterm.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/xterm.js b/src/xterm.js index 933a9e1..8ede46e 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -3086,7 +3086,7 @@ Terminal.prototype.eraseLine = function(y) { /** - * Return the data array of a blank line/ + * Return the data array of a blank line * @param {number} cur First bunch of data for each "blank" character. */ Terminal.prototype.blankLine = function(cur) { @@ -3706,12 +3706,11 @@ Terminal.prototype.insertLines = function(params) { while (param--) { if (this.lines.length === this.lines.maxLength) { - // Trim the start of lines to make room for the new temporary row - // TODO: This section could be optimized by introducing a CircularList function that inserts, - // deletes and shifts elements to accomplish this task. this.lines.trimStart(1); - this.ybase -= 1; - this.ydisp -= 1; + this.ybase--; + this.ydisp--; + row--; + j--; } // test: echo -e '\e[44m\e[1L\e[0m' // blankLine(true) - xterm/linux behavior From d7f2ec89d783692760b9f666fe0d6229a83dac50 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 24 Dec 2016 03:17:30 -0800 Subject: [PATCH 14/15] Polish --- src/utils/CircularList.ts | 1 - src/xterm.js | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts index a84a98e..8835acb 100644 --- a/src/utils/CircularList.ts +++ b/src/utils/CircularList.ts @@ -38,7 +38,6 @@ export class CircularList { } public set length(newLength: number) { - // TODO: Is this auto fill is needed or can it be if (newLength > this._length) { for (let i = this._length; i < newLength; i++) { this._array[i] = undefined; diff --git a/src/xterm.js b/src/xterm.js index 8ede46e..a475b75 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -3706,6 +3706,7 @@ Terminal.prototype.insertLines = function(params) { while (param--) { if (this.lines.length === this.lines.maxLength) { + // Trim the start of lines to make room for the new line this.lines.trimStart(1); this.ybase--; this.ydisp--; @@ -3740,9 +3741,7 @@ Terminal.prototype.deleteLines = function(params) { while (param--) { if (this.lines.length === this.lines.maxLength) { - // Trim the start of lines to make room for the new temporary row - // TODO: This section could be optimized by introducing a CircularList function that inserts, - // deletes and shifts elements to accomplish this task. + // Trim the start of lines to make room for the new line this.lines.trimStart(1); this.ybase -= 1; this.ydisp -= 1; From 50cfe2b21a172706ea37b55176f017fcf9c3f97b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 27 Dec 2016 13:40:49 -0800 Subject: [PATCH 15/15] Improve header comment --- src/utils/CircularList.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts index 8835acb..b72c667 100644 --- a/src/utils/CircularList.ts +++ b/src/utils/CircularList.ts @@ -1,11 +1,8 @@ -/** - * xterm.js: xterm, in the browser - * Copyright (c) 2016, SourceLair Private Company (MIT License) - */ - /** * Represents a circular list; a list with a maximum size that wraps around when push is called, * overriding values at the start of the list. + * @module xterm/utils/CircularList + * @license MIT */ export class CircularList { private _array: T[];