From 33a358033ca26fd9d7d54f01127446357fac8b21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 9 Oct 2018 23:48:58 +0200 Subject: [PATCH 01/13] new approach --- src/BufferLine.ts | 11 +++++------ src/Terminal.ts | 17 ++++++++++++----- src/common/CircularList.ts | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/BufferLine.ts b/src/BufferLine.ts index 53f5d4d4a8..4134673e62 100644 --- a/src/BufferLine.ts +++ b/src/BufferLine.ts @@ -94,13 +94,11 @@ export class BufferLine implements IBufferLine { } } - public copyFrom(line: IBufferLine): void { - this._data = []; - for (let i = 0; i < line.length; ++i) { - this._push(line.get(i)); - } + public copyFrom(line: BufferLine): IBufferLine { + this._data = line._data.slice(0); this.length = line.length; this.isWrapped = line.isWrapped; + return this; } public clone(): IBufferLine { @@ -249,7 +247,7 @@ export class BufferLineTypedArray implements IBufferLine { } /** alter to a full copy of line */ - public copyFrom(line: BufferLineTypedArray): void { + public copyFrom(line: BufferLineTypedArray): IBufferLine { if (this.length !== line.length) { this._data = new Uint32Array(line._data); } else { @@ -262,6 +260,7 @@ export class BufferLineTypedArray implements IBufferLine { this._combined[el] = line._combined[el]; } this.isWrapped = line.isWrapped; + return this; } /** create a new clone */ diff --git a/src/Terminal.ts b/src/Terminal.ts index c995af6220..919b039e12 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -21,7 +21,7 @@ * http://linux.die.net/man/7/urxvt */ -import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminalOptions, ITerminal, IBrowser, ILinkifier, ILinkMatcherOptions, CustomKeyEventHandler, LinkMatcherHandler, CharData, CharacterJoinerHandler } from './Types'; +import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminalOptions, ITerminal, IBrowser, ILinkifier, ILinkMatcherOptions, CustomKeyEventHandler, LinkMatcherHandler, CharData, CharacterJoinerHandler, IBufferLine } from './Types'; import { IMouseZoneManager } from './ui/Types'; import { IRenderer } from './renderer/Types'; import { BufferSet } from './BufferSet'; @@ -1174,7 +1174,14 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * @param isWrapped Whether the new line is wrapped from the previous line. */ public scroll(isWrapped?: boolean): void { - const newLine = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); + // TODO: make blank a member + let blank: IBufferLine = (this as any)._blank; + if (!blank || blank.length !== this.cols) { + blank = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); + (this as any)._blank = blank; + } + blank.isWrapped = !!(isWrapped); + const topRow = this.buffer.ybase + this.buffer.scrollTop; const bottomRow = this.buffer.ybase + this.buffer.scrollBottom; @@ -1184,9 +1191,9 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { - this.buffer.lines.push(newLine); + (this.buffer.lines as any).pushRecycling((line: IBufferLine | undefined) => (line) ? line.copyFrom(blank) : blank.clone()); } else { - this.buffer.lines.splice(bottomRow + 1, 0, newLine); + this.buffer.lines.splice(bottomRow + 1, 0, blank.clone()); } // Only adjust ybase and ydisp when the buffer is not trimmed @@ -1208,7 +1215,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // scrollback, instead we can just shift them in-place. const scrollRegionHeight = bottomRow - topRow + 1/*as it's zero-based*/; this.buffer.lines.shiftElements(topRow + 1, scrollRegionHeight - 1, -1); - this.buffer.lines.set(bottomRow, newLine); + this.buffer.lines.set(bottomRow, blank.clone()); } // Move the viewport to the bottom of the buffer unless the user is diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 542dbf12ab..9609960d3a 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -100,6 +100,24 @@ export class CircularList extends EventEmitter implements ICircularList { } } + /** + * Recycling push variant with a callback. + * The callback gets the value at the current position to be overwritten. + * Return the new value from the callback. + */ + public pushRecycling(callback: (el: T | undefined) => T): void { + this._array[this._getCyclicIndex(this._length)] = callback(this._array[this._getCyclicIndex(this._length)]); + if (this._length === this._maxLength) { + this._startIndex++; + if (this._startIndex === this._maxLength) { + this._startIndex = 0; + } + this.emit('trim', 1); + } else { + this._length++; + } + } + /** * Removes and returns the last value on the list. * @return The popped value. From 751e2efa755d7d5e7b45474725676bf0b2d0100c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 10 Oct 2018 15:00:38 +0200 Subject: [PATCH 02/13] better integration and experimentalPushRecycling option --- src/Terminal.ts | 34 ++++++++++++++++++++++++---------- src/Types.ts | 2 +- src/common/CircularList.ts | 2 +- src/common/Types.ts | 1 + typings/xterm.d.ts | 2 ++ 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 919b039e12..ff82098273 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -106,7 +106,8 @@ const DEFAULT_OPTIONS: ITerminalOptions = { theme: null, rightClickSelectsWord: Browser.isMac, rendererType: 'canvas', - experimentalBufferLineImpl: 'JsArray' + experimentalBufferLineImpl: 'JsArray', + experimentalPushRecycling: false }; export class Terminal extends EventEmitter implements ITerminal, IDisposable, IInputHandlingTerminal { @@ -208,6 +209,9 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II private _screenDprMonitor: ScreenDprMonitor; private _theme: ITheme; + // bufferline to clone/copy from for new blank lines + private _blankLine: IBufferLine = null; + public cols: number; public rows: number; @@ -496,6 +500,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II case 'experimentalBufferLineImpl': this.buffers.normal.setBufferLineFactory(value); this.buffers.alt.setBufferLineFactory(value); + this._blankLine = null; break; } // Inform renderer of changes @@ -1174,13 +1179,18 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * @param isWrapped Whether the new line is wrapped from the previous line. */ public scroll(isWrapped?: boolean): void { - // TODO: make blank a member - let blank: IBufferLine = (this as any)._blank; - if (!blank || blank.length !== this.cols) { - blank = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); - (this as any)._blank = blank; + let newLine: IBufferLine; + const useRecycling = this.options.experimentalPushRecycling; + if (useRecycling) { + newLine = this._blankLine; + if (!newLine || newLine.length !== this.cols) { + newLine = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); + this._blankLine = newLine; + } + newLine.isWrapped = !!(isWrapped); + } else { + newLine = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); } - blank.isWrapped = !!(isWrapped); const topRow = this.buffer.ybase + this.buffer.scrollTop; const bottomRow = this.buffer.ybase + this.buffer.scrollBottom; @@ -1191,9 +1201,13 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { - (this.buffer.lines as any).pushRecycling((line: IBufferLine | undefined) => (line) ? line.copyFrom(blank) : blank.clone()); + if (useRecycling) { + this.buffer.lines.pushRecycling((item) => (item) ? item.copyFrom(newLine) : newLine.clone()); + } else { + this.buffer.lines.push(newLine); + } } else { - this.buffer.lines.splice(bottomRow + 1, 0, blank.clone()); + this.buffer.lines.splice(bottomRow + 1, 0, (useRecycling) ? newLine.clone() : newLine); } // Only adjust ybase and ydisp when the buffer is not trimmed @@ -1215,7 +1229,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // scrollback, instead we can just shift them in-place. const scrollRegionHeight = bottomRow - topRow + 1/*as it's zero-based*/; this.buffer.lines.shiftElements(topRow + 1, scrollRegionHeight - 1, -1); - this.buffer.lines.set(bottomRow, blank.clone()); + this.buffer.lines.set(bottomRow, (useRecycling) ? newLine.clone() : newLine); } // Move the viewport to the bottom of the buffer unless the user is diff --git a/src/Types.ts b/src/Types.ts index e857842616..cafe60dbfb 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -522,7 +522,7 @@ export interface IBufferLine { replaceCells(start: number, end: number, fill: CharData): void; resize(cols: number, fill: CharData, shrink?: boolean): void; fill(fillCharData: CharData): void; - copyFrom(line: IBufferLine): void; + copyFrom(line: IBufferLine): IBufferLine; clone(): IBufferLine; } diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 9609960d3a..cc0809ed48 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -105,7 +105,7 @@ export class CircularList extends EventEmitter implements ICircularList { * The callback gets the value at the current position to be overwritten. * Return the new value from the callback. */ - public pushRecycling(callback: (el: T | undefined) => T): void { + public pushRecycling(callback: (item: T | undefined) => T): void { this._array[this._getCyclicIndex(this._length)] = callback(this._array[this._getCyclicIndex(this._length)]); if (this._length === this._maxLength) { this._startIndex++; diff --git a/src/common/Types.ts b/src/common/Types.ts index aabe721eb5..ec6fcfd513 100644 --- a/src/common/Types.ts +++ b/src/common/Types.ts @@ -28,6 +28,7 @@ export interface ICircularList extends IEventEmitter { get(index: number): T | undefined; set(index: number, value: T): void; push(value: T): void; + pushRecycling(callback: (item: T | undefined) => T): void; pop(): T | undefined; splice(start: number, deleteCount: number, ...items: T[]): void; trimStart(count: number): void; diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index b5a8a10900..7fb33a9b30 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -112,6 +112,8 @@ declare module 'xterm' { */ experimentalBufferLineImpl?: 'JsArray' | 'TypedArray'; + experimentalPushRecycling?: boolean; + /** * The font size used to render text. */ From 3f3263cbb04b1e4669b1e7440979f6069ffb092d Mon Sep 17 00:00:00 2001 From: Vladimir Zeifman Date: Sat, 13 Oct 2018 19:10:56 +0300 Subject: [PATCH 03/13] Fix tslint errors for all addons and un-exclude tests for search addon --- src/addons/attach/attach.test.ts | 2 +- src/addons/fit/fit.test.ts | 2 +- src/addons/fullscreen/fullscreen.test.ts | 2 +- src/addons/fullscreen/fullscreen.ts | 11 ++++++----- src/addons/search/search.test.ts | 1 + src/addons/search/tsconfig.json | 3 --- src/addons/terminado/terminado.test.ts | 2 +- src/addons/webLinks/webLinks.test.ts | 2 +- src/addons/winptyCompat/winptyCompat.test.ts | 2 +- src/addons/zmodem/zmodem.test.ts | 2 +- src/addons/zmodem/zmodem.ts | 4 ++-- 11 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/addons/attach/attach.test.ts b/src/addons/attach/attach.test.ts index 018cfb31c8..e280b65686 100644 --- a/src/addons/attach/attach.test.ts +++ b/src/addons/attach/attach.test.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { assert, expect } from 'chai'; +import { assert } from 'chai'; import * as attach from './attach'; diff --git a/src/addons/fit/fit.test.ts b/src/addons/fit/fit.test.ts index 9a6d89fd0e..781b50101e 100644 --- a/src/addons/fit/fit.test.ts +++ b/src/addons/fit/fit.test.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { assert, expect } from 'chai'; +import { assert } from 'chai'; import * as fit from './fit'; diff --git a/src/addons/fullscreen/fullscreen.test.ts b/src/addons/fullscreen/fullscreen.test.ts index bb98bd30d3..6d41bdfda6 100644 --- a/src/addons/fullscreen/fullscreen.test.ts +++ b/src/addons/fullscreen/fullscreen.test.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { assert, expect } from 'chai'; +import { assert } from 'chai'; import * as fullscreen from './fullscreen'; diff --git a/src/addons/fullscreen/fullscreen.ts b/src/addons/fullscreen/fullscreen.ts index 297a7f5bf5..ef9b8ffd0f 100644 --- a/src/addons/fullscreen/fullscreen.ts +++ b/src/addons/fullscreen/fullscreen.ts @@ -11,17 +11,18 @@ import { Terminal } from 'xterm'; * @param fullscreen Toggle fullscreen on (true) or off (false) */ export function toggleFullScreen(term: Terminal, fullscreen: boolean): void { - let fn: string; + let fn: Function; if (typeof fullscreen === 'undefined') { - fn = (term.element.classList.contains('fullscreen')) ? 'remove' : 'add'; + fn = (term.element.classList.contains('fullscreen')) ? + term.element.classList.remove : term.element.classList.add; } else if (!fullscreen) { - fn = 'remove'; + fn = term.element.classList.remove; } else { - fn = 'add'; + fn = term.element.classList.add; } - term.element.classList[fn]('fullscreen'); + fn('fullscreen'); } export function apply(terminalConstructor: typeof Terminal): void { diff --git a/src/addons/search/search.test.ts b/src/addons/search/search.test.ts index 1fe18b6a46..7451d428c7 100644 --- a/src/addons/search/search.test.ts +++ b/src/addons/search/search.test.ts @@ -2,6 +2,7 @@ * Copyright (c) 2018 The xterm.js authors. All rights reserved. * @license MIT */ +declare var require: any; import { assert, expect } from 'chai'; import * as search from './search'; diff --git a/src/addons/search/tsconfig.json b/src/addons/search/tsconfig.json index 9998dc1bb9..c34a0bc504 100644 --- a/src/addons/search/tsconfig.json +++ b/src/addons/search/tsconfig.json @@ -18,8 +18,5 @@ }, "include": [ "**/*.ts" - ], - "exclude": [ - "**/*.test.ts" ] } diff --git a/src/addons/terminado/terminado.test.ts b/src/addons/terminado/terminado.test.ts index 2e4a53c5aa..e46eafdfa7 100644 --- a/src/addons/terminado/terminado.test.ts +++ b/src/addons/terminado/terminado.test.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { assert, expect } from 'chai'; +import { assert } from 'chai'; import * as terminado from './terminado'; diff --git a/src/addons/webLinks/webLinks.test.ts b/src/addons/webLinks/webLinks.test.ts index c84ee1a5ef..014c25e4c7 100644 --- a/src/addons/webLinks/webLinks.test.ts +++ b/src/addons/webLinks/webLinks.test.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { assert, expect } from 'chai'; +import { assert } from 'chai'; import * as webLinks from './webLinks'; diff --git a/src/addons/winptyCompat/winptyCompat.test.ts b/src/addons/winptyCompat/winptyCompat.test.ts index 0c9269edc0..c3a7e479e6 100644 --- a/src/addons/winptyCompat/winptyCompat.test.ts +++ b/src/addons/winptyCompat/winptyCompat.test.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { assert, expect } from 'chai'; +import { assert } from 'chai'; import * as winptyCompat from './winptyCompat'; diff --git a/src/addons/zmodem/zmodem.test.ts b/src/addons/zmodem/zmodem.test.ts index 682e62c808..d0c7c5fb42 100644 --- a/src/addons/zmodem/zmodem.test.ts +++ b/src/addons/zmodem/zmodem.test.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { assert, expect } from 'chai'; +import { assert } from 'chai'; import * as zmodem from './zmodem'; diff --git a/src/addons/zmodem/zmodem.ts b/src/addons/zmodem/zmodem.ts index 23204f0f62..70fc6e98b2 100644 --- a/src/addons/zmodem/zmodem.ts +++ b/src/addons/zmodem/zmodem.ts @@ -34,7 +34,7 @@ import { Terminal } from 'xterm'; * via `detach()` and a re-`attach()`.) */ -let zmodem; +let zmodem: any; export interface IZmodemOptions { noTerminalWriteOutsideSession?: boolean; @@ -44,7 +44,7 @@ function zmodemAttach(ws: WebSocket, opts: IZmodemOptions = {}): void { const term = this; const senderFunc = (octets: ArrayLike) => ws.send(new Uint8Array(octets)); - let zsentry; + let zsentry: any; function shouldWrite(): boolean { return !!zsentry.get_confirmed_session() || !opts.noTerminalWriteOutsideSession; From 82228ad8913d85faeb442c76a9da29ffd0895917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Mon, 22 Oct 2018 18:15:47 +0200 Subject: [PATCH 04/13] trimAndRecycle with external check --- src/Terminal.ts | 7 ++++++- src/common/CircularList.ts | 19 ++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index ff82098273..516aa2a06f 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1202,7 +1202,12 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { if (useRecycling) { - this.buffer.lines.pushRecycling((item) => (item) ? item.copyFrom(newLine) : newLine.clone()); + // this.buffer.lines.pushRecycling((item) => (item) ? item.copyFrom(newLine) : newLine.clone()); + if (willBufferBeTrimmed) { + (this.buffer.lines as any).trimAndRecycle().copyFrom(newLine); + } else { + this.buffer.lines.push(newLine.clone()); + } } else { this.buffer.lines.push(newLine); } diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index cc0809ed48..682b8775d2 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -118,6 +118,15 @@ export class CircularList extends EventEmitter implements ICircularList { } } + public trimAndRecycle(): T | undefined { + this._startIndex++; + if (this._startIndex === this._maxLength) { + this._startIndex = 0; + } + this.emit('trim', 1); + return this._array[this._getCyclicIndex(this._length - 1)]; + } + /** * Removes and returns the last value on the list. * @return The popped value. @@ -154,10 +163,10 @@ export class CircularList extends EventEmitter implements ICircularList { } // Adjust length as needed - if (this._length + items.length > this.maxLength) { - const countToTrim = (this._length + items.length) - this.maxLength; + if (this._length + items.length > this._maxLength) { + const countToTrim = (this._length + items.length) - this._maxLength; this._startIndex += countToTrim; - this._length = this.maxLength; + this._length = this._maxLength; this.emit('trim', countToTrim); } else { this._length += items.length; @@ -196,7 +205,7 @@ export class CircularList extends EventEmitter implements ICircularList { const expandListBy = (start + count + offset) - this._length; if (expandListBy > 0) { this._length += expandListBy; - while (this._length > this.maxLength) { + while (this._length > this._maxLength) { this._length--; this._startIndex++; this.emit('trim', 1); @@ -216,6 +225,6 @@ export class CircularList extends EventEmitter implements ICircularList { * @returns The cyclic index. */ private _getCyclicIndex(index: number): number { - return (this._startIndex + index) % this.maxLength; + return (this._startIndex + index) % this._maxLength; } } From 1585aba5b5c174a7f9dc88395df87ac2180c4b50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Mon, 22 Oct 2018 21:12:51 +0200 Subject: [PATCH 05/13] slightly faster trimAndRecycle --- src/common/CircularList.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 682b8775d2..48394c8561 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -119,10 +119,7 @@ export class CircularList extends EventEmitter implements ICircularList { } public trimAndRecycle(): T | undefined { - this._startIndex++; - if (this._startIndex === this._maxLength) { - this._startIndex = 0; - } + this._startIndex = ++this._startIndex % this._maxLength; this.emit('trim', 1); return this._array[this._getCyclicIndex(this._length - 1)]; } From 93d7a3021d22f1940f1e3ecd58ff663854bfcec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 26 Oct 2018 16:11:47 +0200 Subject: [PATCH 06/13] remove pushRecycling, test case for trimAndRecycle, docs --- src/Terminal.ts | 17 +++++++---------- src/common/CircularList.test.ts | 16 ++++++++++++++++ src/common/CircularList.ts | 23 +++++++---------------- src/common/Types.ts | 2 +- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 9e79f3a863..5b79750c96 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -25,7 +25,7 @@ import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminalOptions import { IMouseZoneManager } from './ui/Types'; import { IRenderer } from './renderer/Types'; import { BufferSet } from './BufferSet'; -import { Buffer, MAX_BUFFER_SIZE, DEFAULT_ATTR, NULL_CELL_CODE, NULL_CELL_WIDTH, NULL_CELL_CHAR } from './Buffer'; +import { Buffer, MAX_BUFFER_SIZE, DEFAULT_ATTR, NULL_CELL_CODE, NULL_CELL_WIDTH, NULL_CELL_CHAR, CHAR_DATA_ATTR_INDEX } from './Buffer'; import { CompositionHelper } from './CompositionHelper'; import { EventEmitter } from './common/EventEmitter'; import { Viewport } from './Viewport'; @@ -1179,23 +1179,19 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * @param isWrapped Whether the new line is wrapped from the previous line. */ public scroll(isWrapped?: boolean): void { -<<<<<<< HEAD let newLine: IBufferLine; const useRecycling = this.options.experimentalPushRecycling; if (useRecycling) { newLine = this._blankLine; - if (!newLine || newLine.length !== this.cols) { - newLine = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); + if (!newLine || newLine.length !== this.cols || newLine.get(0)[CHAR_DATA_ATTR_INDEX] !== this.eraseAttr()) { + newLine = this.buffer.getBlankLine(this.eraseAttr(), isWrapped); this._blankLine = newLine; } newLine.isWrapped = !!(isWrapped); } else { - newLine = this.buffer.getBlankLine(DEFAULT_ATTR, isWrapped); + newLine = this.buffer.getBlankLine(this.eraseAttr(), isWrapped); } -======= - const newLine = this.buffer.getBlankLine(this.eraseAttr(), isWrapped); ->>>>>>> master const topRow = this.buffer.ybase + this.buffer.scrollTop; const bottomRow = this.buffer.ybase + this.buffer.scrollBottom; @@ -1206,9 +1202,10 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { if (useRecycling) { - // this.buffer.lines.pushRecycling((item) => (item) ? item.copyFrom(newLine) : newLine.clone()); if (willBufferBeTrimmed) { - (this.buffer.lines as any).trimAndRecycle().copyFrom(newLine); + // Warning: Never call .trimAndRecycle() without the + // willBufferBeTrimmed guard! + this.buffer.lines.trimAndRecycle().copyFrom(newLine); } else { this.buffer.lines.push(newLine.clone()); } diff --git a/src/common/CircularList.test.ts b/src/common/CircularList.test.ts index 4c07b16c33..5f0419ab91 100644 --- a/src/common/CircularList.test.ts +++ b/src/common/CircularList.test.ts @@ -257,4 +257,20 @@ describe('CircularList', () => { assert.equal(list.get(3), 4); }); }); + describe('trimAndRecycle', function(): void { + it('should return correct element', function(): void { + const list = new CircularList(5); + list.push([0]); + list.push([1]); + list.push([2]); + list.push([3]); + list.push([4]); + assert.equal(list.trimAndRecycle()[0], 0); + assert.equal(list.trimAndRecycle()[0], 1); + assert.equal(list.trimAndRecycle()[0], 2); + assert.equal(list.trimAndRecycle()[0], 3); + assert.equal(list.trimAndRecycle()[0], 4); + assert.equal(list.trimAndRecycle()[0], 0); + }); + }); }); diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 48394c8561..345eb106c9 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -101,23 +101,14 @@ export class CircularList extends EventEmitter implements ICircularList { } /** - * Recycling push variant with a callback. - * The callback gets the value at the current position to be overwritten. - * Return the new value from the callback. + * Recycling trim. + * This is used to recycle buffer lines in Terminal.scroll when + * the list is at maxLength as a push replacement. + * Returns the old line as new one to be recycled. + * Note: There are no bound checks for performance reasons, + * the method is a special optimization for Terminal.scroll, + * do not use it anywhere else. */ - public pushRecycling(callback: (item: T | undefined) => T): void { - this._array[this._getCyclicIndex(this._length)] = callback(this._array[this._getCyclicIndex(this._length)]); - if (this._length === this._maxLength) { - this._startIndex++; - if (this._startIndex === this._maxLength) { - this._startIndex = 0; - } - this.emit('trim', 1); - } else { - this._length++; - } - } - public trimAndRecycle(): T | undefined { this._startIndex = ++this._startIndex % this._maxLength; this.emit('trim', 1); diff --git a/src/common/Types.ts b/src/common/Types.ts index ec6fcfd513..a1c12adfcf 100644 --- a/src/common/Types.ts +++ b/src/common/Types.ts @@ -28,7 +28,7 @@ export interface ICircularList extends IEventEmitter { get(index: number): T | undefined; set(index: number, value: T): void; push(value: T): void; - pushRecycling(callback: (item: T | undefined) => T): void; + trimAndRecycle(): T | undefined; pop(): T | undefined; splice(start: number, deleteCount: number, ...items: T[]): void; trimStart(count: number): void; From fb64c527a58e9c1ba84c2ed46516010c77b320e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 30 Oct 2018 17:40:11 +0100 Subject: [PATCH 07/13] revert to precheck pushWouldTrim --- src/BufferLine.ts | 6 ++---- src/Terminal.ts | 18 ++++++++++-------- src/Types.ts | 2 +- src/common/CircularList.test.ts | 16 ---------------- src/common/CircularList.ts | 20 +++++--------------- src/common/Types.ts | 2 +- typings/xterm.d.ts | 7 ++++++- 7 files changed, 25 insertions(+), 46 deletions(-) diff --git a/src/BufferLine.ts b/src/BufferLine.ts index c1ec26097c..7c69733464 100644 --- a/src/BufferLine.ts +++ b/src/BufferLine.ts @@ -94,11 +94,10 @@ export class BufferLine implements IBufferLine { } } - public copyFrom(line: BufferLine): IBufferLine { + public copyFrom(line: BufferLine): void { this._data = line._data.slice(0); this.length = line.length; this.isWrapped = line.isWrapped; - return this; } public clone(): IBufferLine { @@ -249,7 +248,7 @@ export class BufferLineTypedArray implements IBufferLine { } /** alter to a full copy of line */ - public copyFrom(line: BufferLineTypedArray): IBufferLine { + public copyFrom(line: BufferLineTypedArray): void { if (this.length !== line.length) { this._data = new Uint32Array(line._data); } else { @@ -262,7 +261,6 @@ export class BufferLineTypedArray implements IBufferLine { this._combined[el] = line._combined[el]; } this.isWrapped = line.isWrapped; - return this; } /** create a new clone */ diff --git a/src/Terminal.ts b/src/Terminal.ts index df73ca5843..420db14605 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -107,7 +107,7 @@ const DEFAULT_OPTIONS: ITerminalOptions = { rightClickSelectsWord: Browser.isMac, rendererType: 'canvas', experimentalBufferLineImpl: 'JsArray', - experimentalPushRecycling: false + experimentalBufferLineRecycling: false }; export class Terminal extends EventEmitter implements ITerminal, IDisposable, IInputHandlingTerminal { @@ -1179,16 +1179,16 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * Scroll the terminal down 1 row, creating a blank line. * @param isWrapped Whether the new line is wrapped from the previous line. */ - public scroll(isWrapped?: boolean): void { + public scroll(isWrapped: boolean = false): void { let newLine: IBufferLine; - const useRecycling = this.options.experimentalPushRecycling; + const useRecycling = this.options.experimentalBufferLineRecycling; if (useRecycling) { newLine = this._blankLine; if (!newLine || newLine.length !== this.cols || newLine.get(0)[CHAR_DATA_ATTR_INDEX] !== this.eraseAttr()) { newLine = this.buffer.getBlankLine(this.eraseAttr(), isWrapped); this._blankLine = newLine; } - newLine.isWrapped = !!(isWrapped); + newLine.isWrapped = isWrapped; } else { newLine = this.buffer.getBlankLine(this.eraseAttr(), isWrapped); } @@ -1198,15 +1198,17 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II if (this.buffer.scrollTop === 0) { // Determine whether the buffer is going to be trimmed after insertion. - const willBufferBeTrimmed = this.buffer.lines.length === this.buffer.lines.maxLength; + const willBufferBeTrimmed = this.buffer.lines.pushWouldTrim(); // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { if (useRecycling) { if (willBufferBeTrimmed) { - // Warning: Never call .trimAndRecycle() without the - // willBufferBeTrimmed guard! - this.buffer.lines.trimAndRecycle().copyFrom(newLine); + // push would trim the oldest line in the ringbuffer + // therefore we can recycle it here as the new line + const recycled = this.buffer.lines.get(0); + recycled.copyFrom(newLine); + this.buffer.lines.push(recycled); } else { this.buffer.lines.push(newLine.clone()); } diff --git a/src/Types.ts b/src/Types.ts index cafe60dbfb..e857842616 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -522,7 +522,7 @@ export interface IBufferLine { replaceCells(start: number, end: number, fill: CharData): void; resize(cols: number, fill: CharData, shrink?: boolean): void; fill(fillCharData: CharData): void; - copyFrom(line: IBufferLine): IBufferLine; + copyFrom(line: IBufferLine): void; clone(): IBufferLine; } diff --git a/src/common/CircularList.test.ts b/src/common/CircularList.test.ts index 5f0419ab91..4c07b16c33 100644 --- a/src/common/CircularList.test.ts +++ b/src/common/CircularList.test.ts @@ -257,20 +257,4 @@ describe('CircularList', () => { assert.equal(list.get(3), 4); }); }); - describe('trimAndRecycle', function(): void { - it('should return correct element', function(): void { - const list = new CircularList(5); - list.push([0]); - list.push([1]); - list.push([2]); - list.push([3]); - list.push([4]); - assert.equal(list.trimAndRecycle()[0], 0); - assert.equal(list.trimAndRecycle()[0], 1); - assert.equal(list.trimAndRecycle()[0], 2); - assert.equal(list.trimAndRecycle()[0], 3); - assert.equal(list.trimAndRecycle()[0], 4); - assert.equal(list.trimAndRecycle()[0], 0); - }); - }); }); diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 345eb106c9..00d5a52083 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -90,10 +90,7 @@ export class CircularList extends EventEmitter implements ICircularList { public push(value: T): void { this._array[this._getCyclicIndex(this._length)] = value; if (this._length === this._maxLength) { - this._startIndex++; - if (this._startIndex === this._maxLength) { - this._startIndex = 0; - } + this._startIndex = ++this._startIndex % this._maxLength; this.emit('trim', 1); } else { this._length++; @@ -101,18 +98,11 @@ export class CircularList extends EventEmitter implements ICircularList { } /** - * Recycling trim. - * This is used to recycle buffer lines in Terminal.scroll when - * the list is at maxLength as a push replacement. - * Returns the old line as new one to be recycled. - * Note: There are no bound checks for performance reasons, - * the method is a special optimization for Terminal.scroll, - * do not use it anywhere else. + * Whether a push would trim. + * True when the ringbuffer is full. */ - public trimAndRecycle(): T | undefined { - this._startIndex = ++this._startIndex % this._maxLength; - this.emit('trim', 1); - return this._array[this._getCyclicIndex(this._length - 1)]; + public pushWouldTrim(): boolean { + return this._length === this._maxLength; } /** diff --git a/src/common/Types.ts b/src/common/Types.ts index a1c12adfcf..c866581d65 100644 --- a/src/common/Types.ts +++ b/src/common/Types.ts @@ -28,7 +28,7 @@ export interface ICircularList extends IEventEmitter { get(index: number): T | undefined; set(index: number, value: T): void; push(value: T): void; - trimAndRecycle(): T | undefined; + pushWouldTrim(): boolean; pop(): T | undefined; splice(start: number, deleteCount: number, ...items: T[]): void; trimStart(count: number): void; diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 68e1e18859..8b36dd757e 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -112,7 +112,12 @@ declare module 'xterm' { */ experimentalBufferLineImpl?: 'JsArray' | 'TypedArray'; - experimentalPushRecycling?: boolean; + /** + * (EXPERIMENTAL) Enable recycling of buffer lines. + * + * This option will be removed in the future. + */ + experimentalBufferLineRecycling?: boolean; /** * The font size used to render text. From e9906f9b9e73f2a8ffefcb6ceccb8dd4c20bd864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 7 Nov 2018 00:56:45 +0100 Subject: [PATCH 08/13] compromise between code safety and speed --- src/Terminal.ts | 8 ++------ src/common/CircularList.ts | 20 +++++++++++++++++--- src/common/Types.ts | 3 ++- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 420db14605..13be90ef8a 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1198,17 +1198,13 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II if (this.buffer.scrollTop === 0) { // Determine whether the buffer is going to be trimmed after insertion. - const willBufferBeTrimmed = this.buffer.lines.pushWouldTrim(); + const willBufferBeTrimmed = this.buffer.lines.isFull(); // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { if (useRecycling) { if (willBufferBeTrimmed) { - // push would trim the oldest line in the ringbuffer - // therefore we can recycle it here as the new line - const recycled = this.buffer.lines.get(0); - recycled.copyFrom(newLine); - this.buffer.lines.push(recycled); + this.buffer.lines.recycle().copyFrom(newLine); } else { this.buffer.lines.push(newLine.clone()); } diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 00d5a52083..0a57ece0d6 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -98,10 +98,24 @@ export class CircularList extends EventEmitter implements ICircularList { } /** - * Whether a push would trim. - * True when the ringbuffer is full. + * Advance ringbuffer index and return current element for recycling. + * Note: If the ringbuffer is not full this method will return undefined, + * Either precheck with isFull() or handle the undefined return value accordingly. */ - public pushWouldTrim(): boolean { + public recycle(): T | undefined { + if (this._length === this._maxLength) { + this._startIndex = ++this._startIndex % this._maxLength; + this.emit('trim', 1); + } else { + this._length++; + } + return this._array[this._getCyclicIndex(this._length - 1)]; + } + + /** + * Ringbuffer is at max length. + */ + public isFull(): boolean { return this._length === this._maxLength; } diff --git a/src/common/Types.ts b/src/common/Types.ts index c866581d65..841029ec95 100644 --- a/src/common/Types.ts +++ b/src/common/Types.ts @@ -28,7 +28,8 @@ export interface ICircularList extends IEventEmitter { get(index: number): T | undefined; set(index: number, value: T): void; push(value: T): void; - pushWouldTrim(): boolean; + recycle(): T | undefined; + isFull(): boolean; pop(): T | undefined; splice(start: number, deleteCount: number, ...items: T[]): void; trimStart(count: number): void; From 006869704b41732cae36822b05e978c8884c29ad Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 20 Nov 2018 09:44:10 -0800 Subject: [PATCH 09/13] Add help wanted/good first issue links --- CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b4b3a9f21d..5c389f5b75 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -30,7 +30,8 @@ opening an issue, read these pointers. ## Contributing code -- Make sure you have a [GitHub account](https://github.com/join) +You can find issues to work on my looking at the [help wanted](https://github.com/xtermjs/xterm.js/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22) or [good first issue](https://github.com/xtermjs/xterm.js/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22) issues. It's a good idea to comment on the issue saying that you're taking it, just in case someone else comes along and you duplicate work. Once you have your issue, here are the steps to contribute: + - Fork [xterm.js](https://github.com/sourcelair/xterm.js/) ([how to fork a repo](https://help.github.com/articles/fork-a-repo)) - Get the [xterm.js demo](https://github.com/xtermjs/xterm.js/wiki/Contributing#running-the-demo) running From 3964bb28b5c3fe061f3c4eba43c78635d4737c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 20 Nov 2018 21:49:31 +0100 Subject: [PATCH 10/13] throw error in recycle with buffer not full --- src/common/CircularList.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 0a57ece0d6..5a6a652ff8 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -99,17 +99,16 @@ export class CircularList extends EventEmitter implements ICircularList { /** * Advance ringbuffer index and return current element for recycling. - * Note: If the ringbuffer is not full this method will return undefined, - * Either precheck with isFull() or handle the undefined return value accordingly. + * Note: The buffer must be full for this method to work. + * @throws When the buffer is not full. */ - public recycle(): T | undefined { - if (this._length === this._maxLength) { - this._startIndex = ++this._startIndex % this._maxLength; - this.emit('trim', 1); - } else { - this._length++; + public recycle(): T { + if (this._length !== this._maxLength) { + throw new Error('Can only recycle when the buffer is full'); } - return this._array[this._getCyclicIndex(this._length - 1)]; + this._startIndex = ++this._startIndex % this._maxLength; + this.emit('trim', 1); + return this._array[this._getCyclicIndex(this._length - 1)]!; } /** From 15c4cc8a74c89a9aab02d5277ec66c39aa88664e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Tue, 20 Nov 2018 22:01:34 +0100 Subject: [PATCH 11/13] remove experimental flag for recycling, merge with TypedArray setting --- src/Terminal.ts | 7 +++---- src/common/CircularList.ts | 2 +- src/common/Types.ts | 2 +- typings/xterm.d.ts | 7 ------- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 70dc15d8fd..38eb3d5bd3 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -106,8 +106,7 @@ const DEFAULT_OPTIONS: ITerminalOptions = { theme: null, rightClickSelectsWord: Browser.isMac, rendererType: 'canvas', - experimentalBufferLineImpl: 'JsArray', - experimentalBufferLineRecycling: false + experimentalBufferLineImpl: 'JsArray' }; export class Terminal extends EventEmitter implements ITerminal, IDisposable, IInputHandlingTerminal { @@ -1181,7 +1180,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II */ public scroll(isWrapped: boolean = false): void { let newLine: IBufferLine; - const useRecycling = this.options.experimentalBufferLineRecycling; + const useRecycling = this.options.experimentalBufferLineImpl === 'TypedArray'; if (useRecycling) { newLine = this._blankLine; if (!newLine || newLine.length !== this.cols || newLine.get(0)[CHAR_DATA_ATTR_INDEX] !== this.eraseAttr()) { @@ -1198,7 +1197,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II if (this.buffer.scrollTop === 0) { // Determine whether the buffer is going to be trimmed after insertion. - const willBufferBeTrimmed = this.buffer.lines.isFull(); + const willBufferBeTrimmed = this.buffer.lines.isFull; // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { diff --git a/src/common/CircularList.ts b/src/common/CircularList.ts index 5a6a652ff8..9faf534aef 100644 --- a/src/common/CircularList.ts +++ b/src/common/CircularList.ts @@ -114,7 +114,7 @@ export class CircularList extends EventEmitter implements ICircularList { /** * Ringbuffer is at max length. */ - public isFull(): boolean { + public get isFull(): boolean { return this._length === this._maxLength; } diff --git a/src/common/Types.ts b/src/common/Types.ts index 841029ec95..8a416bf1ab 100644 --- a/src/common/Types.ts +++ b/src/common/Types.ts @@ -24,12 +24,12 @@ export interface IKeyboardEvent { export interface ICircularList extends IEventEmitter { length: number; maxLength: number; + isFull: boolean; get(index: number): T | undefined; set(index: number, value: T): void; push(value: T): void; recycle(): T | undefined; - isFull(): boolean; pop(): T | undefined; splice(start: number, deleteCount: number, ...items: T[]): void; trimStart(count: number): void; diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 8b36dd757e..c6b6b1e59f 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -112,13 +112,6 @@ declare module 'xterm' { */ experimentalBufferLineImpl?: 'JsArray' | 'TypedArray'; - /** - * (EXPERIMENTAL) Enable recycling of buffer lines. - * - * This option will be removed in the future. - */ - experimentalBufferLineRecycling?: boolean; - /** * The font size used to render text. */ From 98a3892e52408afcbc599dc6c7943b9558e1a060 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 22 Nov 2018 09:44:13 -0800 Subject: [PATCH 12/13] Fix typo --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5c389f5b75..e79240275a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -30,7 +30,7 @@ opening an issue, read these pointers. ## Contributing code -You can find issues to work on my looking at the [help wanted](https://github.com/xtermjs/xterm.js/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22) or [good first issue](https://github.com/xtermjs/xterm.js/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22) issues. It's a good idea to comment on the issue saying that you're taking it, just in case someone else comes along and you duplicate work. Once you have your issue, here are the steps to contribute: +You can find issues to work on by looking at the [help wanted](https://github.com/xtermjs/xterm.js/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22) or [good first issue](https://github.com/xtermjs/xterm.js/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22) issues. It's a good idea to comment on the issue saying that you're taking it, just in case someone else comes along and you duplicate work. Once you have your issue, here are the steps to contribute: - Fork [xterm.js](https://github.com/sourcelair/xterm.js/) ([how to fork a repo](https://help.github.com/articles/fork-a-repo)) From 53fe096455968f8d8158939ea50a929cc3b6e5a2 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 22 Nov 2018 10:52:55 -0800 Subject: [PATCH 13/13] Type fn strongly --- src/addons/fullscreen/fullscreen.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/addons/fullscreen/fullscreen.ts b/src/addons/fullscreen/fullscreen.ts index ef9b8ffd0f..4d05e904ad 100644 --- a/src/addons/fullscreen/fullscreen.ts +++ b/src/addons/fullscreen/fullscreen.ts @@ -11,7 +11,7 @@ import { Terminal } from 'xterm'; * @param fullscreen Toggle fullscreen on (true) or off (false) */ export function toggleFullScreen(term: Terminal, fullscreen: boolean): void { - let fn: Function; + let fn: (...tokens: string[]) => void; if (typeof fullscreen === 'undefined') { fn = (term.element.classList.contains('fullscreen')) ?