From f03d00a4977feefe2d5b92b63b6e54312ea93207 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 5 Aug 2017 19:42:52 -0700 Subject: [PATCH] Clean up buffer clean up/fill logic The alt buffer is now cleared immediated after activating the normal buffer and is filled when switching to it. The tests were failing because the alt buffer wasn't being cleared properly with the previous solution. --- src/Buffer.ts | 72 ++++++++++++++++++++++++------------ src/BufferSet.test.ts | 10 ++--- src/BufferSet.ts | 10 +++++ src/InputHandler.ts | 1 - src/Interfaces.ts | 2 +- src/SelectionManager.test.ts | 26 +++++++------ src/SelectionModel.test.ts | 5 ++- src/utils/CircularList.ts | 3 +- src/utils/TestUtils.ts | 53 ++++++++++++++++++++++++++ src/xterm.js | 9 +---- 10 files changed, 139 insertions(+), 52 deletions(-) create mode 100644 src/utils/TestUtils.ts diff --git a/src/Buffer.ts b/src/Buffer.ts index bd3645e7..714151d0 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -2,7 +2,7 @@ * @license MIT */ -import { ITerminal } from './Interfaces'; +import { ITerminal, IBuffer } from './Interfaces'; import { CircularList } from './utils/CircularList'; /** @@ -12,9 +12,16 @@ import { CircularList } from './utils/CircularList'; * - cursor position * - scroll position */ -export class Buffer { - public readonly lines: CircularList<[number, string, number][]>; +export class Buffer implements IBuffer { + private _lines: CircularList<[number, string, number][]>; + public ydisp: number; + public ybase: number; + public y: number; + public x: number; + public scrollBottom: number; + public scrollTop: number; + public tabs: any; public savedY: number; public savedX: number; @@ -27,34 +34,51 @@ export class Buffer { * @param {number} x - The cursor's x position after ybase */ constructor( - private _terminal: ITerminal, - public ydisp: number = 0, - public ybase: number = 0, - public y: number = 0, - public x: number = 0, - public scrollBottom: number = 0, - public scrollTop: number = 0, - public tabs: any = {}, + private _terminal: ITerminal ) { - this.lines = new CircularList<[number, string, number][]>(this._terminal.scrollback); + this.clear(); + } + + public get lines(): CircularList<[number, string, number][]> { + return this._lines; + } + + public fillViewportRows(): void { + if (this._lines.length === 0) { + let i = this._terminal.rows; + while (i--) { + this.lines.push(this._terminal.blankLine()); + } + } + } + + public clear(): void { + this.ydisp = 0; + this.ybase = 0; + this.y = 0; + this.x = 0; + this.scrollBottom = 0; + this.scrollTop = 0; + this.tabs = {}; + this._lines = new CircularList<[number, string, number][]>(this._terminal.scrollback); this.scrollBottom = this._terminal.rows - 1; } public resize(newCols: number, newRows: number): void { // Don't resize the buffer if it's empty and hasn't been used yet. - if (this.lines.length === 0) { + if (this._lines.length === 0) { return; } // Deal with columns increasing (we don't do anything when columns reduce) if (this._terminal.cols < newCols) { const ch: [number, string, number] = [this._terminal.defAttr, ' ', 1]; // does xterm use the default attr? - for (let i = 0; i < this.lines.length; i++) { - if (this.lines.get(i) === undefined) { - this.lines.set(i, this._terminal.blankLine()); + for (let i = 0; i < this._lines.length; i++) { + if (this._lines.get(i) === undefined) { + this._lines.set(i, this._terminal.blankLine()); } - while (this.lines.get(i).length < newCols) { - this.lines.get(i).push(ch); + while (this._lines.get(i).length < newCols) { + this._lines.get(i).push(ch); } } } @@ -63,8 +87,8 @@ export class Buffer { let addToY = 0; if (this._terminal.rows < newRows) { for (let y = this._terminal.rows; y < newRows; y++) { - if (this.lines.length < newRows + this.ybase) { - if (this.ybase > 0 && this.lines.length <= this.ybase + this.y + addToY + 1) { + if (this._lines.length < newRows + this.ybase) { + if (this.ybase > 0 && this._lines.length <= this.ybase + this.y + addToY + 1) { // There is room above the buffer and there are no empty elements below the line, // scroll up this.ybase--; @@ -76,16 +100,16 @@ export class Buffer { } else { // Add a blank line if there is no buffer left at the top to scroll to, or if there // are blank lines after the cursor - this.lines.push(this._terminal.blankLine()); + this._lines.push(this._terminal.blankLine()); } } } } else { // (this._terminal.rows >= newRows) for (let y = this._terminal.rows; y > newRows; y--) { - if (this.lines.length > newRows + this.ybase) { - if (this.lines.length > this.ybase + this.y + 1) { + if (this._lines.length > newRows + this.ybase) { + if (this._lines.length > this.ybase + this.y + 1) { // The line is a blank line below the cursor, remove it - this.lines.pop(); + this._lines.pop(); } else { // The line is the cursor, scroll down this.ybase++; diff --git a/src/BufferSet.test.ts b/src/BufferSet.test.ts index 2101fbc1..ab814cee 100644 --- a/src/BufferSet.test.ts +++ b/src/BufferSet.test.ts @@ -5,17 +5,17 @@ import { assert } from 'chai'; import { ITerminal } from './Interfaces'; import { BufferSet } from './BufferSet'; import { Buffer } from './Buffer'; +import { MockTerminal } from './utils/TestUtils'; describe('BufferSet', () => { let terminal: ITerminal; let bufferSet: BufferSet; beforeEach(() => { - terminal = { - cols: 80, - rows: 24, - scrollback: 1000 - }; + terminal = new MockTerminal(); + terminal.cols = 80; + terminal.rows = 24; + terminal.scrollback = 1000; bufferSet = new BufferSet(terminal); }); diff --git a/src/BufferSet.ts b/src/BufferSet.ts index 4a65dbfe..345191cf 100644 --- a/src/BufferSet.ts +++ b/src/BufferSet.ts @@ -22,6 +22,7 @@ export class BufferSet extends EventEmitter implements IBufferSet { constructor(private _terminal: ITerminal) { super(); this._normal = new Buffer(this._terminal); + this._normal.fillViewportRows(); this._alt = new Buffer(this._terminal); this._activeBuffer = this._normal; } @@ -54,6 +55,11 @@ export class BufferSet extends EventEmitter implements IBufferSet { * Sets the normal Buffer of the BufferSet as its currently active Buffer */ public activateNormalBuffer(): void { + // The alt buffer should always be cleared when we switch to the normal + // buffer. This frees up memory since the alt buffer should always be new + // when activated. + this._alt.clear(); + this._activeBuffer = this._normal; this.emit('activate', this._normal); } @@ -62,6 +68,10 @@ export class BufferSet extends EventEmitter implements IBufferSet { * Sets the alt Buffer of the BufferSet as its currently active Buffer */ public activateAltBuffer(): void { + // Since the alt buffer is always cleared when the normal buffer is + // activated, we want to fill it when switching to it. + this._alt.fillViewportRows(); + this._activeBuffer = this._alt; this.emit('activate', this._alt); } diff --git a/src/InputHandler.ts b/src/InputHandler.ts index 64da3b3f..b915f2c0 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -954,7 +954,6 @@ export class InputHandler implements IInputHandler { case 47: // alt screen buffer case 1047: // alt screen buffer this._terminal.buffers.activateAltBuffer(); - this._terminal.reset(); this._terminal.viewport.syncScrollArea(); this._terminal.showCursor(); break; diff --git a/src/Interfaces.ts b/src/Interfaces.ts index d463fcb7..70a46d41 100644 --- a/src/Interfaces.ts +++ b/src/Interfaces.ts @@ -93,8 +93,8 @@ export interface ILinkifier { export interface ICircularList extends IEventEmitter { length: number; maxLength: number; + forEach: (callbackfn: (value: T, index: number) => void) => void; - forEach(callbackfn: (value: T, index: number, array: T[]) => void): void; get(index: number): T; set(index: number, value: T): void; push(value: T): void; diff --git a/src/SelectionManager.test.ts b/src/SelectionManager.test.ts index c2173d77..7beafd97 100644 --- a/src/SelectionManager.test.ts +++ b/src/SelectionManager.test.ts @@ -9,6 +9,7 @@ import { CircularList } from './utils/CircularList'; import { SelectionManager } from './SelectionManager'; import { SelectionModel } from './SelectionModel'; import { BufferSet } from './BufferSet'; +import { MockTerminal } from './utils/TestUtils'; class TestSelectionManager extends SelectionManager { constructor( @@ -46,7 +47,9 @@ describe('SelectionManager', () => { window = dom.window; document = window.document; rowContainer = document.createElement('div'); - terminal = { cols: 80, rows: 2 }; + terminal = new MockTerminal(); + terminal.cols = 80; + terminal.rows = 2; terminal.scrollback = 100; terminal.buffers = new BufferSet(terminal); terminal.buffer = terminal.buffers.active; @@ -64,7 +67,7 @@ describe('SelectionManager', () => { describe('_selectWordAt', () => { it('should expand selection for normal width chars', () => { - bufferLines.push(stringToRow('foo bar')); + bufferLines.set(0, stringToRow('foo bar')); selectionManager.selectWordAt([0, 0]); assert.equal(selectionManager.selectionText, 'foo'); selectionManager.selectWordAt([1, 0]); @@ -81,7 +84,7 @@ describe('SelectionManager', () => { assert.equal(selectionManager.selectionText, 'bar'); }); it('should expand selection for whitespace', () => { - bufferLines.push(stringToRow('a b')); + bufferLines.set(0, stringToRow('a b')); selectionManager.selectWordAt([0, 0]); assert.equal(selectionManager.selectionText, 'a'); selectionManager.selectWordAt([1, 0]); @@ -95,7 +98,7 @@ describe('SelectionManager', () => { }); it('should expand selection for wide characters', () => { // Wide characters use a special format - bufferLines.push([ + bufferLines.set(0, [ [null, '中', 2], [null, '', 0], [null, '文', 2], @@ -147,7 +150,7 @@ describe('SelectionManager', () => { assert.equal(selectionManager.selectionText, 'foo'); }); it('should select up to non-path characters that are commonly adjacent to paths', () => { - bufferLines.push(stringToRow('(cd)[ef]{gh}\'ij"')); + bufferLines.set(0, stringToRow('(cd)[ef]{gh}\'ij"')); selectionManager.selectWordAt([0, 0]); assert.equal(selectionManager.selectionText, '(cd'); selectionManager.selectWordAt([1, 0]); @@ -185,7 +188,7 @@ describe('SelectionManager', () => { describe('_selectLineAt', () => { it('should select the entire line', () => { - bufferLines.push(stringToRow('foo bar')); + bufferLines.set(0, stringToRow('foo bar')); selectionManager.selectLineAt(0); assert.equal(selectionManager.selectionText, 'foo bar', 'The selected text is correct'); assert.deepEqual(selectionManager.model.finalSelectionStart, [0, 0]); @@ -195,11 +198,12 @@ describe('SelectionManager', () => { describe('selectAll', () => { it('should select the entire buffer, beyond the viewport', () => { - bufferLines.push(stringToRow('1')); - bufferLines.push(stringToRow('2')); - bufferLines.push(stringToRow('3')); - bufferLines.push(stringToRow('4')); - bufferLines.push(stringToRow('5')); + bufferLines.length = 5; + bufferLines.set(0, stringToRow('1')); + bufferLines.set(1, stringToRow('2')); + bufferLines.set(2, stringToRow('3')); + bufferLines.set(3, stringToRow('4')); + bufferLines.set(4, stringToRow('5')); selectionManager.selectAll(); terminal.buffer.ybase = bufferLines.length - terminal.rows; assert.equal(selectionManager.selectionText, '1\n2\n3\n4\n5'); diff --git a/src/SelectionModel.test.ts b/src/SelectionModel.test.ts index 6da3874b..b0879442 100644 --- a/src/SelectionModel.test.ts +++ b/src/SelectionModel.test.ts @@ -5,6 +5,7 @@ import { assert } from 'chai'; import { ITerminal } from './Interfaces'; import { SelectionModel } from './SelectionModel'; import {BufferSet} from './BufferSet'; +import { MockTerminal } from './utils/TestUtils'; class TestSelectionModel extends SelectionModel { constructor( @@ -22,7 +23,9 @@ describe('SelectionManager', () => { let model: TestSelectionModel; beforeEach(() => { - terminal = { cols: 80, rows: 2, ybase: 0 }; + terminal = new MockTerminal(); + terminal.cols = 80; + terminal.rows = 2; terminal.scrollback = 10; terminal.buffers = new BufferSet(terminal); terminal.buffer = terminal.buffers.active; diff --git a/src/utils/CircularList.ts b/src/utils/CircularList.ts index d0b2f685..54850ab7 100644 --- a/src/utils/CircularList.ts +++ b/src/utils/CircularList.ts @@ -5,8 +5,9 @@ * @license MIT */ import { EventEmitter } from '../EventEmitter'; +import { ICircularList } from '../Interfaces'; -export class CircularList extends EventEmitter { +export class CircularList extends EventEmitter implements ICircularList { private _array: T[]; private _startIndex: number; private _length: number; diff --git a/src/utils/TestUtils.ts b/src/utils/TestUtils.ts new file mode 100644 index 00000000..fbb17261 --- /dev/null +++ b/src/utils/TestUtils.ts @@ -0,0 +1,53 @@ +import { ITerminal, IBuffer, IBufferSet, IBrowser, ICharMeasure, ISelectionManager } from '../Interfaces'; + +export class MockTerminal implements ITerminal { + public element: HTMLElement; + public rowContainer: HTMLElement; + public selectionContainer: HTMLElement; + public selectionManager: ISelectionManager; + public charMeasure: ICharMeasure; + public textarea: HTMLTextAreaElement; + public rows: number; + public cols: number; + public browser: IBrowser; + public writeBuffer: string[]; + public children: HTMLElement[]; + public cursorHidden: boolean; + public cursorState: number; + public defAttr: number; + public scrollback: number; + public buffers: IBufferSet; + public buffer: IBuffer; + + handler(data: string) { + throw new Error('Method not implemented.'); + } + on(event: string, callback: () => void) { + throw new Error('Method not implemented.'); + } + scrollDisp(disp: number, suppressScrollEvent: boolean) { + throw new Error('Method not implemented.'); + } + cancel(ev: Event, force?: boolean) { + throw new Error('Method not implemented.'); + } + log(text: string): void { + throw new Error('Method not implemented.'); + } + emit(event: string, data: any) { + throw new Error('Method not implemented.'); + } + reset(): void { + throw new Error('Method not implemented.'); + } + showCursor(): void { + throw new Error('Method not implemented.'); + } + blankLine(cur?: boolean, isWrapped?: boolean) { + const line = []; + for (let i = 0; i < this.cols; i++) { + line.push([0, ' ', 1]); + } + return line; + } +} diff --git a/src/xterm.js b/src/xterm.js index 92fc0df8..e9e3465e 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -221,17 +221,12 @@ function Terminal(options) { this.surrogate_high = ''; // Create the terminal's buffers and set the current buffer - this.buffers = this.buffers || new BufferSet(this); + this.buffers = new BufferSet(this); this.buffer = this.buffers.active; // Convenience shortcut; this.buffers.on('activate', function (buffer) { this._terminal.buffer = buffer; }); - var i = this.rows; - - while (i--) { - this.buffer.lines.push(this.blankLine()); - } // Ensure the selection manager has the correct buffer if (this.selectionManager) { this.selectionManager.setBuffer(this.buffer.lines); @@ -2228,12 +2223,10 @@ Terminal.prototype.reset = function() { var customKeyEventHandler = this.customKeyEventHandler; var cursorBlinkInterval = this.cursorBlinkInterval; var inputHandler = this.inputHandler; - var buffers = this.buffers; Terminal.call(this, this.options); this.customKeyEventHandler = customKeyEventHandler; this.cursorBlinkInterval = cursorBlinkInterval; this.inputHandler = inputHandler; - this.buffers = buffers; this.refresh(0, this.rows - 1); this.viewport.syncScrollArea(); };