From 560c8680dfe1f6e04687345f87f23a8994ee8d28 Mon Sep 17 00:00:00 2001 From: Per Bothner Date: Mon, 18 Feb 2019 20:24:12 -0800 Subject: [PATCH] Unify trim/delete/insert events as 'move' events. This solves GitHub issue #1946. --- src/Buffer.ts | 46 +++++++++++++------------------------- src/BufferReflow.ts | 8 +++---- src/SelectionManager.ts | 17 +++++++------- src/SelectionModel.test.ts | 8 +++---- src/SelectionModel.ts | 13 ++++++----- src/common/CircularList.ts | 29 ++++++++++++++---------- src/common/Types.ts | 1 + 7 files changed, 59 insertions(+), 63 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 7f4b6071ca..a12843ab63 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -6,7 +6,7 @@ import { IMarker } from 'xterm'; import { BufferLine } from './BufferLine'; import { reflowLargerApplyNewLayout, reflowLargerCreateNewLayout, reflowLargerGetLinesToRemove, reflowSmallerGetNewLineLengths } from './BufferReflow'; -import { CircularList, IDeleteEvent, IInsertEvent } from './common/CircularList'; +import { CircularList, IMoveEvent } from './common/CircularList'; import { EventEmitter } from './common/EventEmitter'; import { DEFAULT_COLOR } from './renderer/atlas/Types'; import { BufferIndex, CharData, IBuffer, IBufferLine, IBufferStringIterator, IBufferStringIteratorResult, ITerminal } from './Types'; @@ -385,7 +385,7 @@ export class Buffer implements IBuffer { if (toInsert.length > 0) { // Record buffer insert events and then play them back backwards so that the indexes are // correct - const insertEvents: IInsertEvent[] = []; + const insertEvents: IMoveEvent[] = []; // Record original lines so they don't get overridden when we rearrange the list const originalLines: BufferLine[] = []; @@ -409,9 +409,9 @@ export class Buffer implements IBuffer { // Create insert events for later insertEvents.push({ - index: originalLineIndex + 1, - amount: nextToInsert.newLines.length - } as IInsertEvent); + start: originalLineIndex + 1, + amount: - nextToInsert.newLines.length + } as IMoveEvent); countInsertedSoFar += nextToInsert.newLines.length; nextToInsert = toInsert[++nextToInsertIndex]; @@ -423,13 +423,14 @@ export class Buffer implements IBuffer { // Update markers let insertCountEmitted = 0; for (let i = insertEvents.length - 1; i >= 0; i--) { - insertEvents[i].index += insertCountEmitted; - this.lines.emit('insert', insertEvents[i]); + insertEvents[i].start += insertCountEmitted; + this.lines.emit('move', insertEvents[i]); insertCountEmitted += insertEvents[i].amount; } const amountToTrim = Math.max(0, originalLinesLength + countToInsert - this.lines.maxLength); if (amountToTrim > 0) { - this.lines.emitMayRemoveListeners('trim', amountToTrim); + this.lines.emitMayRemoveListeners('trim', amountToTrim); // deprecated + this.lines.emitMayRemoveListeners('move', { start: 0, amount: amountToTrim}); } } } @@ -549,29 +550,14 @@ export class Buffer implements IBuffer { public addMarker(y: number): Marker { const marker = new Marker(y); this.markers.push(marker); - marker.register(this.lines.addDisposableListener('trim', amount => { - marker.line -= amount; - // The marker should be disposed when the line is trimmed from the buffer - if (marker.line < 0) { - marker.dispose(); - } - })); - marker.register(this.lines.addDisposableListener('insert', (event: IInsertEvent) => { - if (marker.line >= event.index) { - marker.line += event.amount; - } - })); - marker.register(this.lines.addDisposableListener('delete', (event: IDeleteEvent) => { - // Delete the marker if it's within the range - if (marker.line >= event.index && marker.line < event.index + event.amount) { - marker.dispose(); - } - - // Shift the marker if it's after the deleted range - if (marker.line > event.index) { + marker.register(this.lines.addDisposableListener('move', (event: IMoveEvent) => { + if (marker.line >= event.start && (! event.end || marker.line < event.end)) { marker.line -= event.amount; - } - })); + if (marker.line < event.start || + (event.end && marker.line >= event.end)) { + marker.dispose(); + } + }})); marker.register(marker.addDisposableListener('dispose', () => this._removeMarker(marker))); return marker; } diff --git a/src/BufferReflow.ts b/src/BufferReflow.ts index 24ab69e6bc..f11442ae08 100644 --- a/src/BufferReflow.ts +++ b/src/BufferReflow.ts @@ -5,7 +5,7 @@ import { FILL_CHAR_DATA } from './Buffer'; import { BufferLine } from './BufferLine'; -import { CircularList, IDeleteEvent } from './common/CircularList'; +import { CircularList, IMoveEvent } from './common/CircularList'; import { IBufferLine } from './Types'; export interface INewLayoutResult { @@ -119,10 +119,10 @@ export function reflowLargerCreateNewLayout(lines: CircularList, to const countToRemove = toRemove[++nextToRemoveIndex]; // Tell markers that there was a deletion - lines.emit('delete', { - index: i - countRemovedSoFar, + lines.emit('move', { + start: i - countRemovedSoFar, amount: countToRemove - } as IDeleteEvent); + } as IMoveEvent); i += countToRemove - 1; countRemovedSoFar += countToRemove; diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index f93328fe6f..21ac0924e6 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -4,7 +4,8 @@ */ import { ITerminal, ISelectionManager, IBuffer, CharData, IBufferLine } from './Types'; -import { XtermListener } from './common/Types'; +import { XtermListener } from './common/Types'; // deprecated +import { IMoveEvent } from './common/CircularList'; import { MouseHelper } from './ui/MouseHelper'; import * as Browser from './core/Platform'; import { CharMeasure } from './ui/CharMeasure'; @@ -102,7 +103,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager private _mouseMoveListener: EventListener; private _mouseUpListener: EventListener; - private _trimListener: XtermListener; + private _moveListener: XtermListener; private _mouseDownTimeStamp: number; @@ -133,13 +134,13 @@ 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._moveListener = (event: IMoveEvent) => this._onMove(event); this.initBuffersListeners(); } public initBuffersListeners(): void { - this._terminal.buffer.lines.on('trim', this._trimListener); + this._terminal.buffer.lines.on('move', this._moveListener); this._terminal.buffers.on('activate', e => this._onBufferActivate(e)); } @@ -335,8 +336,8 @@ export class SelectionManager extends EventEmitter implements ISelectionManager * Handle the buffer being trimmed, adjust the selection position. * @param amount The amount the buffer is being trimmed. */ - private _onTrim(amount: number): void { - const needsRefresh = this._model.onTrim(amount); + private _onMove(event: IMoveEvent): void { + const needsRefresh = this._model.onMove(event); if (needsRefresh) { this.refresh(); } @@ -658,8 +659,8 @@ export class SelectionManager extends EventEmitter implements ISelectionManager // 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); + e.inactiveBuffer.lines.off('move', this._moveListener); + e.activeBuffer.lines.on('move', this._moveListener); } /** diff --git a/src/SelectionModel.test.ts b/src/SelectionModel.test.ts index d49f41d0c7..63e548ebd2 100644 --- a/src/SelectionModel.test.ts +++ b/src/SelectionModel.test.ts @@ -63,21 +63,21 @@ describe('SelectionManager', () => { }); }); - describe('onTrim', () => { + describe('onMove', () => { it('should trim a portion of the selection when a part of it is trimmed', () => { model.selectionStart = [0, 0]; model.selectionEnd = [10, 2]; - model.onTrim(1); + model.onMove({start: 0, amount: 1}); assert.deepEqual(model.finalSelectionStart, [0, 0]); assert.deepEqual(model.finalSelectionEnd, [10, 1]); - model.onTrim(1); + model.onMove({start: 0, amount: 1}); assert.deepEqual(model.finalSelectionStart, [0, 0]); assert.deepEqual(model.finalSelectionEnd, [10, 0]); }); it('should clear selection when it is trimmed in its entirety', () => { model.selectionStart = [0, 0]; model.selectionEnd = [10, 0]; - model.onTrim(1); + model.onMove({start: 0, amount: 1}); assert.deepEqual(model.finalSelectionStart, null); assert.deepEqual(model.finalSelectionEnd, null); }); diff --git a/src/SelectionModel.ts b/src/SelectionModel.ts index f87667f24e..8d1bd58eb9 100644 --- a/src/SelectionModel.ts +++ b/src/SelectionModel.ts @@ -4,6 +4,7 @@ */ import { ITerminal } from './Types'; +import { IMoveEvent } from './common/CircularList'; /** * Represents a selection within the buffer. This model only cares about column @@ -112,13 +113,15 @@ export class SelectionModel { * @param amount The amount the buffer is being trimmed. * @return Whether a refresh is necessary. */ - public onTrim(amount: number): boolean { + public onMove(event: IMoveEvent): boolean { // Adjust the selection position based on the trimmed amount. - if (this.selectionStart) { - this.selectionStart[1] -= amount; + if (this.selectionStart && this.selectionStart[1] >= event.start + && (! event.end || this.selectionStart[1] < event.end)) { + this.selectionStart[1] -= event.amount; } - if (this.selectionEnd) { - this.selectionEnd[1] -= amount; + if (this.selectionEnd && this.selectionEnd[1] >= event.start + && (! event.end || this.selectionEnd[1] < event.end)) { + this.selectionEnd[1] -= event.amount; } // The selection has moved off the buffer, clear it. diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 90891b72ae..062b4b9a3b 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -6,13 +6,13 @@ import { EventEmitter } from './EventEmitter'; import { ICircularList } from './Types'; -export interface IInsertEvent { - index: number; - amount: number; -} - -export interface IDeleteEvent { - index: number; +export interface IMoveEvent { + /** Index of first line affected (including ybase). */ + start: number; + /** Index of (line following the) last line affected. + * This is the old index (pre-insert/delete), and is exclusive. */ + end?: number; + /** Number of lines deleted (if positive) or inserted (if negative). */ amount: number; } @@ -101,7 +101,8 @@ export class CircularList extends EventEmitter implements ICircularList { this._array[this._getCyclicIndex(this._length)] = value; if (this._length === this._maxLength) { this._startIndex = ++this._startIndex % this._maxLength; - this.emitMayRemoveListeners('trim', 1); + this.emitMayRemoveListeners('move', { start: 0, amount: 1 }); + this.emitMayRemoveListeners('trim', 1); // deprecated } else { this._length++; } @@ -117,7 +118,8 @@ export class CircularList extends EventEmitter implements ICircularList { throw new Error('Can only recycle when the buffer is full'); } this._startIndex = ++this._startIndex % this._maxLength; - this.emitMayRemoveListeners('trim', 1); + this.emitMayRemoveListeners('move', { start: 0, amount: 1 }); + this.emitMayRemoveListeners('trim', 1); // deprecated return this._array[this._getCyclicIndex(this._length - 1)]!; } @@ -167,7 +169,8 @@ export class CircularList extends EventEmitter implements ICircularList { const countToTrim = (this._length + items.length) - this._maxLength; this._startIndex += countToTrim; this._length = this._maxLength; - this.emitMayRemoveListeners('trim', countToTrim); + this.emitMayRemoveListeners('move', { start: 0, amount: countToTrim }); + this.emitMayRemoveListeners('trim', countToTrim); // deprecated } else { this._length += items.length; } @@ -183,7 +186,8 @@ export class CircularList extends EventEmitter implements ICircularList { } this._startIndex += count; this._length -= count; - this.emitMayRemoveListeners('trim', count); + this.emitMayRemoveListeners('move', { start: 0, amount: count }); + this.emitMayRemoveListeners('trim', count); // deprecated } public shiftElements(start: number, count: number, offset: number): void { @@ -207,7 +211,8 @@ export class CircularList extends EventEmitter implements ICircularList { while (this._length > this._maxLength) { this._length--; this._startIndex++; - this.emitMayRemoveListeners('trim', 1); + this.emitMayRemoveListeners('move', { start: 0, amount: 1 }); + this.emitMayRemoveListeners('trim', 1); // deprecated } } } else { diff --git a/src/common/Types.ts b/src/common/Types.ts index 8a416bf1ab..45e80da372 100644 --- a/src/common/Types.ts +++ b/src/common/Types.ts @@ -34,4 +34,5 @@ export interface ICircularList extends IEventEmitter { splice(start: number, deleteCount: number, ...items: T[]): void; trimStart(count: number): void; shiftElements(start: number, count: number, offset: number): void; + emitMayRemoveListeners(type: string, ...args: any[]): void; }