From 335850380e30c5497b05b4dd7b72d14f9d0f9101 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 4 Jan 2017 08:50:19 -0800 Subject: [PATCH 01/14] Implement DomElementObjectPool with tests --- src/utils/DomElementObjectPool.test.ts | 47 +++++++++++++++++++++++ src/utils/DomElementObjectPool.ts | 53 ++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 src/utils/DomElementObjectPool.test.ts create mode 100644 src/utils/DomElementObjectPool.ts diff --git a/src/utils/DomElementObjectPool.test.ts b/src/utils/DomElementObjectPool.test.ts new file mode 100644 index 00000000..7298f192 --- /dev/null +++ b/src/utils/DomElementObjectPool.test.ts @@ -0,0 +1,47 @@ +import { assert } from 'chai'; +import { DomElementObjectPool } from './DomElementObjectPool'; + +class MockDocument { + private _attr: {[key: string]: string} = {}; + constructor() {} + public getAttribute(key: string): string { return this._attr[key]; }; + public setAttribute(key: string, value: string): void { this._attr[key] = value; } +} + +describe('DomElementObjectPool', () => { + let pool: DomElementObjectPool; + + beforeEach(() => { + pool = new DomElementObjectPool('span'); + (global).document = { + createElement: () => new MockDocument() + }; + }); + + it('should acquire distinct elements', () => { + const element1 = pool.acquire(); + const element2 = pool.acquire(); + assert.notEqual(element1, element2); + }); + + it('should acquire released elements', () => { + const element = pool.acquire(); + pool.release(element); + assert.equal(pool.acquire(), element); + }); + + it('should handle a series of acquisitions and releases', () => { + const element1 = pool.acquire(); + const element2 = pool.acquire(); + pool.release(element1); + assert.equal(pool.acquire(), element1); + pool.release(element1); + pool.release(element2); + assert.equal(pool.acquire(), element2); + assert.equal(pool.acquire(), element1); + }); + + it('should throw when releasing an element that was not acquired', () => { + assert.throws(() => pool.release(document.createElement('span'))); + }); +}); diff --git a/src/utils/DomElementObjectPool.ts b/src/utils/DomElementObjectPool.ts new file mode 100644 index 00000000..8b84fd6b --- /dev/null +++ b/src/utils/DomElementObjectPool.ts @@ -0,0 +1,53 @@ +/** + * @module xterm/utils/DomElementObjectPool + * @license MIT + */ + +/** + * An object pool that manages acquisition and releasing of DOM elements for + * when reuse is desirable. + */ +export class DomElementObjectPool { + private static readonly OBJECT_ID_ATTRIBUTE = 'data-obj-id'; + + private static _objectCount = 0; + + private _type: string; + private _pool: HTMLElement[]; + private _inUse: {[key: string]: HTMLElement}; + + /** + * @param type The DOM element type (div, span, etc.). + */ + constructor(private type: string) { + this._type = type; + this._pool = []; + this._inUse = {}; + } + + public acquire(): HTMLElement { + let element: HTMLElement; + if (this._pool.length === 0) { + element = this.createNew(); + } else { + element = this._pool.pop(); + } + this._inUse[element.getAttribute(DomElementObjectPool.OBJECT_ID_ATTRIBUTE)] = element; + return element; + } + + public release(element: HTMLElement) { + if (!this._inUse[element.getAttribute(DomElementObjectPool.OBJECT_ID_ATTRIBUTE)]) { + throw new Error('Could not release an element not yet acquired'); + } + delete this._inUse[element.getAttribute(DomElementObjectPool.OBJECT_ID_ATTRIBUTE)]; + this._pool.push(element); + } + + private createNew(): HTMLElement { + const element = document.createElement(this._type); + const id = DomElementObjectPool._objectCount++; + element.setAttribute(DomElementObjectPool.OBJECT_ID_ATTRIBUTE, id.toString(10)); + return element; + } +} From 2b4c019f13d52cdc1ff9e43d6eac134aa42d7f63 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 4 Jan 2017 09:32:23 -0800 Subject: [PATCH 02/14] Use object pool in refresh --- src/utils/DomElementObjectPool.ts | 10 ++- src/xterm.js | 114 +++++++++++++++++++++++------- 2 files changed, 96 insertions(+), 28 deletions(-) diff --git a/src/utils/DomElementObjectPool.ts b/src/utils/DomElementObjectPool.ts index 8b84fd6b..3e8a6bad 100644 --- a/src/utils/DomElementObjectPool.ts +++ b/src/utils/DomElementObjectPool.ts @@ -28,7 +28,7 @@ export class DomElementObjectPool { public acquire(): HTMLElement { let element: HTMLElement; if (this._pool.length === 0) { - element = this.createNew(); + element = this._createNew(); } else { element = this._pool.pop(); } @@ -41,13 +41,19 @@ export class DomElementObjectPool { throw new Error('Could not release an element not yet acquired'); } delete this._inUse[element.getAttribute(DomElementObjectPool.OBJECT_ID_ATTRIBUTE)]; + this._cleanElement(element); this._pool.push(element); } - private createNew(): HTMLElement { + private _createNew(): HTMLElement { const element = document.createElement(this._type); const id = DomElementObjectPool._objectCount++; element.setAttribute(DomElementObjectPool.OBJECT_ID_ATTRIBUTE, id.toString(10)); return element; } + + private _cleanElement(element: HTMLElement): void { + element.className = ''; + element.innerHTML = ''; + } } diff --git a/src/xterm.js b/src/xterm.js index 1d220652..ebfd5f20 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -15,6 +15,7 @@ import { EventEmitter } from './EventEmitter.js'; import { Viewport } from './Viewport.js'; import { rightClickHandler, pasteHandler, copyHandler } from './handlers/Clipboard.js'; import { CircularList } from './utils/CircularList.js'; +import { DomElementObjectPool } from './utils/DomElementObjectPool.js'; import * as Browser from './utils/Browser'; import * as Keyboard from './utils/Keyboard'; @@ -189,6 +190,7 @@ function Terminal(options) { this.savedX; this.savedY; this.savedCols; + this.spanElementObjectPool = new DomElementObjectPool('span'); // stream this.readable = true; @@ -1076,6 +1078,8 @@ Terminal.prototype.refresh = function(start, end, queue) { end = this.rows.length - 1; } + var currentElement; + for (; y <= end; y++) { row = y + this.ydisp; @@ -1093,6 +1097,9 @@ Terminal.prototype.refresh = function(start, end, queue) { attr = this.defAttr; i = 0; + var documentFragment = document.createDocumentFragment(); + var innerHTML = ''; + for (; i < width; i++) { data = line[i][0]; ch = line[i][1]; @@ -1104,17 +1111,38 @@ Terminal.prototype.refresh = function(start, end, queue) { if (data !== attr) { if (attr !== this.defAttr) { - out += ''; + if (innerHTML) { + currentElement.innerHTML = innerHTML; + innerHTML = ''; + } + documentFragment.appendChild(currentElement); + currentElement = null; + //out += ''; } if (data !== this.defAttr) { - if (data === -1) { - out += ''; + documentFragment.appendChild(currentElement); + } + currentElement = this.spanElementObjectPool.acquire(); + if (data === -1) { + currentElement.classList.add('reverse-video', 'terminal-cursor'); + //out += ''; } else { - var classNames = []; + //var classNames = []; bg = data & 0x1ff; fg = (data >> 9) & 0x1ff; @@ -1122,18 +1150,21 @@ Terminal.prototype.refresh = function(start, end, queue) { if (flags & Terminal.flags.BOLD) { if (!Terminal.brokenBold) { - classNames.push('xterm-bold'); + currentElement.classList.add('xterm-bold'); + //classNames.push('xterm-bold'); } // See: XTerm*boldColors if (fg < 8) fg += 8; } if (flags & Terminal.flags.UNDERLINE) { - classNames.push('xterm-underline'); + currentElement.classList.add('xterm-underline'); + //classNames.push('xterm-underline'); } if (flags & Terminal.flags.BLINK) { - classNames.push('xterm-blink'); + currentElement.classList.add('xterm-blink'); + //classNames.push('xterm-blink'); } // If inverse flag is on, then swap the foreground and background variables. @@ -1146,7 +1177,8 @@ Terminal.prototype.refresh = function(start, end, queue) { } if (flags & Terminal.flags.INVISIBLE) { - classNames.push('xterm-hidden'); + currentElement.classList.add('xterm-hidden'); + //classNames.push('xterm-hidden'); } /** @@ -1166,37 +1198,44 @@ Terminal.prototype.refresh = function(start, end, queue) { } if (bg < 256) { - classNames.push('xterm-bg-color-' + bg); + currentElement.classList.add('xterm-bg-color-' + bg); + //classNames.push('xterm-bg-color-' + bg); } if (fg < 256) { - classNames.push('xterm-color-' + fg); + currentElement.classList.add('xterm-color-' + fg); + //classNames.push('xterm-color-' + fg); } - out += '': - out += '>'; + innerHTML += '>'; + //out += '>'; break; default: if (ch <= ' ') { - out += ' '; + innerHTML += ' '; + //out += ' '; } else { - out += ch; + innerHTML += ch; + // out += ch; } break; } @@ -1204,11 +1243,34 @@ Terminal.prototype.refresh = function(start, end, queue) { attr = data; } - if (attr !== this.defAttr) { - out += ''; + if (innerHTML && !currentElement) { + currentElement = this.spanElementObjectPool.acquire(); + // For some reason the text nodes only containing   don't get added to the DOM + //currentElement = document.createTextNode(''); + } + if (currentElement) { + if (innerHTML) { + currentElement.innerHTML = innerHTML; + innerHTML = ''; + } + documentFragment.appendChild(currentElement); + currentElement = null; + } + // if (attr !== this.defAttr) { + // out += ''; + // } + + //this.children[y].innerHTML = out; + //this.children[y].innerHTML = ''; + + // Return spans to the pool + while (this.children[y].children.length) { + var child = this.children[y].children[0]; + this.children[y].removeChild(child); + this.spanElementObjectPool.release(child); } - this.children[y].innerHTML = out; + this.children[y].appendChild(documentFragment) } if (parent) { From c7e84981662f5dc4c16de4096da0a1b426c5a6c3 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 4 Jan 2017 09:44:58 -0800 Subject: [PATCH 03/14] Clear row and return spans before generating row --- src/xterm.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/xterm.js b/src/xterm.js index ebfd5f20..3d8bd835 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -1078,8 +1078,6 @@ Terminal.prototype.refresh = function(start, end, queue) { end = this.rows.length - 1; } - var currentElement; - for (; y <= end; y++) { row = y + this.ydisp; @@ -1099,6 +1097,14 @@ Terminal.prototype.refresh = function(start, end, queue) { var documentFragment = document.createDocumentFragment(); var innerHTML = ''; + var currentElement; + + // Return the row's spans to the pool + while (this.children[y].children.length) { + var child = this.children[y].children[0]; + this.children[y].removeChild(child); + this.spanElementObjectPool.release(child); + } for (; i < width; i++) { data = line[i][0]; @@ -1263,13 +1269,6 @@ Terminal.prototype.refresh = function(start, end, queue) { //this.children[y].innerHTML = out; //this.children[y].innerHTML = ''; - // Return spans to the pool - while (this.children[y].children.length) { - var child = this.children[y].children[0]; - this.children[y].removeChild(child); - this.spanElementObjectPool.release(child); - } - this.children[y].appendChild(documentFragment) } From 3baa6b92371efcb86e689747ea89943d262bfe41 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 7 May 2017 16:08:01 -0700 Subject: [PATCH 04/14] Wrap all characters temporarily in spans to force fixed with Fixes #467 --- src/Renderer.ts | 11 +++++++++-- src/xterm.css | 3 ++- src/xterm.js | 4 +++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Renderer.ts b/src/Renderer.ts index 041fd67b..25a04ffc 100644 --- a/src/Renderer.ts +++ b/src/Renderer.ts @@ -283,8 +283,12 @@ export class Renderer { } } + // TODO: Consider performance implications of not pulling these from the pool if (ch_width === 2) { - out += ''; + innerHTML += ''; + } else { + // TODO: Only wrap unicode characters that may vary in width + innerHTML += ''; } switch (ch) { case '&': @@ -310,7 +314,10 @@ export class Renderer { break; } if (ch_width === 2) { - out += ''; + innerHTML += ''; + } else { + // TODO: Only wrap unicode characters that may vary in width + innerHTML += ''; } attr = data; diff --git a/src/xterm.css b/src/xterm.css index a67485e5..efdc0169 100644 --- a/src/xterm.css +++ b/src/xterm.css @@ -153,7 +153,8 @@ overflow-y: scroll; } -.terminal .xterm-wide-char { +.terminal .xterm-wide-char, +.terminal .xterm-normal-char { display: inline-block; } diff --git a/src/xterm.js b/src/xterm.js index 9e9f9ee9..fb5301c3 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -760,7 +760,9 @@ Terminal.loadAddon = function(addon, callback) { * character width has been changed. */ Terminal.prototype.updateCharSizeCSS = function() { - this.charSizeStyleElement.textContent = '.xterm-wide-char{width:' + (this.charMeasure.width * 2) + 'px;}'; + this.charSizeStyleElement.textContent = + '.xterm-wide-char{width:' + (this.charMeasure.width * 2) + 'px;}' + + '.xterm-normal-char{width:' + this.charMeasure.width + 'px;}' } /** From 6c8949bba19aaa001029a17bf55737a5f6d42e6d Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 14 May 2017 12:20:01 -0700 Subject: [PATCH 05/14] Only wrap single width unicode chars --- src/Renderer.ts | 56 ++++++++++++++++++++----------------------------- 1 file changed, 23 insertions(+), 33 deletions(-) diff --git a/src/Renderer.ts b/src/Renderer.ts index 25a04ffc..48da2a17 100644 --- a/src/Renderer.ts +++ b/src/Renderer.ts @@ -283,41 +283,31 @@ export class Renderer { } } - // TODO: Consider performance implications of not pulling these from the pool if (ch_width === 2) { - innerHTML += ''; + // Wrap wide characters so they're sized correctly + innerHTML += `${ch}`; + } else if (ch.charCodeAt(0) > 255) { + // Wrap any non-wide unicode character as some fonts size them badly + innerHTML += `${ch}`; } else { - // TODO: Only wrap unicode characters that may vary in width - innerHTML += ''; - } - switch (ch) { - case '&': - innerHTML += '&'; - //out += '&'; - break; - case '<': - innerHTML += '<'; - //out += '<'; - break; - case '>': - innerHTML += '>'; - //out += '>'; - break; - default: - if (ch <= ' ') { - innerHTML += ' '; - //out += ' '; - } else { - innerHTML += ch; - // out += ch; - } - break; - } - if (ch_width === 2) { - innerHTML += ''; - } else { - // TODO: Only wrap unicode characters that may vary in width - innerHTML += ''; + switch (ch) { + case '&': + innerHTML += '&'; + break; + case '<': + innerHTML += '<'; + break; + case '>': + innerHTML += '>'; + break; + default: + if (ch <= ' ') { + innerHTML += ' '; + } else { + innerHTML += ch; + } + break; + } } attr = data; From c887c6e0f63d9bddfd41416876b03354cc8c9ed7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 14 May 2017 12:27:06 -0700 Subject: [PATCH 06/14] Clean up --- src/Renderer.ts | 61 ++++++++++++------------------------------------- 1 file changed, 15 insertions(+), 46 deletions(-) diff --git a/src/Renderer.ts b/src/Renderer.ts index 48da2a17..2c1923f0 100644 --- a/src/Renderer.ts +++ b/src/Renderer.ts @@ -121,7 +121,7 @@ export class Renderer { * @param {number} end The row to end at (between fromRow and terminal's height terminal - 1) */ private _refresh(start: number, end: number): void { - let x, y, i, line, out, ch, ch_width, width, data, attr, bg, fg, flags, row, parent, focused = document.activeElement; + let parent; // If this is a big refresh, remove the terminal rows from the DOM for faster calculations if (end - start >= this._terminal.rows / 2) { @@ -131,8 +131,8 @@ export class Renderer { } } - width = this._terminal.cols; - y = start; + let width = this._terminal.cols; + let y = start; if (end >= this._terminal.rows) { this._terminal.log('`end` is too large. Most likely a bad CSR.'); @@ -140,11 +140,11 @@ export class Renderer { } for (; y <= end; y++) { - row = y + this._terminal.ydisp; + let row = y + this._terminal.ydisp; - line = this._terminal.lines.get(row); - out = ''; + let line = this._terminal.lines.get(row); + let x; if (this._terminal.y === y - (this._terminal.ybase - this._terminal.ydisp) && this._terminal.cursorState && !this._terminal.cursorHidden) { @@ -153,8 +153,7 @@ export class Renderer { x = -1; } - attr = this._terminal.defAttr; - i = 0; + let attr = this._terminal.defAttr; var documentFragment = document.createDocumentFragment(); var innerHTML = ''; @@ -167,10 +166,11 @@ export class Renderer { this._spanElementObjectPool.release(child); } - for (; i < width; i++) { - data = line[i][0]; - ch = line[i][1]; - ch_width = line[i][2]; + for (let i = 0; i < width; i++) { + // TODO: Could data be a more specific type? + let data: any = line[i][0]; + let ch = line[i][1]; + let ch_width: any = line[i][2]; if (!ch_width) continue; @@ -184,13 +184,10 @@ export class Renderer { } documentFragment.appendChild(currentElement); currentElement = null; - //out += ''; } if (data !== this._terminal.defAttr) { if (innerHTML && !currentElement) { currentElement = this._spanElementObjectPool.acquire(); - // For some reason the text nodes only containing   don't get added to the DOM - //currentElement = document.createTextNode(''); } if (currentElement) { if (innerHTML) { @@ -202,23 +199,14 @@ export class Renderer { currentElement = this._spanElementObjectPool.acquire(); if (data === -1) { currentElement.classList.add('reverse-video', 'terminal-cursor'); - //out += ''; } else { - //var classNames = []; - - bg = data & 0x1ff; - fg = (data >> 9) & 0x1ff; - flags = data >> 18; + let bg = data & 0x1ff; + let fg = (data >> 9) & 0x1ff; + let flags = data >> 18; if (flags & FLAGS.BOLD) { if (!brokenBold) { currentElement.classList.add('xterm-bold'); - //classNames.push('xterm-bold'); } // See: XTerm*boldColors if (fg < 8) fg += 8; @@ -226,12 +214,10 @@ export class Renderer { if (flags & FLAGS.UNDERLINE) { currentElement.classList.add('xterm-underline'); - //classNames.push('xterm-underline'); } if (flags & FLAGS.BLINK) { currentElement.classList.add('xterm-blink'); - //classNames.push('xterm-blink'); } // If inverse flag is on, then swap the foreground and background variables. @@ -245,7 +231,6 @@ export class Renderer { if (flags & FLAGS.INVISIBLE) { currentElement.classList.add('xterm-hidden'); - //classNames.push('xterm-hidden'); } /** @@ -266,19 +251,11 @@ export class Renderer { if (bg < 256) { currentElement.classList.add('xterm-bg-color-' + bg); - //classNames.push('xterm-bg-color-' + bg); } if (fg < 256) { currentElement.classList.add('xterm-color-' + fg); - //classNames.push('xterm-color-' + fg); } - - // out += ' Date: Sun, 14 May 2017 12:40:45 -0700 Subject: [PATCH 07/14] More clean up --- src/Renderer.ts | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/Renderer.ts b/src/Renderer.ts index 2c1923f0..78ca3574 100644 --- a/src/Renderer.ts +++ b/src/Renderer.ts @@ -121,9 +121,8 @@ export class Renderer { * @param {number} end The row to end at (between fromRow and terminal's height terminal - 1) */ private _refresh(start: number, end: number): void { - let parent; - // If this is a big refresh, remove the terminal rows from the DOM for faster calculations + let parent; if (end - start >= this._terminal.rows / 2) { parent = this._terminal.element.parentNode; if (parent) { @@ -145,9 +144,9 @@ export class Renderer { let line = this._terminal.lines.get(row); let x; - if (this._terminal.y === y - (this._terminal.ybase - this._terminal.ydisp) - && this._terminal.cursorState - && !this._terminal.cursorHidden) { + if (this._terminal.y === y - (this._terminal.ybase - this._terminal.ydisp) && + this._terminal.cursorState && + !this._terminal.cursorHidden) { x = this._terminal.x; } else { x = -1; @@ -171,10 +170,13 @@ export class Renderer { let data: any = line[i][0]; let ch = line[i][1]; let ch_width: any = line[i][2]; - if (!ch_width) + if (!ch_width) { continue; + } - if (i === x) data = -1; + if (i === x) { + data = -1; + } if (data !== attr) { if (attr !== this._terminal.defAttr) { @@ -209,7 +211,9 @@ export class Renderer { currentElement.classList.add('xterm-bold'); } // See: XTerm*boldColors - if (fg < 8) fg += 8; + if (fg < 8) { + fg += 8; + } } if (flags & FLAGS.UNDERLINE) { @@ -222,11 +226,13 @@ export class Renderer { // If inverse flag is on, then swap the foreground and background variables. if (flags & FLAGS.INVERSE) { - /* One-line variable swap in JavaScript: http://stackoverflow.com/a/16201730 */ - bg = [fg, fg = bg][0]; - // Should inverse just be before the - // above boldColors effect instead? - if ((flags & 1) && fg < 8) fg += 8; + let temp = bg; + bg = fg; + fg = temp; + // Should inverse just be before the above boldColors effect instead? + if ((flags & 1) && fg < 8) { + fg += 8; + } } if (flags & FLAGS.INVISIBLE) { From 47814847f9348fe33cb4c76ae78845ad9f7187c9 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 14 May 2017 12:47:17 -0700 Subject: [PATCH 08/14] Remove var usages --- src/Renderer.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Renderer.ts b/src/Renderer.ts index 78ca3574..85d57eb9 100644 --- a/src/Renderer.ts +++ b/src/Renderer.ts @@ -154,13 +154,13 @@ export class Renderer { let attr = this._terminal.defAttr; - var documentFragment = document.createDocumentFragment(); - var innerHTML = ''; - var currentElement; + const documentFragment = document.createDocumentFragment(); + let innerHTML = ''; + let currentElement; // Return the row's spans to the pool while (this._terminal.children[y].children.length) { - var child = this._terminal.children[y].children[0]; + const child = this._terminal.children[y].children[0]; this._terminal.children[y].removeChild(child); this._spanElementObjectPool.release(child); } @@ -168,8 +168,8 @@ export class Renderer { for (let i = 0; i < width; i++) { // TODO: Could data be a more specific type? let data: any = line[i][0]; - let ch = line[i][1]; - let ch_width: any = line[i][2]; + const ch = line[i][1]; + const ch_width: any = line[i][2]; if (!ch_width) { continue; } From cf863d484944bc78c9480cb70b7cc8aaaf568a83 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 14 May 2017 14:26:45 -0700 Subject: [PATCH 09/14] Use string interpolation --- src/Renderer.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Renderer.ts b/src/Renderer.ts index 85d57eb9..8f0ca5db 100644 --- a/src/Renderer.ts +++ b/src/Renderer.ts @@ -256,18 +256,19 @@ export class Renderer { } if (bg < 256) { - currentElement.classList.add('xterm-bg-color-' + bg); + currentElement.classList.add(`xterm-bg-color-${bg}`); } if (fg < 256) { - currentElement.classList.add('xterm-color-' + fg); + currentElement.classList.add(`xterm-color-${fg}`); } } } } if (ch_width === 2) { - // Wrap wide characters so they're sized correctly + // Wrap wide characters so they're sized correctly. It's more difficult to release these + // from the object pool so just create new ones via innerHTML. innerHTML += `${ch}`; } else if (ch.charCodeAt(0) > 255) { // Wrap any non-wide unicode character as some fonts size them badly @@ -320,8 +321,7 @@ export class Renderer { } -// if bold is broken, we can't -// use it in the terminal. +// If bold is broken, we can't use it in the terminal. function checkBoldBroken(terminal) { const document = terminal.ownerDocument; const el = document.createElement('span'); From 5fb30979c1b81236c7ab67e499befe65d027d9e3 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 14 May 2017 14:28:06 -0700 Subject: [PATCH 10/14] Add jsdoc back, was lost in merge --- src/xterm.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/xterm.js b/src/xterm.js index fb5301c3..b4bcdcd4 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -1118,6 +1118,11 @@ Terminal.prototype.refresh = function(start, end) { } }; +/** + * Queues linkification for the specified rows. + * @param {number} start The row to start from (between 0 and this.rows - 1). + * @param {number} end The row to end at (between start and this.rows - 1). + */ Terminal.prototype.queueLinkification = function(start, end) { if (this.linkifier) { for (let i = start; i <= end; i++) { From 67ca3a9334bcb7168f744f19a7425461cfc28174 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 May 2017 09:57:50 -0700 Subject: [PATCH 11/14] More string interpolation --- src/xterm.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xterm.js b/src/xterm.js index b4bcdcd4..c632eb70 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -761,8 +761,8 @@ Terminal.loadAddon = function(addon, callback) { */ Terminal.prototype.updateCharSizeCSS = function() { this.charSizeStyleElement.textContent = - '.xterm-wide-char{width:' + (this.charMeasure.width * 2) + 'px;}' + - '.xterm-normal-char{width:' + this.charMeasure.width + 'px;}' + `.xterm-wide-char{width:${this.charMeasure.width * 2}px;}` + + `.xterm-normal-char{width:${this.charMeasure.width}px;}` } /** From 537018f71ff04f14623101ea40b75be43f1098b8 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 May 2017 10:03:56 -0700 Subject: [PATCH 12/14] DomElementObjectPool jsdoc --- src/utils/DomElementObjectPool.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/utils/DomElementObjectPool.ts b/src/utils/DomElementObjectPool.ts index 3e8a6bad..1bfe0026 100644 --- a/src/utils/DomElementObjectPool.ts +++ b/src/utils/DomElementObjectPool.ts @@ -25,6 +25,9 @@ export class DomElementObjectPool { this._inUse = {}; } + /** + * Acquire an element from the pool, creating it if the pool is empty. + */ public acquire(): HTMLElement { let element: HTMLElement; if (this._pool.length === 0) { @@ -36,6 +39,12 @@ export class DomElementObjectPool { return element; } + /** + * Release an element back into the pool. It's up to the caller of this + * function to ensure that all external references to the element have been + * removed. + * @param element The element being released. + */ public release(element: HTMLElement) { if (!this._inUse[element.getAttribute(DomElementObjectPool.OBJECT_ID_ATTRIBUTE)]) { throw new Error('Could not release an element not yet acquired'); @@ -45,6 +54,9 @@ export class DomElementObjectPool { this._pool.push(element); } + /** + * Creates a new element for the pool. + */ private _createNew(): HTMLElement { const element = document.createElement(this._type); const id = DomElementObjectPool._objectCount++; @@ -52,6 +64,10 @@ export class DomElementObjectPool { return element; } + /** + * Resets an element back to a "clean state". + * @param element The element to be cleaned. + */ private _cleanElement(element: HTMLElement): void { element.className = ''; element.innerHTML = ''; From 1a777edffe83beaba18183394ff186e26f77543d Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 May 2017 10:04:12 -0700 Subject: [PATCH 13/14] Add missing return type --- src/utils/DomElementObjectPool.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/DomElementObjectPool.ts b/src/utils/DomElementObjectPool.ts index 1bfe0026..7707ad9a 100644 --- a/src/utils/DomElementObjectPool.ts +++ b/src/utils/DomElementObjectPool.ts @@ -45,7 +45,7 @@ export class DomElementObjectPool { * removed. * @param element The element being released. */ - public release(element: HTMLElement) { + public release(element: HTMLElement): void { if (!this._inUse[element.getAttribute(DomElementObjectPool.OBJECT_ID_ATTRIBUTE)]) { throw new Error('Could not release an element not yet acquired'); } From aad84395427d932e3ce50009ed88175661b4b0ca Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 May 2017 10:07:55 -0700 Subject: [PATCH 14/14] Fix lint errors --- src/Renderer.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Renderer.ts b/src/Renderer.ts index 8f0ca5db..af95d51e 100644 --- a/src/Renderer.ts +++ b/src/Renderer.ts @@ -247,10 +247,10 @@ export class Renderer { * Source: https://github.com/sourcelair/xterm.js/issues/57 */ if (flags & FLAGS.INVERSE) { - if (bg == 257) { + if (bg === 257) { bg = 15; } - if (fg == 256) { + if (fg === 256) { fg = 0; } } @@ -309,7 +309,7 @@ export class Renderer { currentElement = null; } - this._terminal.children[y].appendChild(documentFragment) + this._terminal.children[y].appendChild(documentFragment); } if (parent) {