From 90c8c85c09094b884573e335b2d4ad5c678e7a9a Mon Sep 17 00:00:00 2001 From: Andres Mejia Date: Sat, 13 Jan 2018 14:20:10 -0500 Subject: [PATCH 1/5] Fix issue with resizing terminal. Need to ensure the active buffer is always being used. Fixes https://github.com/xtermjs/xterm.js/issues/824 . --- src/Buffer.ts | 5 ----- src/InputHandler.ts | 2 -- src/Interfaces.ts | 1 - src/SelectionManager.test.ts | 5 ++--- src/SelectionManager.ts | 15 ++++----------- src/Terminal.ts | 19 +++++++------------ 6 files changed, 13 insertions(+), 34 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 623c3854..3e7d6122 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -121,11 +121,6 @@ export class Buffer implements IBuffer { if (this._terminal.cols < newCols) { const ch: CharData = [this._terminal.defAttr, ' ', 1, 32]; // does xterm use the default attr? for (let i = 0; i < this._lines.length; i++) { - // TODO: This should be removed, with tests setup for the case that was - // causing the underlying bug, see https://github.com/sourcelair/xterm.js/issues/824 - if (this._lines.get(i) === undefined) { - this._lines.set(i, this._terminal.blankLine(undefined, undefined, newCols)); - } while (this._lines.get(i).length < newCols) { this._lines.get(i).push(ch); } diff --git a/src/InputHandler.ts b/src/InputHandler.ts index 7d3a4d24..96e34cc0 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -928,7 +928,6 @@ export class InputHandler implements IInputHandler { case 47: // alt screen buffer case 1047: // alt screen buffer this._terminal.buffers.activateAltBuffer(); - this._terminal.selectionManager.setBuffer(this._terminal.buffer); this._terminal.viewport.syncScrollArea(); this._terminal.showCursor(); break; @@ -1100,7 +1099,6 @@ export class InputHandler implements IInputHandler { // if (params[0] === 1049) { // this.restoreCursor(params); // } - this._terminal.selectionManager.setBuffer(this._terminal.buffer); this._terminal.refresh(0, this._terminal.rows - 1); this._terminal.viewport.syncScrollArea(); this._terminal.showCursor(); diff --git a/src/Interfaces.ts b/src/Interfaces.ts index 8f0baa2f..07610956 100644 --- a/src/Interfaces.ts +++ b/src/Interfaces.ts @@ -198,7 +198,6 @@ export interface ISelectionManager { disable(): void; enable(): void; - setBuffer(buffer: IBuffer): void; setSelection(row: number, col: number, length: number): void; } diff --git a/src/SelectionManager.test.ts b/src/SelectionManager.test.ts index a10dc669..97afcf2b 100644 --- a/src/SelectionManager.test.ts +++ b/src/SelectionManager.test.ts @@ -21,10 +21,9 @@ class TestMockTerminal extends MockTerminal { class TestSelectionManager extends SelectionManager { constructor( terminal: ITerminal, - buffer: IBuffer, charMeasure: CharMeasure ) { - super(terminal, buffer, charMeasure); + super(terminal, charMeasure); } public get model(): SelectionModel { return this._model; } @@ -59,7 +58,7 @@ describe('SelectionManager', () => { terminal.buffers = new BufferSet(terminal); terminal.buffer = terminal.buffers.active; buffer = terminal.buffer; - selectionManager = new TestSelectionManager(terminal, buffer, null); + selectionManager = new TestSelectionManager(terminal, null); }); function stringToRow(text: string): LineData { diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index cd6fc996..3e9a4d40 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -98,7 +98,6 @@ export class SelectionManager extends EventEmitter implements ISelectionManager constructor( private _terminal: ITerminal, - private _buffer: IBuffer, private _charMeasure: CharMeasure ) { super(); @@ -109,6 +108,10 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._activeSelectionMode = SelectionMode.NORMAL; } + private get _buffer(): IBuffer { + return this._terminal.buffers.active; + } + /** * Initializes listener variables. */ @@ -139,16 +142,6 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._enabled = true; } - /** - * Sets the active buffer, this should be called when the alt buffer is - * switched in or out. - * @param buffer The active buffer. - */ - public setBuffer(buffer: IBuffer): void { - this._buffer = buffer; - this.clearSelection(); - } - public get selectionStart(): [number, number] { return this._model.finalSelectionStart; } public get selectionEnd(): [number, number] { return this._model.finalSelectionEnd; } diff --git a/src/Terminal.ts b/src/Terminal.ts index d9bb27d1..511cb384 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -192,7 +192,6 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT public selectionManager: SelectionManager; public linkifier: ILinkifier; public buffers: BufferSet; - public buffer: Buffer; public viewport: IViewport; private compositionHelper: ICompositionHelper; public charMeasure: CharMeasure; @@ -293,15 +292,13 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT // Create the terminal's buffers and set the current buffer this.buffers = new BufferSet(this); - this.buffer = this.buffers.active; // Convenience shortcut; - this.buffers.on('activate', (buffer: Buffer) => { - this.buffer = buffer; - }); + } - // Ensure the selection manager has the correct buffer - if (this.selectionManager) { - this.selectionManager.setBuffer(this.buffer); - } + /** + * Convenience property to active buffer. + */ + public get buffer(): Buffer { + return this.buffers.active; } /** @@ -640,7 +637,7 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT this.charMeasure.on('charsizechanged', () => this.renderer.onResize(this.cols, this.rows, true)); this.renderer.on('resize', (dimensions) => this.viewport.syncScrollArea()); - this.selectionManager = new SelectionManager(this, this.buffer, this.charMeasure); + this.selectionManager = new SelectionManager(this, this.charMeasure); this.element.addEventListener('mousedown', (e: MouseEvent) => this.selectionManager.onMouseDown(e)); this.selectionManager.on('refresh', data => this.renderer.onSelectionChanged(data.start, data.end)); this.selectionManager.on('newselection', text => { @@ -2079,11 +2076,9 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT this.options.cols = this.cols; const customKeyEventHandler = this.customKeyEventHandler; const inputHandler = this.inputHandler; - const buffers = this.buffers; this.setup(); this.customKeyEventHandler = customKeyEventHandler; this.inputHandler = inputHandler; - this.buffers = buffers; this.refresh(0, this.rows - 1); this.viewport.syncScrollArea(); } From a9b7ef5dc38373a232f4e6d91b0516867d06f020 Mon Sep 17 00:00:00 2001 From: Andres Mejia Date: Sun, 14 Jan 2018 13:03:07 -0500 Subject: [PATCH 2/5] Clear selection manager when the terminal switches its buffer set or buffer. --- src/Interfaces.ts | 2 +- src/SelectionManager.ts | 26 +++++++++++++++++++++----- src/Terminal.ts | 1 + src/utils/TestUtils.test.ts | 2 +- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/Interfaces.ts b/src/Interfaces.ts index 07610956..780a4047 100644 --- a/src/Interfaces.ts +++ b/src/Interfaces.ts @@ -169,7 +169,7 @@ export interface IBuffer { prevStop(x?: number): number; } -export interface IBufferSet { +export interface IBufferSet extends IEventEmitter { alt: IBuffer; normal: IBuffer; active: IBuffer; diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 3e9a4d40..29003629 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -119,11 +119,27 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._mouseMoveListener = event => this._onMouseMove(event); this._mouseUpListener = event => this._onMouseUp(event); - // Only adjust the selection on trim, shiftElements is rarely used (only in - // reverseIndex) and delete in a splice is only ever used when the same - // number of elements was just added. Given this is could actually be - // beneficial to leave the selection as is for these cases. - this._buffer.lines.on('trim', (amount: number) => this._onTrim(amount)); + this._updateBufferSetHandlers(); + this._terminal.on('setup', () => { + this._onSetupHandler(); + }); + } + + private _onSetupHandler(): void { + this.clearSelection(); + this._updateBufferSetHandlers(); + } + + private _updateBufferSetHandlers(): void { + this._terminal.buffers.on('activate', (buffer: IBuffer) => { + this.clearSelection(); + }); + // Only adjust the selection on trim, shiftElements is rarely used (only in + // reverseIndex) and delete in a splice is only ever used when the same + // number of elements was just added. Given this is could actually be + // beneficial to leave the selection as is for these cases. + this._terminal.buffers.normal.lines.on('trim', (amount: number) => this._onTrim(amount)); + this._terminal.buffers.alt.lines.on('trim', (amount: number) => this._onTrim(amount)); } /** diff --git a/src/Terminal.ts b/src/Terminal.ts index 511cb384..11c2cf5e 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -292,6 +292,7 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT // Create the terminal's buffers and set the current buffer this.buffers = new BufferSet(this); + this.emit('setup'); } /** diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index 80e25277..54c52078 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -37,7 +37,7 @@ export class MockTerminal implements ITerminal { throw new Error('Method not implemented.'); } on(event: string, callback: () => void): void { - throw new Error('Method not implemented.'); + /* Simply do nothing */ } off(type: string, listener: IListenerType): void { throw new Error('Method not implemented.'); From cfd00529eb2c48958c4d83ef3343ea4655bea1e5 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 14 Jan 2018 17:19:31 -0800 Subject: [PATCH 3/5] Various fixes - Add buffers being retained on full reset (CIS) - Remove setup event, simplify - Only have one trim listener active at once - Disallow activate events if nothing changed (just in case) --- src/BufferSet.ts | 17 ++++++++++++++--- src/SelectionManager.ts | 37 +++++++++++++++---------------------- src/Terminal.ts | 3 ++- src/utils/TestUtils.test.ts | 2 +- 4 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/BufferSet.ts b/src/BufferSet.ts index e31d2278..da8c75f2 100644 --- a/src/BufferSet.ts +++ b/src/BufferSet.ts @@ -61,24 +61,35 @@ export class BufferSet extends EventEmitter implements IBufferSet { * Sets the normal Buffer of the BufferSet as its currently active Buffer */ public activateNormalBuffer(): void { + if (this._activeBuffer === this._normal) { + return; + } // 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); + this.emit('activate', { + activeBuffer: this._normal, + inactiveBuffer: this._alt + }); } /** * Sets the alt Buffer of the BufferSet as its currently active Buffer */ public activateAltBuffer(): void { + if (this._activeBuffer === this._alt) { + return; + } // 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); + this.emit('activate', { + activeBuffer: this._alt, + inactiveBuffer: this._normal + }); } /** diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 29003629..e625ab73 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -8,7 +8,7 @@ import * as Browser from './utils/Browser'; import { CharMeasure } from './utils/CharMeasure'; import { CircularList } from './utils/CircularList'; import { EventEmitter } from './EventEmitter'; -import { ITerminal, ICircularList, ISelectionManager, IBuffer } from './Interfaces'; +import { ITerminal, ICircularList, ISelectionManager, IBuffer, IListenerType } from './Interfaces'; import { SelectionModel } from './SelectionModel'; import { LineData, CharData } from './Types'; import { CHAR_DATA_WIDTH_INDEX, CHAR_DATA_CHAR_INDEX } from './Buffer'; @@ -95,6 +95,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager private _mouseMoveListener: EventListener; private _mouseUpListener: EventListener; + private _trimListener: IListenerType; constructor( private _terminal: ITerminal, @@ -118,28 +119,10 @@ export class SelectionManager extends EventEmitter implements ISelectionManager private _initListeners(): void { this._mouseMoveListener = event => this._onMouseMove(event); this._mouseUpListener = event => this._onMouseUp(event); + this._trimListener = (amount: number) => this._onTrim(amount); - this._updateBufferSetHandlers(); - this._terminal.on('setup', () => { - this._onSetupHandler(); - }); - } - - private _onSetupHandler(): void { - this.clearSelection(); - this._updateBufferSetHandlers(); - } - - private _updateBufferSetHandlers(): void { - this._terminal.buffers.on('activate', (buffer: IBuffer) => { - this.clearSelection(); - }); - // Only adjust the selection on trim, shiftElements is rarely used (only in - // reverseIndex) and delete in a splice is only ever used when the same - // number of elements was just added. Given this is could actually be - // beneficial to leave the selection as is for these cases. - this._terminal.buffers.normal.lines.on('trim', (amount: number) => this._onTrim(amount)); - this._terminal.buffers.alt.lines.on('trim', (amount: number) => this._onTrim(amount)); + this._terminal.buffer.lines.on('trim', this._trimListener); + this._terminal.buffers.on('activate', e => this._onBufferActivate(e)); } /** @@ -556,6 +539,16 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._terminal.emit('selection'); } + private _onBufferActivate(e: {activeBuffer: IBuffer, inactiveBuffer: IBuffer}): void { + this.clearSelection(); + // Only adjust the selection on trim, shiftElements is rarely used (only in + // reverseIndex) and delete in a splice is only ever used when the same + // number of elements was just added. Given this is could actually be + // beneficial to leave the selection as is for these cases. + e.inactiveBuffer.lines.off('trim', this._trimListener); + e.activeBuffer.lines.on('trim', this._trimListener); + } + /** * Converts a viewport column to the character index on the buffer line, the * latter takes into account wide characters. diff --git a/src/Terminal.ts b/src/Terminal.ts index 11c2cf5e..02c74a3d 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -292,7 +292,6 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT // Create the terminal's buffers and set the current buffer this.buffers = new BufferSet(this); - this.emit('setup'); } /** @@ -2077,9 +2076,11 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT this.options.cols = this.cols; const customKeyEventHandler = this.customKeyEventHandler; const inputHandler = this.inputHandler; + const buffers = this.buffers; this.setup(); this.customKeyEventHandler = customKeyEventHandler; this.inputHandler = inputHandler; + this.buffers = buffers; this.refresh(0, this.rows - 1); this.viewport.syncScrollArea(); } diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index 54c52078..80e25277 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -37,7 +37,7 @@ export class MockTerminal implements ITerminal { throw new Error('Method not implemented.'); } on(event: string, callback: () => void): void { - /* Simply do nothing */ + throw new Error('Method not implemented.'); } off(type: string, listener: IListenerType): void { throw new Error('Method not implemented.'); From 5133f25fb7d47ba98080b3e03fd86fda771f71e0 Mon Sep 17 00:00:00 2001 From: Andres Mejia Date: Tue, 16 Jan 2018 18:27:00 -0500 Subject: [PATCH 4/5] Fix issue with resetting terminal. Ensure new BufferSet is used and clear selection manager on reset. --- src/SelectionManager.ts | 4 ++++ src/Terminal.ts | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index e625ab73..33585f98 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -121,6 +121,10 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._mouseUpListener = event => this._onMouseUp(event); this._trimListener = (amount: number) => this._onTrim(amount); + this.initBuffersListeners(); + } + + public initBuffersListeners(): void { this._terminal.buffer.lines.on('trim', this._trimListener); this._terminal.buffers.on('activate', e => this._onBufferActivate(e)); } diff --git a/src/Terminal.ts b/src/Terminal.ts index 02c74a3d..8b38fdbd 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -292,6 +292,10 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT // Create the terminal's buffers and set the current buffer this.buffers = new BufferSet(this); + if (this.selectionManager) { + this.selectionManager.clearSelection(); + this.selectionManager.initBuffersListeners(); + } } /** @@ -2076,11 +2080,9 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT this.options.cols = this.cols; const customKeyEventHandler = this.customKeyEventHandler; const inputHandler = this.inputHandler; - const buffers = this.buffers; this.setup(); this.customKeyEventHandler = customKeyEventHandler; this.inputHandler = inputHandler; - this.buffers = buffers; this.refresh(0, this.rows - 1); this.viewport.syncScrollArea(); } From 6e3cd2cb2382ed270550edae98fce8739270823f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 19 Jan 2018 12:34:32 -0800 Subject: [PATCH 5/5] Fix indentation --- src/SelectionManager.ts | 2 +- src/Terminal.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 33585f98..608ca412 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -110,7 +110,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager } private get _buffer(): IBuffer { - return this._terminal.buffers.active; + return this._terminal.buffers.active; } /** diff --git a/src/Terminal.ts b/src/Terminal.ts index 8b38fdbd..31c9a250 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -293,8 +293,8 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT // Create the terminal's buffers and set the current buffer this.buffers = new BufferSet(this); if (this.selectionManager) { - this.selectionManager.clearSelection(); - this.selectionManager.initBuffersListeners(); + this.selectionManager.clearSelection(); + this.selectionManager.initBuffersListeners(); } } @@ -302,7 +302,7 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT * Convenience property to active buffer. */ public get buffer(): Buffer { - return this.buffers.active; + return this.buffers.active; } /**