From e7ee3ec63c005f7073d8daab8dd95eaff2f33c83 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 6 Aug 2017 18:56:37 -0700 Subject: [PATCH] Fix trimming on a buffer with 0 scrollback Unfortunately this removed the padding idea from CircularList as it would only work when the list is not full. --- src/Buffer.test.ts | 15 +++++++++++++++ src/Buffer.ts | 12 ++++++------ src/utils/CircularList.test.ts | 25 ------------------------- src/utils/CircularList.ts | 28 +++------------------------- 4 files changed, 24 insertions(+), 56 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 2e2ae50c..d96e6bde 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -88,6 +88,21 @@ describe('Buffer', () => { assert.equal(buffer.ydisp, 5); assert.equal(buffer.ybase, 5); }); + + describe('no scrollback', () => { + it('should trim from the top of the buffer when the cursor reaches the bottom', () => { + terminal.options.scrollback = 0; + buffer = new Buffer(terminal, false); + assert.equal(buffer.lines.maxLength, INIT_ROWS); + buffer.y = INIT_ROWS - 1; + buffer.fillViewportRows(); + buffer.lines.get(5)[0][1] = 'a'; + buffer.lines.get(INIT_ROWS - 1)[0][1] = 'b'; + buffer.resize(INIT_COLS, INIT_ROWS - 5); + assert.equal(buffer.lines.get(0)[0][1], 'a'); + assert.equal(buffer.lines.get(INIT_ROWS - 1 - 5)[0][1], 'b'); + }); + }); }); describe('row size increased', () => { diff --git a/src/Buffer.ts b/src/Buffer.ts index 5fe97779..74f440ea 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -154,12 +154,12 @@ export class Buffer implements IBuffer { // would otherwise cut data from the bottom of the buffer. if (newMaxLength < this._lines.maxLength) { // Trim from the top of the buffer and adjust ybase and ydisp. - // const amountToTrim = this._lines.length - newMaxLength; - // if (amountToTrim > 0) { - // this._lines.trimStart(amountToTrim); - // this.ybase = Math.max(this.ybase - amountToTrim, 0); - // this.ydisp = Math.max(this.ydisp - amountToTrim, 0); - // } + const amountToTrim = this._lines.length - newMaxLength; + if (amountToTrim > 0) { + this._lines.trimStart(amountToTrim); + this.ybase = Math.max(this.ybase - amountToTrim, 0); + this.ydisp = Math.max(this.ydisp - amountToTrim, 0); + } this._lines.maxLength = newMaxLength; } diff --git a/src/utils/CircularList.test.ts b/src/utils/CircularList.test.ts index 891ba3c5..f404b6f2 100644 --- a/src/utils/CircularList.test.ts +++ b/src/utils/CircularList.test.ts @@ -9,9 +9,6 @@ class TestCircularList extends CircularList { public get array(): T[] { return this._array; } } -const MAX_LENGTH_PADDING = 40; -const MAX_LENGTH_SHRINK_REBUILD_THRESHOLD = 120; - describe('CircularList', () => { describe('push', () => { it('should push values onto the array', () => { @@ -75,28 +72,6 @@ describe('CircularList', () => { list.maxLength = 4; assert.equal(list.maxLength, 4); }); - - it('should resize the backing array\'s size only when it increases beyond the allocated padding', () => { - const initSize = 200; - const list = new TestCircularList(initSize); - const expectedBackingMaxLength = initSize + MAX_LENGTH_PADDING; - assert.equal(list.array.length, expectedBackingMaxLength); - list.maxLength = expectedBackingMaxLength; - assert.equal(list.array.length, expectedBackingMaxLength); - list.maxLength = expectedBackingMaxLength + 1; - assert.equal(list.array.length, expectedBackingMaxLength + 1 + MAX_LENGTH_PADDING); - }); - - it('should resize the backing array\'s size only when it reduces beyond the shrink threshold', () => { - const initSize = 200; - const list = new TestCircularList(initSize); - const expectedThresholdMaxLength = initSize + MAX_LENGTH_PADDING - MAX_LENGTH_SHRINK_REBUILD_THRESHOLD; - assert.equal(list.array.length, initSize + MAX_LENGTH_PADDING); - list.maxLength = expectedThresholdMaxLength; - assert.equal(list.array.length, initSize + MAX_LENGTH_PADDING); - list.maxLength = expectedThresholdMaxLength - 1; - assert.equal(list.array.length, expectedThresholdMaxLength - 1 + MAX_LENGTH_PADDING); - }); }); describe('length', () => { diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts index ed8c065d..e188fb5d 100644 --- a/src/utils/CircularList.ts +++ b/src/utils/CircularList.ts @@ -8,15 +8,6 @@ import { EventEmitter } from '../EventEmitter'; import { ICircularList } from '../Interfaces'; -/** - * Padding added to the max length of the CircularList, this allows increasing - * the max length of the list without reconstructing the backing array and then - * copying over the values. - */ -const MAX_LENGTH_PADDING = 40; - -const MAX_LENGTH_SHRINK_REBUILD_THRESHOLD = MAX_LENGTH_PADDING * 3; - export class CircularList extends EventEmitter implements ICircularList { protected _array: T[]; private _startIndex: number; @@ -26,7 +17,7 @@ export class CircularList extends EventEmitter implements ICircularList { private _maxLength: number ) { super(); - this._array = new Array(this._maxLength + MAX_LENGTH_PADDING); + this._array = new Array(this._maxLength); this._startIndex = 0; this._length = 0; } @@ -41,22 +32,9 @@ export class CircularList extends EventEmitter implements ICircularList { return; } - // Ensure the array has padding to expand into. - if (this._array.length >= newMaxLength) { - // Ensure that the array hasn't shrunk significantly, if it has we - // should rebuild the array with a new max length to reclaim some memory. - if (this._array.length - newMaxLength <= MAX_LENGTH_SHRINK_REBUILD_THRESHOLD) { - // Increase the max length and return. - this._maxLength = newMaxLength; - return; - } - } - // Reconstruct array, starting at index 0. Only transfer values from the - // indexes 0 to length. The new array's size includes some padding that - // allows the array's size to increase a little without rebuilding the - // array. - let newArray = new Array(newMaxLength + MAX_LENGTH_PADDING); + // 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)]; }