From 6c3b6b46557001e5cb89c517faeba51967997a91 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 12 Jun 2018 17:54:26 +0200 Subject: [PATCH 01/41] Ensure drawBoldTextInBrightColors redraws screen --- src/Terminal.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Terminal.ts b/src/Terminal.ts index 8a71df96..1f979528 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -474,6 +474,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.charMeasure.measure(this.options); } break; + case 'drawBoldTextInBrightColors': case 'experimentalCharAtlas': case 'enableBold': case 'letterSpacing': From 8dcb4698d364557e9333d2f4d410d5c2b6e0e877 Mon Sep 17 00:00:00 2001 From: Benjamin Raymond Date: Wed, 20 Jun 2018 13:57:42 +0200 Subject: [PATCH 02/41] #1521: now saving and restoring characters attributes when using ANSI escape sequences 'ESC 7' (DECSC) and 'ESC 8' (DECRC) --- src/InputHandler.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/InputHandler.ts b/src/InputHandler.ts index dbf6dfb2..f35ab522 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -1822,6 +1822,7 @@ export class InputHandler implements IInputHandler { public saveCursor(params: number[]): void { this._terminal.buffer.savedX = this._terminal.buffer.x; this._terminal.buffer.savedY = this._terminal.buffer.y; + this._terminal.savedCurAttr = this._terminal.curAttr; } @@ -1833,6 +1834,7 @@ export class InputHandler implements IInputHandler { public restoreCursor(params: number[]): void { this._terminal.buffer.x = this._terminal.buffer.savedX || 0; this._terminal.buffer.y = this._terminal.buffer.savedY || 0; + this._terminal.curAttr = this._terminal.savedCurAttr; } From c345dba591956ce10bc9f1b6661b6cf91c6ce1b8 Mon Sep 17 00:00:00 2001 From: Benjamin Raymond Date: Wed, 20 Jun 2018 13:58:53 +0200 Subject: [PATCH 03/41] #1521: added 'savedCurAttr' property in 'Terminal' class which holds the saved state of the character attributes ('ESC 7' escape sequence code) --- src/Terminal.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Terminal.ts b/src/Terminal.ts index 8a71df96..970423fe 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -198,6 +198,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II public savedCols: number; public curAttr: number; + public savedCurAttr: number; public params: (string | number)[]; public currentParam: string | number; From a1b8f96d50d7dcccc2cbe4c0392dc263f7e9ecd5 Mon Sep 17 00:00:00 2001 From: Benjamin Raymond Date: Wed, 20 Jun 2018 14:04:13 +0200 Subject: [PATCH 04/41] #1521: added default behavior of 'ESC 8' escape sequence (DECRC) when cursor state was never saved before using 'ESC 7' (DECSC) --- src/InputHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/InputHandler.ts b/src/InputHandler.ts index f35ab522..72b3614c 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -1834,7 +1834,7 @@ export class InputHandler implements IInputHandler { public restoreCursor(params: number[]): void { this._terminal.buffer.x = this._terminal.buffer.savedX || 0; this._terminal.buffer.y = this._terminal.buffer.savedY || 0; - this._terminal.curAttr = this._terminal.savedCurAttr; + this._terminal.curAttr = this._terminal.savedCurAttr || 0; } From 285b0b6702f1b9164c18708d897514ccb4585d16 Mon Sep 17 00:00:00 2001 From: 7PH Date: Wed, 20 Jun 2018 14:23:05 +0200 Subject: [PATCH 05/41] #1521: added 'savedCurAttr' attribute to 'MockInputHandlingTerminal' class --- src/utils/TestUtils.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index f4bad717..763eb3e6 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -182,6 +182,7 @@ export class MockInputHandlingTerminal implements IInputHandlingTerminal { wraparoundMode: boolean; bracketedPasteMode: boolean; curAttr: number; + savedCurAttr: number; savedCols: number; x10Mouse: boolean; vt200Mouse: boolean; From 4fecf05dabcfeb99f02c5b86c352b12f899ad194 Mon Sep 17 00:00:00 2001 From: 7PH Date: Wed, 20 Jun 2018 14:31:12 +0200 Subject: [PATCH 06/41] #1521: added unit tests for saving & restoring cursor attributes --- src/InputHandler.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/InputHandler.test.ts b/src/InputHandler.test.ts index 6dc043db..e8099914 100644 --- a/src/InputHandler.test.ts +++ b/src/InputHandler.test.ts @@ -12,18 +12,22 @@ describe('InputHandler', () => { const terminal = new MockInputHandlingTerminal(); terminal.buffer.x = 1; terminal.buffer.y = 2; + terminal.curAttr = 3; const inputHandler = new InputHandler(terminal); // Save cursor position inputHandler.saveCursor([]); assert.equal(terminal.buffer.x, 1); assert.equal(terminal.buffer.y, 2); + assert.equal(terminal.curAttr, 3); // Change cursor position terminal.buffer.x = 10; terminal.buffer.y = 20; + terminal.curAttr = 30; // Restore cursor position inputHandler.restoreCursor([]); assert.equal(terminal.buffer.x, 1); assert.equal(terminal.buffer.y, 2); + assert.equal(terminal.curAttr, 3); }); describe('setCursorStyle', () => { it('should call Terminal.setOption with correct params', () => { From 086b2c04e082d42683277a94b57969ebd281c31c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 20 Jun 2018 23:02:05 +1000 Subject: [PATCH 07/41] Make sure DOM renderer doesn't render cells beyond cols Fixes #1523 --- src/renderer/dom/DomRenderer.ts | 2 +- .../dom/DomRendererRowFactory.test.ts | 31 ++++++++++++------- src/renderer/dom/DomRendererRowFactory.ts | 10 +++++- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts index 5b1813b5..8c335dca 100644 --- a/src/renderer/dom/DomRenderer.ts +++ b/src/renderer/dom/DomRenderer.ts @@ -303,7 +303,7 @@ export class DomRenderer extends EventEmitter implements IRenderer { const row = y + terminal.buffer.ydisp; const lineData = terminal.buffer.lines.get(row); - rowElement.appendChild(this._rowFactory.createRow(lineData, row === cursorAbsoluteY, cursorX, terminal.charMeasure.width)); + rowElement.appendChild(this._rowFactory.createRow(lineData, row === cursorAbsoluteY, cursorX, terminal.charMeasure.width, terminal.cols)); } this._terminal.emit('refresh', {start, end}); diff --git a/src/renderer/dom/DomRendererRowFactory.test.ts b/src/renderer/dom/DomRendererRowFactory.test.ts index f28830ce..e91c71fe 100644 --- a/src/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/renderer/dom/DomRendererRowFactory.test.ts @@ -23,7 +23,7 @@ describe('DomRendererRowFactory', () => { describe('createRow', () => { it('should create an element for every character in the row', () => { - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), ' ' + ' ' @@ -34,24 +34,33 @@ describe('DomRendererRowFactory', () => { lineData[0] = [DEFAULT_ATTR, '語', 2, '語'.charCodeAt(0)]; // There should be no element for the following "empty" cell lineData[1] = [DEFAULT_ATTR, '', 0, undefined]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), '語' ); }); it('should add class for cursor', () => { - const fragment = rowFactory.createRow(lineData, true, 0, 5); + const fragment = rowFactory.createRow(lineData, true, 0, 5, 20); assert.equal(getFragmentHtml(fragment), ' ' + ' ' ); }); + it('should not render cells that go beyond the terminal\'s columns', () => { + lineData[0] = [DEFAULT_ATTR, 'a', 1, 'a'.charCodeAt(0)]; + lineData[1] = [DEFAULT_ATTR, 'b', 1, 'b'.charCodeAt(0)]; + const fragment = rowFactory.createRow(lineData, false, 0, 5, 1); + assert.equal(getFragmentHtml(fragment), + 'a' + ); + }); + describe('attributes', () => { it('should add class for bold', () => { lineData[0] = [DEFAULT_ATTR | (FLAGS.BOLD << 18), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -60,7 +69,7 @@ describe('DomRendererRowFactory', () => { it('should add class for italic', () => { lineData[0] = [DEFAULT_ATTR | (FLAGS.ITALIC << 18), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -71,7 +80,7 @@ describe('DomRendererRowFactory', () => { const defaultAttrNoFgColor = (0 << 9) | (256 << 0); for (let i = 0; i < 256; i++) { lineData[0] = [defaultAttrNoFgColor | (i << 9), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' @@ -83,7 +92,7 @@ describe('DomRendererRowFactory', () => { const defaultAttrNoBgColor = (257 << 9) | (0 << 0); for (let i = 0; i < 256; i++) { lineData[0] = [defaultAttrNoBgColor | (i << 0), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' @@ -93,7 +102,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert colors', () => { lineData[0] = [(FLAGS.INVERSE << 18) | (2 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -102,7 +111,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert default fg color', () => { lineData[0] = [(FLAGS.INVERSE << 18) | (257 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -111,7 +120,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert default bg color', () => { lineData[0] = [(FLAGS.INVERSE << 18) | (1 << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -121,7 +130,7 @@ describe('DomRendererRowFactory', () => { it('should turn bold fg text bright', () => { for (let i = 0; i < 8; i++) { lineData[0] = [(FLAGS.BOLD << 18) | (i << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' diff --git a/src/renderer/dom/DomRendererRowFactory.ts b/src/renderer/dom/DomRendererRowFactory.ts index d72629a3..eedb1d34 100644 --- a/src/renderer/dom/DomRendererRowFactory.ts +++ b/src/renderer/dom/DomRendererRowFactory.ts @@ -17,9 +17,16 @@ export class DomRendererRowFactory { ) { } - public createRow(lineData: LineData, isCursorRow: boolean, cursorX: number, cellWidth: number): DocumentFragment { + public createRow(lineData: LineData, isCursorRow: boolean, cursorX: number, cellWidth: number, cols: number): DocumentFragment { const fragment = this._document.createDocumentFragment(); + let colCount = 0; + for (let x = 0; x < lineData.length; x++) { + // Don't allow any buffer to the right to be displayed + if (colCount >= cols) { + continue; + } + const charData = lineData[x]; const char: string = charData[CHAR_DATA_CHAR_INDEX]; const attr: number = charData[CHAR_DATA_ATTR_INDEX]; @@ -76,6 +83,7 @@ export class DomRendererRowFactory { charElement.classList.add(`xterm-bg-${bg}`); } fragment.appendChild(charElement); + colCount += width; } return fragment; } From c0f4a8eb7ce638f4a44d3cb390dd26fd2341655b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 10:03:53 +1000 Subject: [PATCH 08/41] Move Dom listener helper to ui/ Part of #1518 --- src/AccessibilityManager.ts | 2 +- src/Terminal.ts | 2 +- src/{utils/Dom.ts => ui/Lifecycle.ts} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/{utils/Dom.ts => ui/Lifecycle.ts} (100%) diff --git a/src/AccessibilityManager.ts b/src/AccessibilityManager.ts index ea300767..74b5ab9a 100644 --- a/src/AccessibilityManager.ts +++ b/src/AccessibilityManager.ts @@ -7,7 +7,7 @@ import * as Strings from './Strings'; import { ITerminal, IBuffer } from './Types'; import { isMac } from './shared/utils/Browser'; import { RenderDebouncer } from './utils/RenderDebouncer'; -import { addDisposableListener } from './utils/Dom'; +import { addDisposableListener } from './ui/Lifecycle'; import { IDisposable } from 'xterm'; const MAX_ROWS_TO_READ = 20; diff --git a/src/Terminal.ts b/src/Terminal.ts index 06f9defc..38d152e5 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -38,7 +38,7 @@ import { Linkifier } from './Linkifier'; import { SelectionManager } from './SelectionManager'; import { CharMeasure } from './utils/CharMeasure'; import * as Browser from './shared/utils/Browser'; -import * as Dom from './utils/Dom'; +import * as Dom from './ui/Lifecycle'; import * as Strings from './Strings'; import { MouseHelper } from './utils/MouseHelper'; import { clone } from './utils/Clone'; diff --git a/src/utils/Dom.ts b/src/ui/Lifecycle.ts similarity index 100% rename from src/utils/Dom.ts rename to src/ui/Lifecycle.ts From a44f496ec7c10c33997240dafb79f1046e1cb381 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 10:05:40 +1000 Subject: [PATCH 09/41] Rename dom listener function for better clarity --- src/AccessibilityManager.ts | 4 ++-- src/Terminal.ts | 2 +- src/ui/Lifecycle.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/AccessibilityManager.ts b/src/AccessibilityManager.ts index 74b5ab9a..6ff595ce 100644 --- a/src/AccessibilityManager.ts +++ b/src/AccessibilityManager.ts @@ -7,7 +7,7 @@ import * as Strings from './Strings'; import { ITerminal, IBuffer } from './Types'; import { isMac } from './shared/utils/Browser'; import { RenderDebouncer } from './utils/RenderDebouncer'; -import { addDisposableListener } from './ui/Lifecycle'; +import { addDisposableDomListener } from './ui/Lifecycle'; import { IDisposable } from 'xterm'; const MAX_ROWS_TO_READ = 20; @@ -89,7 +89,7 @@ export class AccessibilityManager implements IDisposable { this._disposables.push(this._terminal.renderer.addDisposableListener('resize', () => this._refreshRowsDimensions())); // This shouldn't be needed on modern browsers but is present in case the // media query that drives the dprchange event isn't supported - this._disposables.push(addDisposableListener(window, 'resize', () => this._refreshRowsDimensions())); + this._disposables.push(addDisposableDomListener(window, 'resize', () => this._refreshRowsDimensions())); } public dispose(): void { diff --git a/src/Terminal.ts b/src/Terminal.ts index 38d152e5..55f3b493 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -688,7 +688,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.on('dprchange', () => this.renderer.onWindowResize(window.devicePixelRatio)); // dprchange should handle this case, we need this as well for browsers that don't support the // matchMedia query. - this._disposables.push(Dom.addDisposableListener(window, 'resize', () => this.renderer.onWindowResize(window.devicePixelRatio))); + this._disposables.push(Dom.addDisposableDomListener(window, 'resize', () => this.renderer.onWindowResize(window.devicePixelRatio))); this.charMeasure.on('charsizechanged', () => this.renderer.onCharSizeChanged()); this.renderer.on('resize', (dimensions) => this.viewport.syncScrollArea()); diff --git a/src/ui/Lifecycle.ts b/src/ui/Lifecycle.ts index 889c88c2..d8367113 100644 --- a/src/ui/Lifecycle.ts +++ b/src/ui/Lifecycle.ts @@ -10,7 +10,7 @@ import { IDisposable } from 'xterm'; * @param type The event type. * @param handler The handler for the listener. */ -export function addDisposableListener( +export function addDisposableDomListener( node: Element | Window | Document, type: string, handler: (e: any) => void, From dfd17be8bb34662291cc299f8d5510febece4ae4 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 10:24:37 +1000 Subject: [PATCH 10/41] Introduce Disposable abstract class --- src/Buffer.ts | 8 +++----- src/EventEmitter.ts | 5 ++++- src/Terminal.ts | 6 +----- src/common/Lifecycle.ts | 22 ++++++++++++++++++++++ 4 files changed, 30 insertions(+), 11 deletions(-) create mode 100644 src/common/Lifecycle.ts diff --git a/src/Buffer.ts b/src/Buffer.ts index 5ac2a4f2..78bcae27 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -6,7 +6,7 @@ import { CircularList } from './utils/CircularList'; import { LineData, CharData, ITerminal, IBuffer } from './Types'; import { EventEmitter } from './EventEmitter'; -import { IDisposable, IMarker } from 'xterm'; +import { IMarker } from 'xterm'; export const DEFAULT_ATTR = (0 << 18) | (257 << 9) | (256 << 0); export const CHAR_DATA_ATTR_INDEX = 0; @@ -320,7 +320,7 @@ export class Buffer implements IBuffer { public addMarker(y: number): Marker { const marker = new Marker(y); this.markers.push(marker); - marker.disposables.push(this.lines.addDisposableListener('trim', amount => { + 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) { @@ -342,7 +342,6 @@ export class Marker extends EventEmitter implements IMarker { private _id: number = Marker._nextId++; public isDisposed: boolean = false; - public disposables: IDisposable[] = []; public get id(): number { return this._id; } @@ -357,8 +356,7 @@ export class Marker extends EventEmitter implements IMarker { return; } this.isDisposed = true; - this.disposables.forEach(d => d.dispose()); - this.disposables.length = 0; + super.dispose(); this.emit('dispose'); } } diff --git a/src/EventEmitter.ts b/src/EventEmitter.ts index 9ce31bb5..1970f3e6 100644 --- a/src/EventEmitter.ts +++ b/src/EventEmitter.ts @@ -5,11 +5,13 @@ import { XtermListener } from './Types'; import { IEventEmitter, IDisposable } from 'xterm'; +import { Disposable } from './common/Lifecycle'; -export class EventEmitter implements IEventEmitter, IDisposable { +export class EventEmitter extends Disposable implements IEventEmitter, IDisposable { private _events: {[type: string]: XtermListener[]}; constructor() { + super(); // Restore the previous events if available, this will happen if the // constructor is called multiple times on the same object (terminal reset). this._events = this._events || {}; @@ -26,6 +28,7 @@ export class EventEmitter implements IEventEmitter, IDisposable { * @param handler The handler for the listener. */ public addDisposableListener(type: string, handler: XtermListener): IDisposable { + // TODO: Rename addDisposableEventListener to more easily disambiguate from Dom listener this.on(type, handler); return { dispose: () => { diff --git a/src/Terminal.ts b/src/Terminal.ts index 55f3b493..05f47de2 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -113,8 +113,6 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II public element: HTMLElement; public screenElement: HTMLElement; - private _disposables: IDisposable[]; - /** * The HTMLElement that the terminal is created in, set by Terminal.open. */ @@ -252,8 +250,6 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } private _setup(): void { - this._disposables = []; - Object.keys(DEFAULT_OPTIONS).forEach((key) => { if (this.options[key] == null) { this.options[key] = DEFAULT_OPTIONS[key]; @@ -688,7 +684,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.on('dprchange', () => this.renderer.onWindowResize(window.devicePixelRatio)); // dprchange should handle this case, we need this as well for browsers that don't support the // matchMedia query. - this._disposables.push(Dom.addDisposableDomListener(window, 'resize', () => this.renderer.onWindowResize(window.devicePixelRatio))); + this.register(Dom.addDisposableDomListener(window, 'resize', () => this.renderer.onWindowResize(window.devicePixelRatio))); this.charMeasure.on('charsizechanged', () => this.renderer.onCharSizeChanged()); this.renderer.on('resize', (dimensions) => this.viewport.syncScrollArea()); diff --git a/src/common/Lifecycle.ts b/src/common/Lifecycle.ts new file mode 100644 index 00000000..f83f09af --- /dev/null +++ b/src/common/Lifecycle.ts @@ -0,0 +1,22 @@ +/** + * Copyright (c) 2018 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { IDisposable } from 'xterm'; + +export abstract class Disposable implements IDisposable { + protected _disposables: IDisposable[] = []; + + constructor() { + } + + public dispose(): void { + this._disposables.forEach(d => d.dispose()); + this._disposables.length = 0; + } + + public register(t: T): void { + this._disposables.push(t); + } +} From 7e779cd19565883b0f93877b0edcc70d393d2da1 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 10:29:02 +1000 Subject: [PATCH 11/41] Replace _disposables with Disposable, add jsdoc --- src/AccessibilityManager.ts | 34 ++++++++++++++++------------------ src/EventEmitter.ts | 1 + src/Terminal.ts | 2 -- src/common/Lifecycle.ts | 15 +++++++++++++-- 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/AccessibilityManager.ts b/src/AccessibilityManager.ts index 6ff595ce..282a5e5a 100644 --- a/src/AccessibilityManager.ts +++ b/src/AccessibilityManager.ts @@ -8,7 +8,7 @@ import { ITerminal, IBuffer } from './Types'; import { isMac } from './shared/utils/Browser'; import { RenderDebouncer } from './utils/RenderDebouncer'; import { addDisposableDomListener } from './ui/Lifecycle'; -import { IDisposable } from 'xterm'; +import { Disposable } from './common/Lifecycle'; const MAX_ROWS_TO_READ = 20; @@ -17,7 +17,7 @@ const enum BoundaryPosition { BOTTOM } -export class AccessibilityManager implements IDisposable { +export class AccessibilityManager extends Disposable { private _accessibilityTreeRoot: HTMLElement; private _rowContainer: HTMLElement; private _rowElements: HTMLElement[]; @@ -29,8 +29,6 @@ export class AccessibilityManager implements IDisposable { private _topBoundaryFocusListener: (e: FocusEvent) => void; private _bottomBoundaryFocusListener: (e: FocusEvent) => void; - private _disposables: IDisposable[] = []; - /** * This queue has a character pushed to it for keys that are pressed, if the * next character added to the terminal is equal to the key char then it is @@ -43,6 +41,7 @@ export class AccessibilityManager implements IDisposable { private _charsToConsume: string[] = []; constructor(private _terminal: ITerminal) { + super(); this._accessibilityTreeRoot = document.createElement('div'); this._accessibilityTreeRoot.classList.add('xterm-accessibility'); @@ -72,29 +71,28 @@ export class AccessibilityManager implements IDisposable { this._terminal.element.insertAdjacentElement('afterbegin', this._accessibilityTreeRoot); - this._disposables.push(this._renderRowsDebouncer); - this._disposables.push(this._terminal.addDisposableListener('resize', data => this._onResize(data.cols, data.rows))); - this._disposables.push(this._terminal.addDisposableListener('refresh', data => this._refreshRows(data.start, data.end))); - this._disposables.push(this._terminal.addDisposableListener('scroll', data => this._refreshRows())); + this.register(this._renderRowsDebouncer); + this.register(this._terminal.addDisposableListener('resize', data => this._onResize(data.cols, data.rows))); + this.register(this._terminal.addDisposableListener('refresh', data => this._refreshRows(data.start, data.end))); + this.register(this._terminal.addDisposableListener('scroll', data => this._refreshRows())); // Line feed is an issue as the prompt won't be read out after a command is run - this._disposables.push(this._terminal.addDisposableListener('a11y.char', (char) => this._onChar(char))); - this._disposables.push(this._terminal.addDisposableListener('linefeed', () => this._onChar('\n'))); - this._disposables.push(this._terminal.addDisposableListener('a11y.tab', spaceCount => this._onTab(spaceCount))); - this._disposables.push(this._terminal.addDisposableListener('key', keyChar => this._onKey(keyChar))); - this._disposables.push(this._terminal.addDisposableListener('blur', () => this._clearLiveRegion())); + this.register(this._terminal.addDisposableListener('a11y.char', (char) => this._onChar(char))); + this.register(this._terminal.addDisposableListener('linefeed', () => this._onChar('\n'))); + this.register(this._terminal.addDisposableListener('a11y.tab', spaceCount => this._onTab(spaceCount))); + this.register(this._terminal.addDisposableListener('key', keyChar => this._onKey(keyChar))); + this.register(this._terminal.addDisposableListener('blur', () => this._clearLiveRegion())); // TODO: Maybe renderer should fire an event on terminal when the characters change and that // should be listened to instead? That would mean that the order of events are always // guarenteed - this._disposables.push(this._terminal.addDisposableListener('dprchange', () => this._refreshRowsDimensions())); - this._disposables.push(this._terminal.renderer.addDisposableListener('resize', () => this._refreshRowsDimensions())); + this.register(this._terminal.addDisposableListener('dprchange', () => this._refreshRowsDimensions())); + this.register(this._terminal.renderer.addDisposableListener('resize', () => this._refreshRowsDimensions())); // This shouldn't be needed on modern browsers but is present in case the // media query that drives the dprchange event isn't supported - this._disposables.push(addDisposableDomListener(window, 'resize', () => this._refreshRowsDimensions())); + this.register(addDisposableDomListener(window, 'resize', () => this._refreshRowsDimensions())); } public dispose(): void { - this._disposables.forEach(d => d.dispose()); - this._disposables.length = 0; + super.dispose(); this._terminal.element.removeChild(this._accessibilityTreeRoot); this._rowElements.length = 0; } diff --git a/src/EventEmitter.ts b/src/EventEmitter.ts index 1970f3e6..0de5999a 100644 --- a/src/EventEmitter.ts +++ b/src/EventEmitter.ts @@ -79,6 +79,7 @@ export class EventEmitter extends Disposable implements IEventEmitter, IDisposab } public dispose(): void { + super.dispose(); this._events = {}; } } diff --git a/src/Terminal.ts b/src/Terminal.ts index 05f47de2..ab703aa2 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -232,8 +232,6 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II public dispose(): void { super.dispose(); - this._disposables.forEach(d => d.dispose()); - this._disposables.length = 0; removeTerminalFromCache(this); this.handler = () => {}; this.write = () => {}; diff --git a/src/common/Lifecycle.ts b/src/common/Lifecycle.ts index f83f09af..b9cf1418 100644 --- a/src/common/Lifecycle.ts +++ b/src/common/Lifecycle.ts @@ -5,18 +5,29 @@ import { IDisposable } from 'xterm'; +/** + * A base class that can be extended to provide convenience methods for managing the lifecycle of an + * object and its components. + */ export abstract class Disposable implements IDisposable { protected _disposables: IDisposable[] = []; constructor() { } + /** + * Disposes the object, triggering the `dispose` method on all registered IDisposables. + */ public dispose(): void { this._disposables.forEach(d => d.dispose()); this._disposables.length = 0; } - public register(t: T): void { - this._disposables.push(t); + /** + * Registers a disposable object. + * @param d The disposable to register. + */ + public register(d: T): void { + this._disposables.push(d); } } From 9293616f0296816dd7a6b8809228de633f18a6e0 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 10:32:58 +1000 Subject: [PATCH 12/41] Add dispose terminal button to demo --- demo/index.html | 1 + demo/main.js | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/demo/index.html b/demo/index.html index f374d998..fff77d09 100644 --- a/demo/index.html +++ b/demo/index.html @@ -32,6 +32,7 @@

Attention: The demo is a barebones implementation and is designed for the development and evaluation of xterm.js only. Exposing the demo to the public as is would introduce security risks for the host.

+ diff --git a/demo/main.js b/demo/main.js index 1ed6a180..004a48f0 100644 --- a/demo/main.js +++ b/demo/main.js @@ -76,6 +76,12 @@ function createTerminal() { term.fit(); term.focus(); + document.getElementById('dispose').addEventListener('click', () => { + term.dispose(); + term = null; + window.term = null; + }); + // fit is called within a setTimeout, cols and rows need this. setTimeout(function () { initOptions(term); From 5adf962eb5b7fe6f07141dec3d137c42ff2c811d Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 10:39:52 +1000 Subject: [PATCH 13/41] Dispose of attach addon event listeners --- src/addons/attach/Interfaces.ts | 6 +++++- src/addons/attach/attach.ts | 22 ++++++++++++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/addons/attach/Interfaces.ts b/src/addons/attach/Interfaces.ts index ab809af9..ab5846f5 100644 --- a/src/addons/attach/Interfaces.ts +++ b/src/addons/attach/Interfaces.ts @@ -5,9 +5,13 @@ * Implements the attach method, that attaches the terminal to a WebSocket stream. */ -import { Terminal } from 'xterm'; +import { Terminal, IDisposable } from 'xterm'; export interface IAttachAddonTerminal extends Terminal { + _core: { + register(d: T): void; + }; + __socket?: WebSocket; __attachSocketBuffer?: string; diff --git a/src/addons/attach/attach.ts b/src/addons/attach/attach.ts index e6d92b56..e94c0b46 100644 --- a/src/addons/attach/attach.ts +++ b/src/addons/attach/attach.ts @@ -5,7 +5,7 @@ * Implements the attach method, that attaches the terminal to a WebSocket stream. */ -import { Terminal } from 'xterm'; +import { Terminal, IDisposable } from 'xterm'; import { IAttachAddonTerminal } from './Interfaces'; /** @@ -87,14 +87,28 @@ export function attach(term: Terminal, socket: WebSocket, bidirectional: boolean socket.send(data); }; - socket.addEventListener('message', addonTerminal.__getMessage); + addonTerminal._core.register(addSocketListener(socket, 'message', addonTerminal.__getMessage)); if (bidirectional) { addonTerminal.on('data', addonTerminal.__sendData); } - socket.addEventListener('close', () => detach(addonTerminal, socket)); - socket.addEventListener('error', () => detach(addonTerminal, socket)); + addonTerminal._core.register(addSocketListener(socket, 'close', () => detach(addonTerminal, socket))); + addonTerminal._core.register(addSocketListener(socket, 'error', () => detach(addonTerminal, socket))); +} + +function addSocketListener(socket: WebSocket, type: string, handler: (this: WebSocket, ev: Event) => any): IDisposable { + socket.addEventListener(type, handler); + return { + dispose: () => { + if (!handler) { + // Already disposed + return; + } + socket.removeEventListener(type, handler); + handler = null; + } + }; } /** From ede1d5f7c8636d728ede80049e5e7dfa7de3e722 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 10:49:03 +1000 Subject: [PATCH 14/41] Correctly dispose of Renderer and ScreenDprMonitor --- src/Terminal.ts | 2 ++ src/renderer/Renderer.ts | 2 ++ src/renderer/Types.ts | 5 +++-- src/utils/ScreenDprMonitor.ts | 9 ++++++++- src/utils/TestUtils.test.ts | 3 +++ 5 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index ab703aa2..51c3f189 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -611,6 +611,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this._screenDprMonitor = new ScreenDprMonitor(); this._screenDprMonitor.setListener(() => this.emit('dprchange', window.devicePixelRatio)); + this.register(this._screenDprMonitor); // Create main element container this.element = this._document.createElement('div'); @@ -671,6 +672,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II case 'dom': this.renderer = new DomRenderer(this, this.options.theme); break; default: throw new Error(`Unrecognized rendererType "${this.options.rendererType}"`); } + this.register(this.renderer); this.options.theme = null; this.viewport = new Viewport(this, this._viewportElement, this._viewportScrollArea, this.charMeasure); this.viewport.onThemeChanged(this.renderer.colorManager.colors); diff --git a/src/renderer/Renderer.ts b/src/renderer/Renderer.ts index c41dece5..b6f4f640 100644 --- a/src/renderer/Renderer.ts +++ b/src/renderer/Renderer.ts @@ -62,12 +62,14 @@ export class Renderer extends EventEmitter implements IRenderer { this._renderDebouncer = new RenderDebouncer(this._terminal, this._renderRows.bind(this)); this._screenDprMonitor = new ScreenDprMonitor(); this._screenDprMonitor.setListener(() => this.onWindowResize(window.devicePixelRatio)); + this.register(this._screenDprMonitor); // Detect whether IntersectionObserver is detected and enable renderer pause // and resume based on terminal visibility if so if ('IntersectionObserver' in window) { const observer = new IntersectionObserver(e => this.onIntersectionChange(e[0]), {threshold: 0}); observer.observe(this._terminal.element); + this.register({ dispose: () => observer.disconnect() }); } } diff --git a/src/renderer/Types.ts b/src/renderer/Types.ts index bafc9ad4..e7054c13 100644 --- a/src/renderer/Types.ts +++ b/src/renderer/Types.ts @@ -4,7 +4,7 @@ */ import { ITerminal } from '../Types'; -import { IEventEmitter, ITheme } from 'xterm'; +import { IEventEmitter, ITheme, IDisposable } from 'xterm'; import { IColorSet } from '../shared/Types'; /** @@ -24,10 +24,11 @@ export const enum FLAGS { * Note that IRenderer implementations should emit the refresh event after * rendering rows to the screen. */ -export interface IRenderer extends IEventEmitter { +export interface IRenderer extends IEventEmitter, IDisposable { dimensions: IRenderDimensions; colorManager: IColorManager; + dispose(): void; setTheme(theme: ITheme): IColorSet; onWindowResize(devicePixelRatio: number): void; onResize(cols: number, rows: number): void; diff --git a/src/utils/ScreenDprMonitor.ts b/src/utils/ScreenDprMonitor.ts index 15f3ac00..9247a032 100644 --- a/src/utils/ScreenDprMonitor.ts +++ b/src/utils/ScreenDprMonitor.ts @@ -3,6 +3,8 @@ * @license MIT */ +import { Disposable } from '../common/Lifecycle'; + export type ScreenDprListener = (newDevicePixelRatio?: number, oldDevicePixelRatio?: number) => void; /** @@ -15,7 +17,7 @@ export type ScreenDprListener = (newDevicePixelRatio?: number, oldDevicePixelRat * The listener should fire on both window zoom changes and switching to a * monitor with a different DPI. */ -export class ScreenDprMonitor { +export class ScreenDprMonitor extends Disposable { private _currentDevicePixelRatio: number; private _outerListener: MediaQueryListListener; private _listener: ScreenDprListener; @@ -33,6 +35,11 @@ export class ScreenDprMonitor { this._updateDpr(); } + public dispose(): void { + super.dispose(); + this.clearListener(); + } + private _updateDpr(): void { // Clear listeners for old DPR if (this._resolutionMediaMatchList) { diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index f4bad717..589ddac8 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -315,6 +315,9 @@ export class MockBuffer implements IBuffer { } export class MockRenderer implements IRenderer { + dispose(): void { + throw new Error('Method not implemented.'); + } colorManager: IColorManager; on(type: string, listener: XtermListener): void { throw new Error('Method not implemented.'); From 545d0782c9289cc33acedac7e233faeca356aa3b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 11:07:29 +1000 Subject: [PATCH 15/41] Properly register and dispose of majority of remaining listeners --- demo/main.js | 11 +++- src/Linkifier.test.ts | 2 + src/Terminal.ts | 109 ++++++++++++++++++---------------- src/Types.ts | 4 +- src/Viewport.ts | 8 ++- src/input/MouseZoneManager.ts | 13 +++- src/input/Types.ts | 4 +- src/utils/TestUtils.test.ts | 3 + 8 files changed, 92 insertions(+), 62 deletions(-) diff --git a/demo/main.js b/demo/main.js index 004a48f0..0b8c10f3 100644 --- a/demo/main.js +++ b/demo/main.js @@ -219,14 +219,14 @@ function initOptions(term) { // Attach listeners booleanOptions.forEach(o => { var input = document.getElementById(`opt-${o}`); - input.addEventListener('change', () => { + addDomListener(input, 'change', () => { console.log('change', o, input.checked); term.setOption(o, input.checked); }); }); numberOptions.forEach(o => { var input = document.getElementById(`opt-${o}`); - input.addEventListener('change', () => { + addDomListener(input, 'change', () => { console.log('change', o, input.value); if (o === 'cols' || o === 'rows') { updateTerminalSize(); @@ -237,13 +237,18 @@ function initOptions(term) { }); Object.keys(stringOptions).forEach(o => { var input = document.getElementById(`opt-${o}`); - input.addEventListener('change', () => { + addDomListener(input, 'change', () => { console.log('change', o, input.value); term.setOption(o, input.value); }); }); } +function addDomListener(element, type, handler) { + element.addEventListener(type, handler); + term._core.register({ dispose: () => element.removeEventListener(type, handler) }); +} + function updateTerminalSize() { var cols = parseInt(document.getElementById(`opt-cols`).value, 10); var rows = parseInt(document.getElementById(`opt-rows`).value, 10); diff --git a/src/Linkifier.test.ts b/src/Linkifier.test.ts index 4c0415de..c09f2ef9 100644 --- a/src/Linkifier.test.ts +++ b/src/Linkifier.test.ts @@ -21,6 +21,8 @@ class TestLinkifier extends Linkifier { } class TestMouseZoneManager implements IMouseZoneManager { + dispose(): void { + } public clears: number = 0; public zones: IMouseZone[] = []; add(zone: IMouseZone): void { diff --git a/src/Terminal.ts b/src/Terminal.ts index 51c3f189..2bf05d99 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -38,7 +38,7 @@ import { Linkifier } from './Linkifier'; import { SelectionManager } from './SelectionManager'; import { CharMeasure } from './utils/CharMeasure'; import * as Browser from './shared/utils/Browser'; -import * as Dom from './ui/Lifecycle'; +import { addDisposableDomListener } from './ui/Lifecycle'; import * as Strings from './Strings'; import { MouseHelper } from './utils/MouseHelper'; import { clone } from './utils/Clone'; @@ -519,30 +519,30 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this._bindKeys(); // Bind clipboard functionality - on(this.element, 'copy', (event: ClipboardEvent) => { + this.register(addDisposableDomListener(this.element, 'copy', (event: ClipboardEvent) => { // If mouse events are active it means the selection manager is disabled and // copy should be handled by the host program. if (!this.hasSelection()) { return; } copyHandler(event, this, this.selectionManager); - }); + })); const pasteHandlerWrapper = (event: ClipboardEvent) => pasteHandler(event, this); - on(this.textarea, 'paste', pasteHandlerWrapper); - on(this.element, 'paste', pasteHandlerWrapper); + this.register(addDisposableDomListener(this.textarea, 'paste', pasteHandlerWrapper)); + this.register(addDisposableDomListener(this.element, 'paste', pasteHandlerWrapper)); // Handle right click context menus if (Browser.isFirefox) { // Firefox doesn't appear to fire the contextmenu event on right click - on(this.element, 'mousedown', (event: MouseEvent) => { + this.register(addDisposableDomListener(this.element, 'mousedown', (event: MouseEvent) => { if (event.button === 2) { rightClickHandler(event, this.textarea, this.selectionManager, this.options.rightClickSelectsWord); } - }); + })); } else { - on(this.element, 'contextmenu', (event: MouseEvent) => { + this.register(addDisposableDomListener(this.element, 'contextmenu', (event: MouseEvent) => { rightClickHandler(event, this.textarea, this.selectionManager, this.options.rightClickSelectsWord); - }); + })); } // Move the textarea under the cursor when middle clicking on Linux to ensure @@ -551,11 +551,11 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II if (Browser.isLinux) { // Use auxclick event over mousedown the latter doesn't seem to work. Note // that the regular click event doesn't fire for the middle mouse button. - on(this.element, 'auxclick', (event: MouseEvent) => { + this.register(addDisposableDomListener(this.element, 'auxclick', (event: MouseEvent) => { if (event.button === 1) { moveTextAreaUnderMouseCursor(event, this.textarea); } - }); + })); } } @@ -564,33 +564,33 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II */ private _bindKeys(): void { const self = this; - on(this.element, 'keydown', function (ev: KeyboardEvent): void { + this.register(addDisposableDomListener(this.element, 'keydown', function (ev: KeyboardEvent): void { if (document.activeElement !== this) { return; } self._keyDown(ev); - }, true); + }, true)); - on(this.element, 'keypress', function (ev: KeyboardEvent): void { + this.register(addDisposableDomListener(this.element, 'keypress', function (ev: KeyboardEvent): void { if (document.activeElement !== this) { return; } self._keyPress(ev); - }, true); + }, true)); - on(this.element, 'keyup', (ev: KeyboardEvent) => { + this.register(addDisposableDomListener(this.element, 'keyup', (ev: KeyboardEvent) => { if (!wasMondifierKeyOnlyEvent(ev)) { this.focus(); } - }, true); + }, true)); - on(this.textarea, 'keydown', (ev: KeyboardEvent) => this._keyDown(ev), true); - on(this.textarea, 'keypress', (ev: KeyboardEvent) => this._keyPress(ev), true); - on(this.textarea, 'compositionstart', () => this._compositionHelper.compositionstart()); - on(this.textarea, 'compositionupdate', (e: CompositionEvent) => this._compositionHelper.compositionupdate(e)); - on(this.textarea, 'compositionend', () => this._compositionHelper.compositionend()); - this.on('refresh', () => this._compositionHelper.updateCompositionElements()); - this.on('refresh', (data) => this._queueLinkification(data.start, data.end)); + this.register(addDisposableDomListener(this.textarea, 'keydown', (ev: KeyboardEvent) => this._keyDown(ev), true)); + this.register(addDisposableDomListener(this.textarea, 'keypress', (ev: KeyboardEvent) => this._keyPress(ev), true)); + this.register(addDisposableDomListener(this.textarea, 'compositionstart', () => this._compositionHelper.compositionstart())); + this.register(addDisposableDomListener(this.textarea, 'compositionupdate', (e: CompositionEvent) => this._compositionHelper.compositionupdate(e))); + this.register(addDisposableDomListener(this.textarea, 'compositionend', () => this._compositionHelper.compositionend())); + this.register(this.addDisposableListener('refresh', () => this._compositionHelper.updateCompositionElements())); + this.register(this.addDisposableListener('refresh', (data) => this._queueLinkification(data.start, data.end))); } /** @@ -641,7 +641,8 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II fragment.appendChild(this.screenElement); this._mouseZoneManager = new MouseZoneManager(this); - this.on('scroll', () => this._mouseZoneManager.clearAll()); + this.register(this._mouseZoneManager); + this.register(this.addDisposableListener('scroll', () => this._mouseZoneManager.clearAll())); this.linkifier.attachToDom(this._mouseZoneManager); this.textarea = document.createElement('textarea'); @@ -653,8 +654,8 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.textarea.setAttribute('autocapitalize', 'off'); this.textarea.setAttribute('spellcheck', 'false'); this.textarea.tabIndex = 0; - this.textarea.addEventListener('focus', () => this._onTextAreaFocus()); - this.textarea.addEventListener('blur', () => this._onTextAreaBlur()); + this.register(addDisposableDomListener(this.textarea, 'focus', () => this._onTextAreaFocus())); + this.register(addDisposableDomListener(this.textarea, 'blur', () => this._onTextAreaBlur())); this._helperContainer.appendChild(this.textarea); this._compositionView = document.createElement('div'); @@ -676,34 +677,35 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.options.theme = null; this.viewport = new Viewport(this, this._viewportElement, this._viewportScrollArea, this.charMeasure); this.viewport.onThemeChanged(this.renderer.colorManager.colors); + this.register(this.viewport); - this.on('cursormove', () => this.renderer.onCursorMove()); - this.on('resize', () => this.renderer.onResize(this.cols, this.rows)); - this.on('blur', () => this.renderer.onBlur()); - this.on('focus', () => this.renderer.onFocus()); - this.on('dprchange', () => this.renderer.onWindowResize(window.devicePixelRatio)); + this.register(this.addDisposableListener('cursormove', () => this.renderer.onCursorMove())); + this.register(this.addDisposableListener('resize', () => this.renderer.onResize(this.cols, this.rows))); + this.register(this.addDisposableListener('blur', () => this.renderer.onBlur())); + this.register(this.addDisposableListener('focus', () => this.renderer.onFocus())); + this.register(this.addDisposableListener('dprchange', () => this.renderer.onWindowResize(window.devicePixelRatio))); // dprchange should handle this case, we need this as well for browsers that don't support the // matchMedia query. - this.register(Dom.addDisposableDomListener(window, 'resize', () => this.renderer.onWindowResize(window.devicePixelRatio))); - this.charMeasure.on('charsizechanged', () => this.renderer.onCharSizeChanged()); - this.renderer.on('resize', (dimensions) => this.viewport.syncScrollArea()); + this.register(addDisposableDomListener(window, 'resize', () => this.renderer.onWindowResize(window.devicePixelRatio))); + this.register(this.charMeasure.addDisposableListener('charsizechanged', () => this.renderer.onCharSizeChanged())); + this.register(this.renderer.addDisposableListener('resize', (dimensions) => this.viewport.syncScrollArea())); 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 => { + this.register(addDisposableDomListener(this.element, 'mousedown', (e: MouseEvent) => this.selectionManager.onMouseDown(e))); + this.register(this.selectionManager.addDisposableListener('refresh', data => this.renderer.onSelectionChanged(data.start, data.end))); + this.register(this.selectionManager.addDisposableListener('newselection', text => { // If there's a new selection, put it into the textarea, focus and select it // in order to register it as a selection on the OS. This event is fired // only on Linux to enable middle click to paste selection. this.textarea.value = text; this.textarea.focus(); this.textarea.select(); - }); - this.on('scroll', () => { + })); + this.register(this.addDisposableListener('scroll', () => { this.viewport.syncScrollArea(); this.selectionManager.refresh(); - }); - this._viewportElement.addEventListener('scroll', () => this.selectionManager.refresh()); + })); + this.register(addDisposableDomListener(this._viewportElement, 'scroll', () => this.selectionManager.refresh())); this.mouseHelper = new MouseHelper(this.renderer); @@ -973,7 +975,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II return button; } - on(el, 'mousedown', (ev: MouseEvent) => { + this.register(addDisposableDomListener(el, 'mousedown', (ev: MouseEvent) => { // Prevent the focus on the textarea from getting lost // and make sure we get focused on mousedown @@ -998,6 +1000,9 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II return this.cancel(ev); } + // TODO: these event listeners should be managed by the disposable, the Terminal reference may + // be kept aroud if Terminal.dispose is fired when the mouse is down + // bind events if (this.normalMouse) on(this._document, 'mousemove', sendMove); @@ -1015,13 +1020,13 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } return this.cancel(ev); - }); + })); // if (this.normalMouse) { // on(this.document, 'mousemove', sendMove); // } - on(el, 'wheel', (ev: WheelEvent) => { + this.register(addDisposableDomListener(el, 'wheel', (ev: WheelEvent) => { if (!this.mouseEvents) { // Convert wheel events into up/down events when the buffer does not have scrollback, this // enables scrolling in apps hosted in the alt buffer such as vim or tmux. @@ -1046,27 +1051,27 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II if (this.x10Mouse || this._vt300Mouse || this._decLocator) return; sendButton(ev); ev.preventDefault(); - }); + })); // allow wheel scrolling in // the shell for example - on(el, 'wheel', (ev: WheelEvent) => { + this.register(addDisposableDomListener(el, 'wheel', (ev: WheelEvent) => { if (this.mouseEvents) return; this.viewport.onWheel(ev); return this.cancel(ev); - }); + })); - on(el, 'touchstart', (ev: TouchEvent) => { + this.register(addDisposableDomListener(el, 'touchstart', (ev: TouchEvent) => { if (this.mouseEvents) return; this.viewport.onTouchStart(ev); return this.cancel(ev); - }); + })); - on(el, 'touchmove', (ev: TouchEvent) => { + this.register(addDisposableDomListener(el, 'touchmove', (ev: TouchEvent) => { if (this.mouseEvents) return; this.viewport.onTouchMove(ev); return this.cancel(ev); - }); + })); } /** diff --git a/src/Types.ts b/src/Types.ts index 1de223f7..eed1e674 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { Terminal as PublicTerminal, ITerminalOptions as IPublicTerminalOptions, IEventEmitter } from 'xterm'; +import { Terminal as PublicTerminal, ITerminalOptions as IPublicTerminalOptions, IEventEmitter, IDisposable } from 'xterm'; import { IColorSet, IRenderer } from './renderer/Types'; import { IMouseZoneManager } from './input/Types'; import { ICharset } from './core/Types'; @@ -86,7 +86,7 @@ export interface IInputHandlingTerminal extends IEventEmitter { tabSet(): void; } -export interface IViewport { +export interface IViewport extends IDisposable { scrollBarWidth: number; syncScrollArea(): void; getLinesScrolled(ev: WheelEvent): number; diff --git a/src/Viewport.ts b/src/Viewport.ts index f690c348..645d89b6 100644 --- a/src/Viewport.ts +++ b/src/Viewport.ts @@ -6,6 +6,8 @@ import { IColorSet } from './renderer/Types'; import { ITerminal, IViewport } from './Types'; import { CharMeasure } from './utils/CharMeasure'; +import { Disposable } from './common/Lifecycle'; +import { addDisposableDomListener } from './ui/Lifecycle'; const FALLBACK_SCROLL_BAR_WIDTH = 15; @@ -13,7 +15,7 @@ const FALLBACK_SCROLL_BAR_WIDTH = 15; * Represents the viewport of a terminal, the visible area within the larger buffer of output. * Logic for the virtual scroll bar is included in this object. */ -export class Viewport implements IViewport { +export class Viewport extends Disposable implements IViewport { public scrollBarWidth: number = 0; private _currentRowHeight: number = 0; private _lastRecordedBufferLength: number = 0; @@ -39,11 +41,13 @@ export class Viewport implements IViewport { private _scrollArea: HTMLElement, private _charMeasure: CharMeasure ) { + super(); + // Measure the width of the scrollbar. If it is 0 we can assume it's an OSX overlay scrollbar. // Unfortunately the overlay scrollbar would be hidden underneath the screen element in that case, // therefore we account for a standard amount to make it visible this.scrollBarWidth = (this._viewportElement.offsetWidth - this._scrollArea.offsetWidth) || FALLBACK_SCROLL_BAR_WIDTH; - this._viewportElement.addEventListener('scroll', this._onScroll.bind(this)); + this.register(addDisposableDomListener(this._viewportElement, 'scroll', this._onScroll.bind(this))); // Perform this async to ensure the CharMeasure is ready. setTimeout(() => this.syncScrollArea(), 0); diff --git a/src/input/MouseZoneManager.ts b/src/input/MouseZoneManager.ts index 65fe74de..e791a982 100644 --- a/src/input/MouseZoneManager.ts +++ b/src/input/MouseZoneManager.ts @@ -5,6 +5,8 @@ import { ITerminal } from '../Types'; import { IMouseZoneManager, IMouseZone } from './Types'; +import { Disposable } from '../common/Lifecycle'; +import { addDisposableDomListener } from '../ui/Lifecycle'; const HOVER_DURATION = 500; @@ -16,7 +18,7 @@ const HOVER_DURATION = 500; * needed to support was single-line links which never overlap. Improvements can * be made in the future. */ -export class MouseZoneManager implements IMouseZoneManager { +export class MouseZoneManager extends Disposable implements IMouseZoneManager { private _zones: IMouseZone[] = []; private _areZonesActive: boolean = false; @@ -30,13 +32,20 @@ export class MouseZoneManager implements IMouseZoneManager { constructor( private _terminal: ITerminal ) { - this._terminal.element.addEventListener('mousedown', e => this._onMouseDown(e)); + super(); + + this.register(addDisposableDomListener(this._terminal.element, 'mousedown', e => this._onMouseDown(e))); // These events are expensive, only listen to it when mouse zones are active this._mouseMoveListener = e => this._onMouseMove(e); this._clickListener = e => this._onClick(e); } + public dispose(): void { + super.dispose(); + this._deactivate(); + } + public add(zone: IMouseZone): void { this._zones.push(zone); if (this._zones.length === 1) { diff --git a/src/input/Types.ts b/src/input/Types.ts index 2bd805bf..89dfa1a4 100644 --- a/src/input/Types.ts +++ b/src/input/Types.ts @@ -3,7 +3,9 @@ * @license MIT */ -export interface IMouseZoneManager { +import { IDisposable } from 'xterm'; + +export interface IMouseZoneManager extends IDisposable { add(zone: IMouseZone): void; clearAll(start?: number, end?: number): void; } diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index 589ddac8..ebb56ee2 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -346,6 +346,9 @@ export class MockRenderer implements IRenderer { } export class MockViewport implements IViewport { + dispose(): void { + throw new Error('Method not implemented.'); + } scrollBarWidth: number = 0; onThemeChanged(colors: IColorSet): void { throw new Error('Method not implemented.'); From 45e4486bf5bbdc2ef80d9f6753fba251a36d9daa Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 11:10:55 +1000 Subject: [PATCH 16/41] Remove on, off, globalOn --- demo/main.js | 6 +++--- src/SelectionManager.ts | 5 +++++ src/Terminal.ts | 27 ++++++++------------------- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/demo/main.js b/demo/main.js index 0b8c10f3..7a114383 100644 --- a/demo/main.js +++ b/demo/main.js @@ -33,15 +33,15 @@ function setPadding() { term.fit(); } -paddingElement.addEventListener('change', setPadding); +addDomListener(paddingElement, 'change', setPadding); -actionElements.findNext.addEventListener('keypress', function (e) { +addDomListener(actionElements.findNext, 'keypress', function (e) { if (e.key === "Enter") { e.preventDefault(); term.findNext(actionElements.findNext.value); } }); -actionElements.findPrevious.addEventListener('keypress', function (e) { +addDomListener(actionElements.findPrevious, 'keypress', function (e) { if (e.key === "Enter") { e.preventDefault(); term.findPrevious(actionElements.findPrevious.value); diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 1b056e83..2632b6bb 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -116,6 +116,11 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._activeSelectionMode = SelectionMode.NORMAL; } + public dispose(): void { + super.dispose(); + this._removeMouseDownListeners(); + } + private get _buffer(): IBuffer { return this._terminal.buffers.active; } diff --git a/src/Terminal.ts b/src/Terminal.ts index 2bf05d99..ca2e6dab 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1004,19 +1004,23 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // be kept aroud if Terminal.dispose is fired when the mouse is down // bind events - if (this.normalMouse) on(this._document, 'mousemove', sendMove); + if (this.normalMouse) { + this._document.addEventListener('mousemove', sendMove); + } // x10 compatibility mode can't send button releases if (!this.x10Mouse) { const handler = (ev: MouseEvent) => { sendButton(ev); // TODO: Seems dangerous calling this on document? - if (this.normalMouse) off(this._document, 'mousemove', sendMove); - off(this._document, 'mouseup', handler); + if (this.normalMouse) { + this._document.removeEventListener('mousemove', sendMove); + } + this._document.removeEventListener('mouseup', handler); return this.cancel(ev); }; // TODO: Seems dangerous calling this on document? - on(this._document, 'mouseup', handler); + this._document.addEventListener('mouseup', handler); } return this.cancel(ev); @@ -1913,21 +1917,6 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * Helpers */ -function globalOn(el: any, type: string, handler: (event: Event) => any, capture?: boolean, passive?: boolean): void { - if (!Array.isArray(el)) { - el = [el]; - } - el.forEach((element: HTMLElement) => { - element.addEventListener(type, handler, { capture: capture || false, passive: passive || false }); - }); -} -// TODO: Remove once everything is typed -const on = globalOn; - -function off(el: any, type: string, handler: (event: Event) => any, capture: boolean = false): void { - el.removeEventListener(type, handler, capture); -} - function wasMondifierKeyOnlyEvent(ev: KeyboardEvent): boolean { return ev.keyCode === 16 || // Shift ev.keyCode === 17 || // Ctrl From 867ce262e864890e1ff1f05884166524a6018f16 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 11:15:10 +1000 Subject: [PATCH 17/41] Fix test --- src/Buffer.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 78bcae27..f60672aa 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -327,7 +327,7 @@ export class Buffer implements IBuffer { marker.dispose(); } })); - marker.on('dispose', () => this._removeMarker(marker)); + marker.register(marker.addDisposableListener('dispose', () => this._removeMarker(marker))); return marker; } @@ -356,7 +356,8 @@ export class Marker extends EventEmitter implements IMarker { return; } this.isDisposed = true; - super.dispose(); + // Emit before super.dispose such that dispose listeners get a change to react this.emit('dispose'); + super.dispose(); } } From 27b1c89f43ca3d5f7cc3da70f4a839fa93735d76 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 11:48:53 +1000 Subject: [PATCH 18/41] More cleaning up of references --- demo/main.js | 19 +++++++++++-------- src/EscapeSequenceParser.ts | 23 ++++++++++++++++++++++- src/InputHandler.ts | 12 +++++++++++- src/Terminal.ts | 1 + src/Types.ts | 2 +- src/addons/attach/attach.ts | 2 +- src/public/Terminal.ts | 2 +- 7 files changed, 48 insertions(+), 13 deletions(-) diff --git a/demo/main.js b/demo/main.js index 7a114383..37bd8a7f 100644 --- a/demo/main.js +++ b/demo/main.js @@ -33,6 +33,8 @@ function setPadding() { term.fit(); } +createTerminal(); + addDomListener(paddingElement, 'change', setPadding); addDomListener(actionElements.findNext, 'keypress', function (e) { @@ -48,8 +50,6 @@ addDomListener(actionElements.findPrevious, 'keypress', function (e) { } }); -createTerminal(); - function createTerminal() { // Clean terminal while (terminalContainer.children.length) { @@ -76,11 +76,14 @@ function createTerminal() { term.fit(); term.focus(); - document.getElementById('dispose').addEventListener('click', () => { + const buttonHandler = () => { term.dispose(); term = null; window.term = null; - }); + socket = null; + document.getElementById('dispose').removeEventListener('click', buttonHandler); + }; + document.getElementById('dispose').addEventListener('click', buttonHandler); // fit is called within a setTimeout, cols and rows need this. setTimeout(function () { @@ -130,7 +133,7 @@ function runFakeTerminal() { term.writeln(''); term.prompt(); - term.on('key', function (key, ev) { + term._core.register(term.addDisposableListener('key', function (key, ev) { var printable = ( !ev.altKey && !ev.altGraphKey && !ev.ctrlKey && !ev.metaKey ); @@ -145,11 +148,11 @@ function runFakeTerminal() { } else if (printable) { term.write(key); } - }); + })); - term.on('paste', function (data, ev) { + term._core,register(term.addDisposableListener('paste', function (data, ev) { term.write(data); - }); + })); } function initOptions(term) { diff --git a/src/EscapeSequenceParser.ts b/src/EscapeSequenceParser.ts index 2849dbc1..8995b203 100644 --- a/src/EscapeSequenceParser.ts +++ b/src/EscapeSequenceParser.ts @@ -4,6 +4,7 @@ */ import { ParserState, ParserAction, IParsingState, IDcsHandler, IEscapeSequenceParser } from './Types'; +import { Disposable } from './common/Lifecycle'; /** * Returns an array filled with numbers between the low and high parameters (right exclusive). @@ -207,7 +208,7 @@ class DcsDummy implements IDcsHandler { * NOTE: The parameter element notation is currently not supported. * TODO: implement error recovery hook via error handler return values */ -export class EscapeSequenceParser implements IEscapeSequenceParser { +export class EscapeSequenceParser extends Disposable implements IEscapeSequenceParser { public initialState: number; public currentState: number; @@ -236,6 +237,8 @@ export class EscapeSequenceParser implements IEscapeSequenceParser { protected _errorHandlerFb: (state: IParsingState) => IParsingState; constructor(readonly TRANSITIONS: TransitionTable = VT500_TRANSITION_TABLE) { + super(); + this.initialState = ParserState.GROUND; this.currentState = this.initialState; this._osc = ''; @@ -260,6 +263,24 @@ export class EscapeSequenceParser implements IEscapeSequenceParser { this._errorHandler = this._errorHandlerFb; } + public dispose(): void { + this._printHandlerFb = null; + this._executeHandlerFb = null; + this._csiHandlerFb = null; + this._escHandlerFb = null; + this._oscHandlerFb = null; + this._dcsHandlerFb = null; + this._errorHandlerFb = null; + this._printHandler = null; + this._executeHandlers = null; + this._csiHandlers = null; + this._escHandlers = null; + this._oscHandlers = null; + this._dcsHandlers = null; + this._activeDcsHandler = null; + this._errorHandler = null; + } + setPrintHandler(callback: (data: string, start: number, end: number) => void): void { this._printHandler = callback; } diff --git a/src/InputHandler.ts b/src/InputHandler.ts index 9df24c7d..089c2d0d 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -12,6 +12,7 @@ import { FLAGS } from './renderer/Types'; import { wcwidth } from './CharWidth'; import { EscapeSequenceParser } from './EscapeSequenceParser'; import { ICharset } from './core/Types'; +import { Disposable } from './common/Lifecycle'; /** * Map collect to glevel. Used in `selectCharset`. @@ -111,13 +112,17 @@ class DECRQSS implements IDcsHandler { * Refer to http://invisible-island.net/xterm/ctlseqs/ctlseqs.html to understand * each function's header comment. */ -export class InputHandler implements IInputHandler { +export class InputHandler extends Disposable implements IInputHandler { private _surrogateHigh: string; constructor( private _terminal: any, // TODO: reestablish IInputHandlingTerminal here private _parser: IEscapeSequenceParser = new EscapeSequenceParser()) { + super(); + + this.register(this._parser); + this._surrogateHigh = ''; /** @@ -285,6 +290,11 @@ export class InputHandler implements IInputHandler { this._parser.setDcsHandler('+q', new RequestTerminfo(this._terminal)); } + public dispose(): void { + super.dispose(); + this._terminal = null; + } + public parse(data: string): void { let buffer = this._terminal.buffer; const cursorStartX = buffer.x; diff --git a/src/Terminal.ts b/src/Terminal.ts index ca2e6dab..0e7bf210 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -300,6 +300,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this._userScrolling = false; this._inputHandler = new InputHandler(this); + this.register(this._inputHandler); // Reuse renderer if the Terminal is being recreated via a reset call. this.renderer = this.renderer || null; this.selectionManager = this.selectionManager || null; diff --git a/src/Types.ts b/src/Types.ts index eed1e674..f7273eb6 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -468,7 +468,7 @@ export interface IDcsHandler { /** * EscapeSequenceParser interface. */ -export interface IEscapeSequenceParser { +export interface IEscapeSequenceParser extends IDisposable { /** * Reset the parser to its initial state (handlers are kept). */ diff --git a/src/addons/attach/attach.ts b/src/addons/attach/attach.ts index e94c0b46..98a0bfaa 100644 --- a/src/addons/attach/attach.ts +++ b/src/addons/attach/attach.ts @@ -90,7 +90,7 @@ export function attach(term: Terminal, socket: WebSocket, bidirectional: boolean addonTerminal._core.register(addSocketListener(socket, 'message', addonTerminal.__getMessage)); if (bidirectional) { - addonTerminal.on('data', addonTerminal.__sendData); + addonTerminal._core.register(addonTerminal.addDisposableListener('data', addonTerminal.__sendData)); } addonTerminal._core.register(addSocketListener(socket, 'close', () => detach(addonTerminal, socket))); diff --git a/src/public/Terminal.ts b/src/public/Terminal.ts index 76e7fba0..a56a270a 100644 --- a/src/public/Terminal.ts +++ b/src/public/Terminal.ts @@ -45,7 +45,7 @@ export class Terminal implements ITerminalApi { this._core.emit(type, data); } public addDisposableListener(type: string, handler: (...args: any[]) => void): IDisposable { - return this.addDisposableListener(type, handler); + return this._core.addDisposableListener(type, handler); } public resize(columns: number, rows: number): void { this._core.resize(columns, rows); From 4d52cd3c66c2b8167161efe1857032b57f8cfdcc Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 21 Jun 2018 14:14:36 +1000 Subject: [PATCH 19/41] Clean reference to custom key event handler on dispose --- src/Terminal.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Terminal.ts b/src/Terminal.ts index 0e7bf210..a9a309c5 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -232,6 +232,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II public dispose(): void { super.dispose(); + this._customKeyEventHandler = null; removeTerminalFromCache(this); this.handler = () => {}; this.write = () => {}; From 43ffa957ab32f156863fae0abea9573ff0aee70b Mon Sep 17 00:00:00 2001 From: 7PH Date: Thu, 21 Jun 2018 09:22:17 +0200 Subject: [PATCH 20/41] #1521: fixed default value of 'Terminal.curAttr' when restoring a not previously saved cursor state --- src/InputHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/InputHandler.ts b/src/InputHandler.ts index 72b3614c..e83cff40 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -1834,7 +1834,7 @@ export class InputHandler implements IInputHandler { public restoreCursor(params: number[]): void { this._terminal.buffer.x = this._terminal.buffer.savedX || 0; this._terminal.buffer.y = this._terminal.buffer.savedY || 0; - this._terminal.curAttr = this._terminal.savedCurAttr || 0; + this._terminal.curAttr = this._terminal.savedCurAttr || DEFAULT_ATTR; } From cc41fefe3164ffa77a50f5fea1b3468bdc01bb6f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 23 Jun 2018 17:25:07 +1000 Subject: [PATCH 21/41] Move MouseZoneManager into ui/ --- src/Linkifier.test.ts | 2 +- src/Linkifier.ts | 4 ++-- src/Terminal.ts | 4 ++-- src/Types.ts | 2 +- src/{input => ui}/MouseZoneManager.ts | 2 +- src/{input => ui}/Types.ts | 0 6 files changed, 7 insertions(+), 7 deletions(-) rename src/{input => ui}/MouseZoneManager.ts (99%) rename src/{input => ui}/Types.ts (100%) diff --git a/src/Linkifier.test.ts b/src/Linkifier.test.ts index c09f2ef9..54e7e88d 100644 --- a/src/Linkifier.test.ts +++ b/src/Linkifier.test.ts @@ -4,7 +4,7 @@ */ import { assert } from 'chai'; -import { IMouseZoneManager, IMouseZone } from './input/Types'; +import { IMouseZoneManager, IMouseZone } from './ui/Types'; import { ILinkMatcher, LineData, ITerminal } from './Types'; import { Linkifier } from './Linkifier'; import { MockBuffer, MockTerminal } from './utils/TestUtils.test'; diff --git a/src/Linkifier.ts b/src/Linkifier.ts index e548551a..88ff1b78 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -3,9 +3,9 @@ * @license MIT */ -import { IMouseZoneManager } from './input/Types'; +import { IMouseZoneManager } from './ui/Types'; import { ILinkHoverEvent, ILinkMatcher, LinkMatcherHandler, LinkHoverEventTypes, ILinkMatcherOptions, ILinkifier, ITerminal } from './Types'; -import { MouseZone } from './input/MouseZoneManager'; +import { MouseZone } from './ui/MouseZoneManager'; import { EventEmitter } from './EventEmitter'; /** diff --git a/src/Terminal.ts b/src/Terminal.ts index a9a309c5..fb175ad0 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -22,7 +22,7 @@ */ import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminalOptions, ITerminal, IBrowser, ILinkifier, ILinkMatcherOptions, CustomKeyEventHandler, LinkMatcherHandler, CharData, LineData } from './Types'; -import { IMouseZoneManager } from './input/Types'; +import { IMouseZoneManager } from './ui/Types'; import { IRenderer } from './renderer/Types'; import { BufferSet } from './BufferSet'; import { Buffer, MAX_BUFFER_SIZE, DEFAULT_ATTR } from './Buffer'; @@ -44,7 +44,7 @@ import { MouseHelper } from './utils/MouseHelper'; import { clone } from './utils/Clone'; import { DEFAULT_BELL_SOUND, SoundManager } from './SoundManager'; import { DEFAULT_ANSI_COLORS } from './renderer/ColorManager'; -import { MouseZoneManager } from './input/MouseZoneManager'; +import { MouseZoneManager } from './ui/MouseZoneManager'; import { AccessibilityManager } from './AccessibilityManager'; import { ScreenDprMonitor } from './utils/ScreenDprMonitor'; import { ITheme, IMarker, IDisposable } from 'xterm'; diff --git a/src/Types.ts b/src/Types.ts index f7273eb6..f3a84550 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -5,7 +5,7 @@ import { Terminal as PublicTerminal, ITerminalOptions as IPublicTerminalOptions, IEventEmitter, IDisposable } from 'xterm'; import { IColorSet, IRenderer } from './renderer/Types'; -import { IMouseZoneManager } from './input/Types'; +import { IMouseZoneManager } from './ui/Types'; import { ICharset } from './core/Types'; export type CustomKeyEventHandler = (event: KeyboardEvent) => boolean; diff --git a/src/input/MouseZoneManager.ts b/src/ui/MouseZoneManager.ts similarity index 99% rename from src/input/MouseZoneManager.ts rename to src/ui/MouseZoneManager.ts index e791a982..491e2a05 100644 --- a/src/input/MouseZoneManager.ts +++ b/src/ui/MouseZoneManager.ts @@ -6,7 +6,7 @@ import { ITerminal } from '../Types'; import { IMouseZoneManager, IMouseZone } from './Types'; import { Disposable } from '../common/Lifecycle'; -import { addDisposableDomListener } from '../ui/Lifecycle'; +import { addDisposableDomListener } from './Lifecycle'; const HOVER_DURATION = 500; diff --git a/src/input/Types.ts b/src/ui/Types.ts similarity index 100% rename from src/input/Types.ts rename to src/ui/Types.ts From 4c7e5d2201d7b97a0c4bcda82a1692bb3288d91c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 23 Jun 2018 17:27:34 +1000 Subject: [PATCH 22/41] Move CircularList to common (tests in ui/ need) --- src/Buffer.test.ts | 2 +- src/Buffer.ts | 2 +- src/Linkifier.test.ts | 2 +- src/{utils => common}/CircularList.test.ts | 0 src/{utils => common}/CircularList.ts | 0 5 files changed, 3 insertions(+), 3 deletions(-) rename src/{utils => common}/CircularList.test.ts (100%) rename src/{utils => common}/CircularList.ts (100%) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 82aa0224..316809e2 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -6,7 +6,7 @@ import { assert } from 'chai'; import { ITerminal } from './Types'; import { Buffer } from './Buffer'; -import { CircularList } from './utils/CircularList'; +import { CircularList } from './common/CircularList'; import { MockTerminal } from './utils/TestUtils.test'; const INIT_COLS = 80; diff --git a/src/Buffer.ts b/src/Buffer.ts index f60672aa..7dc2cc7b 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { CircularList } from './utils/CircularList'; +import { CircularList } from './common/CircularList'; import { LineData, CharData, ITerminal, IBuffer } from './Types'; import { EventEmitter } from './EventEmitter'; import { IMarker } from 'xterm'; diff --git a/src/Linkifier.test.ts b/src/Linkifier.test.ts index 54e7e88d..7f4d5603 100644 --- a/src/Linkifier.test.ts +++ b/src/Linkifier.test.ts @@ -8,7 +8,7 @@ import { IMouseZoneManager, IMouseZone } from './ui/Types'; import { ILinkMatcher, LineData, ITerminal } from './Types'; import { Linkifier } from './Linkifier'; import { MockBuffer, MockTerminal } from './utils/TestUtils.test'; -import { CircularList } from './utils/CircularList'; +import { CircularList } from './common/CircularList'; class TestLinkifier extends Linkifier { constructor(terminal: ITerminal) { diff --git a/src/utils/CircularList.test.ts b/src/common/CircularList.test.ts similarity index 100% rename from src/utils/CircularList.test.ts rename to src/common/CircularList.test.ts diff --git a/src/utils/CircularList.ts b/src/common/CircularList.ts similarity index 100% rename from src/utils/CircularList.ts rename to src/common/CircularList.ts From 8dda3e68f8b2f7687c1ff0be7a3f9929c8c9ffd9 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 23 Jun 2018 17:32:32 +1000 Subject: [PATCH 23/41] MOve CharMeasure into ui/ --- src/SelectionManager.test.ts | 2 +- src/SelectionManager.ts | 2 +- src/Terminal.ts | 2 +- src/Viewport.ts | 2 +- src/{utils => ui}/CharMeasure.test.ts | 0 src/{utils => ui}/CharMeasure.ts | 0 6 files changed, 4 insertions(+), 4 deletions(-) rename src/{utils => ui}/CharMeasure.test.ts (100%) rename src/{utils => ui}/CharMeasure.ts (100%) diff --git a/src/SelectionManager.test.ts b/src/SelectionManager.test.ts index 8e89ea3f..d7afad70 100644 --- a/src/SelectionManager.test.ts +++ b/src/SelectionManager.test.ts @@ -4,7 +4,7 @@ */ import { assert } from 'chai'; -import { CharMeasure } from './utils/CharMeasure'; +import { CharMeasure } from './ui/CharMeasure'; import { SelectionManager } from './SelectionManager'; import { SelectionModel } from './SelectionModel'; import { BufferSet } from './BufferSet'; diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 2632b6bb..6521a209 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -6,7 +6,7 @@ import { ITerminal, ISelectionManager, IBuffer, CharData, XtermListener } from './Types'; import { MouseHelper } from './utils/MouseHelper'; import * as Browser from './shared/utils/Browser'; -import { CharMeasure } from './utils/CharMeasure'; +import { CharMeasure } from './ui/CharMeasure'; import { EventEmitter } from './EventEmitter'; import { SelectionModel } from './SelectionModel'; import { CHAR_DATA_WIDTH_INDEX, CHAR_DATA_CHAR_INDEX } from './Buffer'; diff --git a/src/Terminal.ts b/src/Terminal.ts index fb175ad0..21731b95 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -36,7 +36,7 @@ import { InputHandler } from './InputHandler'; import { Renderer } from './renderer/Renderer'; import { Linkifier } from './Linkifier'; import { SelectionManager } from './SelectionManager'; -import { CharMeasure } from './utils/CharMeasure'; +import { CharMeasure } from './ui/CharMeasure'; import * as Browser from './shared/utils/Browser'; import { addDisposableDomListener } from './ui/Lifecycle'; import * as Strings from './Strings'; diff --git a/src/Viewport.ts b/src/Viewport.ts index 645d89b6..a8966d14 100644 --- a/src/Viewport.ts +++ b/src/Viewport.ts @@ -5,7 +5,7 @@ import { IColorSet } from './renderer/Types'; import { ITerminal, IViewport } from './Types'; -import { CharMeasure } from './utils/CharMeasure'; +import { CharMeasure } from './ui/CharMeasure'; import { Disposable } from './common/Lifecycle'; import { addDisposableDomListener } from './ui/Lifecycle'; diff --git a/src/utils/CharMeasure.test.ts b/src/ui/CharMeasure.test.ts similarity index 100% rename from src/utils/CharMeasure.test.ts rename to src/ui/CharMeasure.test.ts diff --git a/src/utils/CharMeasure.ts b/src/ui/CharMeasure.ts similarity index 100% rename from src/utils/CharMeasure.ts rename to src/ui/CharMeasure.ts From d585515de10f9e2a6ae917576555e2f1dde21cf3 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 23 Jun 2018 17:34:01 +1000 Subject: [PATCH 24/41] Move RenderDebouncer and ScreenDprMonitor into ui --- src/AccessibilityManager.ts | 2 +- src/Terminal.ts | 2 +- src/renderer/Renderer.ts | 4 ++-- src/renderer/dom/DomRenderer.ts | 2 +- src/{utils => ui}/RenderDebouncer.ts | 0 src/{utils => ui}/ScreenDprMonitor.ts | 0 6 files changed, 5 insertions(+), 5 deletions(-) rename src/{utils => ui}/RenderDebouncer.ts (100%) rename src/{utils => ui}/ScreenDprMonitor.ts (100%) diff --git a/src/AccessibilityManager.ts b/src/AccessibilityManager.ts index 282a5e5a..0975e0b6 100644 --- a/src/AccessibilityManager.ts +++ b/src/AccessibilityManager.ts @@ -6,7 +6,7 @@ import * as Strings from './Strings'; import { ITerminal, IBuffer } from './Types'; import { isMac } from './shared/utils/Browser'; -import { RenderDebouncer } from './utils/RenderDebouncer'; +import { RenderDebouncer } from './ui/RenderDebouncer'; import { addDisposableDomListener } from './ui/Lifecycle'; import { Disposable } from './common/Lifecycle'; diff --git a/src/Terminal.ts b/src/Terminal.ts index 21731b95..7dd5117a 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -46,7 +46,7 @@ import { DEFAULT_BELL_SOUND, SoundManager } from './SoundManager'; import { DEFAULT_ANSI_COLORS } from './renderer/ColorManager'; import { MouseZoneManager } from './ui/MouseZoneManager'; import { AccessibilityManager } from './AccessibilityManager'; -import { ScreenDprMonitor } from './utils/ScreenDprMonitor'; +import { ScreenDprMonitor } from './ui/ScreenDprMonitor'; import { ITheme, IMarker, IDisposable } from 'xterm'; import { removeTerminalFromCache } from './renderer/atlas/CharAtlasCache'; import { DomRenderer } from './renderer/dom/DomRenderer'; diff --git a/src/renderer/Renderer.ts b/src/renderer/Renderer.ts index b6f4f640..aecd5c79 100644 --- a/src/renderer/Renderer.ts +++ b/src/renderer/Renderer.ts @@ -11,8 +11,8 @@ import { IRenderLayer, IColorSet, IRenderer, IRenderDimensions } from './Types'; import { ITerminal } from '../Types'; import { LinkRenderLayer } from './LinkRenderLayer'; import { EventEmitter } from '../EventEmitter'; -import { RenderDebouncer } from '../utils/RenderDebouncer'; -import { ScreenDprMonitor } from '../utils/ScreenDprMonitor'; +import { RenderDebouncer } from '../ui/RenderDebouncer'; +import { ScreenDprMonitor } from '../ui/ScreenDprMonitor'; import { ITheme } from 'xterm'; export class Renderer extends EventEmitter implements IRenderer { diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts index 5b1813b5..12932a5e 100644 --- a/src/renderer/dom/DomRenderer.ts +++ b/src/renderer/dom/DomRenderer.ts @@ -8,7 +8,7 @@ import { ITerminal } from '../../Types'; import { ITheme } from 'xterm'; import { EventEmitter } from '../../EventEmitter'; import { ColorManager } from '../ColorManager'; -import { RenderDebouncer } from '../../utils/RenderDebouncer'; +import { RenderDebouncer } from '../../ui/RenderDebouncer'; import { BOLD_CLASS, ITALIC_CLASS, CURSOR_CLASS, DomRendererRowFactory } from './DomRendererRowFactory'; const TERMINAL_CLASS_PREFIX = 'xterm-dom-renderer-owner-'; diff --git a/src/utils/RenderDebouncer.ts b/src/ui/RenderDebouncer.ts similarity index 100% rename from src/utils/RenderDebouncer.ts rename to src/ui/RenderDebouncer.ts diff --git a/src/utils/ScreenDprMonitor.ts b/src/ui/ScreenDprMonitor.ts similarity index 100% rename from src/utils/ScreenDprMonitor.ts rename to src/ui/ScreenDprMonitor.ts From 8ae8705ae6c3a04719a21facff3b35e11176feaa Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 25 Jun 2018 11:21:00 -0700 Subject: [PATCH 25/41] Fix typo --- demo/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo/main.js b/demo/main.js index 37bd8a7f..cccf5a04 100644 --- a/demo/main.js +++ b/demo/main.js @@ -150,7 +150,7 @@ function runFakeTerminal() { } })); - term._core,register(term.addDisposableListener('paste', function (data, ev) { + term._core.register(term.addDisposableListener('paste', function (data, ev) { term.write(data); })); } From ff18b193eb2cf8615488d503b4ef148c69f64e7b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 25 Jun 2018 11:27:57 -0700 Subject: [PATCH 26/41] Pin node-pty to 0.7.6 --- package.json | 2 +- src/Terminal.integration.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 4aaac342..2887d56a 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "jsdoc": "3.4.3", "jsdom": "^11.11.0", "merge-stream": "^1.0.1", - "node-pty": "^0.7.2", + "node-pty": "0.7.6", "nodemon": "1.10.2", "npm-run-all": "^4.1.2", "nyc": "^11.8.0", diff --git a/src/Terminal.integration.ts b/src/Terminal.integration.ts index 7b7cdbdd..f37d7840 100644 --- a/src/Terminal.integration.ts +++ b/src/Terminal.integration.ts @@ -88,7 +88,7 @@ if (os.platform() !== 'win32') { /** some helpers for pty interaction */ // we need a pty in between to get the termios decorations // for the basic test cases a raw pty device is enough - primitivePty = pty.native.open(cols, rows); + primitivePty = (pty).native.open(cols, rows); /** tests */ describe('xterm output comparison', () => { From f1504e5c0fd8297a9f5e3773ee6b87444fb45c2b Mon Sep 17 00:00:00 2001 From: 7PH Date: Tue, 26 Jun 2018 10:33:46 +0200 Subject: [PATCH 27/41] #1532: Terminal reset does not affect cursorState anymore --- src/Terminal.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Terminal.ts b/src/Terminal.ts index 4d062f76..9f581461 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1830,9 +1830,11 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this.options.cols = this.cols; const customKeyEventHandler = this._customKeyEventHandler; const inputHandler = this._inputHandler; + const cursorState = this.cursorState; this._setup(); this._customKeyEventHandler = customKeyEventHandler; this._inputHandler = inputHandler; + this.cursorState = cursorState; this.refresh(0, this.rows - 1); if (this.viewport) { this.viewport.syncScrollArea(); From 0ad7b8e27b2da454991ad6fd0d5a3c27c870f67e Mon Sep 17 00:00:00 2001 From: 7PH Date: Tue, 26 Jun 2018 10:36:11 +0200 Subject: [PATCH 28/41] #1532: Add test to ensure terminal reset does not affect cursorState --- src/Terminal.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Terminal.test.ts b/src/Terminal.test.ts index 0745ddda..28ce6ce8 100644 --- a/src/Terminal.test.ts +++ b/src/Terminal.test.ts @@ -120,6 +120,14 @@ describe('term.js addons', () => { }); }); + describe('reset', () => { + it('should not affect cursorState', () => { + term.cursorState = 1; + term.reset(); + assert.equal(term.cursorState, 1); + }); + }); + describe('clear', () => { it('should clear a buffer equal to rows', () => { const promptLine = term.buffer.lines.get(term.buffer.ybase + term.buffer.y); From 218a4014c78b64ec6b681ca1964e0235c86e67c2 Mon Sep 17 00:00:00 2001 From: 7PH Date: Tue, 26 Jun 2018 10:42:08 +0200 Subject: [PATCH 29/41] #1532: Add test to ensure terminal reset does not display cursor if hidden before --- src/Terminal.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Terminal.test.ts b/src/Terminal.test.ts index 28ce6ce8..0ea854e2 100644 --- a/src/Terminal.test.ts +++ b/src/Terminal.test.ts @@ -125,6 +125,9 @@ describe('term.js addons', () => { term.cursorState = 1; term.reset(); assert.equal(term.cursorState, 1); + term.cursorState = 0; + term.reset(); + assert.equal(term.cursorState, 0); }); }); From 3202ddc96d2bed5b4e68ea5b979bd334f2ce9354 Mon Sep 17 00:00:00 2001 From: Viraj Sinha Date: Thu, 28 Jun 2018 15:55:44 -0700 Subject: [PATCH 30/41] Adds support for column selections if holding alt on mac and shift on windows/linux --- src/Buffer.test.ts | 77 +++++++++++++++++++ src/Buffer.ts | 2 +- src/SelectionManager.test.ts | 61 ++++++++++++++- src/SelectionManager.ts | 106 ++++++++++++++++++++------- src/Terminal.ts | 22 +++++- src/renderer/BaseRenderLayer.ts | 2 +- src/renderer/Renderer.ts | 4 +- src/renderer/SelectionRenderLayer.ts | 35 +++++---- src/renderer/Types.ts | 4 +- src/renderer/dom/DomRenderer.ts | 31 +++++--- src/xterm.css | 5 ++ 11 files changed, 291 insertions(+), 58 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 82aa0224..085f3d99 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -269,4 +269,81 @@ describe('Buffer', () => { assert.equal(buffer.markers.length, 0); }); }); + + describe ('translateBufferLineToString', () => { + it('should handle selecting a section of ascii text', () => { + buffer.lines.set(0, [ + [ null, 'a', 1, 'a'.charCodeAt(0)], + [ null, 'b', 1, 'b'.charCodeAt(0)], + [ null, 'c', 1, 'c'.charCodeAt(0)], + [ null, 'd', 1, 'd'.charCodeAt(0)] + ]); + + const str = buffer.translateBufferLineToString(0, true, 0, 2); + assert.equal(str, 'ab'); + }); + + it('should handle a cut-off double width character by including it', () => { + buffer.lines.set(0, [ + [ null, '語', 2, 35486 ], + [ null, '', 0, null], + [ null, 'a', 1, 'a'.charCodeAt(0)] + ]); + + const str1 = buffer.translateBufferLineToString(0, true, 0, 1); + assert.equal(str1, '語'); + }); + + it('should handle a zero width character in the middle of the string by not including it', () => { + buffer.lines.set(0, [ + [ null, '語', 2, '語'.charCodeAt(0) ], + [ null, '', 0, null], + [ null, 'a', 1, 'a'.charCodeAt(0)] + ]); + + const str0 = buffer.translateBufferLineToString(0, true, 0, 1); + assert.equal(str0, '語'); + + const str1 = buffer.translateBufferLineToString(0, true, 0, 2); + assert.equal(str1, '語'); + + const str2 = buffer.translateBufferLineToString(0, true, 0, 3); + assert.equal(str2, '語a'); + }); + + it('should handle single width emojis', () => { + buffer.lines.set(0, [ + [ null, '😁', 1, '😁'.charCodeAt(0) ], + [ null, 'a', 1, 'a'.charCodeAt(0)] + ]); + + const str1 = buffer.translateBufferLineToString(0, true, 0, 1); + assert.equal(str1, '😁'); + + const str2 = buffer.translateBufferLineToString(0, true, 0, 2); + assert.equal(str2, '😁a'); + }); + + it('should handle double width emojis', () => { + buffer.lines.set(0, [ + [ null, '😁', 2, '😁'.charCodeAt(0) ], + [ null, '', 0, null] + ]); + + const str1 = buffer.translateBufferLineToString(0, true, 0, 1); + assert.equal(str1, '😁'); + + const str2 = buffer.translateBufferLineToString(0, true, 0, 2); + assert.equal(str2, '😁'); + + buffer.lines.set(0, [ + [ null, '😁', 2, '😁'.charCodeAt(0) ], + [ null, '', 0, null], + [ null, 'a', 1, 'a'.charCodeAt(0)] + ]); + + const str3 = buffer.translateBufferLineToString(0, true, 0, 3); + assert.equal(str3, '😁a'); + }); + }); }); diff --git a/src/Buffer.ts b/src/Buffer.ts index 5ac2a4f2..3b48e8a6 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -227,7 +227,7 @@ export class Buffer implements IBuffer { if (startCol >= i) { startIndex--; } - if (endCol >= i) { + if (endCol > i) { endIndex--; } } else { diff --git a/src/SelectionManager.test.ts b/src/SelectionManager.test.ts index 8e89ea3f..70d3fc1b 100644 --- a/src/SelectionManager.test.ts +++ b/src/SelectionManager.test.ts @@ -5,7 +5,7 @@ import { assert } from 'chai'; import { CharMeasure } from './utils/CharMeasure'; -import { SelectionManager } from './SelectionManager'; +import { SelectionManager, SelectionMode } from './SelectionManager'; import { SelectionModel } from './SelectionModel'; import { BufferSet } from './BufferSet'; import { LineData, CharData, ITerminal, IBuffer } from './Types'; @@ -25,6 +25,8 @@ class TestSelectionManager extends SelectionManager { public get model(): SelectionModel { return this._model; } + public set selectionMode(mode: SelectionMode) { this._activeSelectionMode = mode; } + public selectLineAt(line: number): void { this._selectLineAt(line); } public selectWordAt(coords: [number, number]): void { this._selectWordAt(coords, true); } @@ -378,4 +380,61 @@ describe('SelectionManager', () => { assert.equal(selectionManager.hasSelection, true); }); }); + + describe('column selection', () => { + it('should select a column of text', () => { + buffer.lines.length = 3; + buffer.lines.set(0, stringToRow('abcdefghij')); + buffer.lines.set(1, stringToRow('klmnopqrst')); + buffer.lines.set(2, stringToRow('uvwxyz')); + + selectionManager.selectionMode = SelectionMode.COLUMN; + selectionManager.model.selectionStart = [2, 0]; + selectionManager.model.selectionEnd = [4, 2]; + + assert.equal(selectionManager.selectionText, 'cd\nmn\nwx'); + }); + + it('should select a column of text without chopping up double width characters', () => { + buffer.lines.length = 3; + buffer.lines.set(0, stringToRow('a')); + buffer.lines.set(1, stringToRow('語')); + buffer.lines.set(2, stringToRow('b')); + + selectionManager.selectionMode = SelectionMode.COLUMN; + selectionManager.model.selectionStart = [0, 0]; + selectionManager.model.selectionEnd = [1, 2]; + + assert.equal(selectionManager.selectionText, 'a\n語\nb'); + }); + + it('should select a column of text with single character emojis', () => { + buffer.lines.length = 3; + buffer.lines.set(0, stringToRow('a')); + buffer.lines.set(1, stringToRow('☃')); + buffer.lines.set(2, stringToRow('c')); + + selectionManager.selectionMode = SelectionMode.COLUMN; + selectionManager.model.selectionStart = [0, 0]; + selectionManager.model.selectionEnd = [1, 2]; + + assert.equal(selectionManager.selectionText, 'a\n☃\nc'); + }); + + it('should select a column of text with double character emojis', () => { + // TODO the case this is testing works for me in the demo webapp, + // but doing it programmatically fails. + buffer.lines.length = 3; + buffer.lines.set(0, stringToRow('a')); + buffer.lines.set(1, stringToRow('😁')); + buffer.lines.set(2, stringToRow('c')); + + selectionManager.selectionMode = SelectionMode.COLUMN; + selectionManager.model.selectionStart = [0, 0]; + selectionManager.model.selectionEnd = [1, 2]; + + assert.equal(selectionManager.selectionText, 'a\n😁\nc'); + }); + }); }); + diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 1b056e83..4f86c6de 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -54,10 +54,11 @@ interface IWordPosition { /** * A selection mode, this drives how the selection behaves on mouse move. */ -const enum SelectionMode { +export const enum SelectionMode { NORMAL, WORD, - LINE + LINE, + COLUMN } /** @@ -80,7 +81,12 @@ export class SelectionManager extends EventEmitter implements ISelectionManager /** * The current selection mode. */ - private _activeSelectionMode: SelectionMode; + protected _activeSelectionMode: SelectionMode; + + /** + * The modifier keys required to trigger block select mode with left click + drag + */ + private _columnSelectRequiredModifiers: string[]; /** * A setInterval timer that is active while the mouse is down whose callback @@ -114,6 +120,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._model = new SelectionModel(_terminal); this._activeSelectionMode = SelectionMode.NORMAL; + this._columnSelectRequiredModifiers = this._initColumnSelectModifierKeys(); } private get _buffer(): IBuffer { @@ -177,30 +184,41 @@ export class SelectionManager extends EventEmitter implements ISelectionManager return ''; } - // Get first row - const startRowEndCol = start[1] === end[1] ? end[0] : null; const result: string[] = []; - result.push(this._buffer.translateBufferLineToString(start[1], true, start[0], startRowEndCol)); - // Get middle rows - for (let i = start[1] + 1; i <= end[1] - 1; i++) { - const bufferLine = this._buffer.lines.get(i); - const lineText = this._buffer.translateBufferLineToString(i, true); - if ((bufferLine).isWrapped) { - result[result.length - 1] += lineText; - } else { - result.push(lineText); + if (this._activeSelectionMode === SelectionMode.COLUMN) { + // Ignore zero width selections + if (start[0] !== end[0]) { + for (let i = start[1]; i <= end[1]; i++) { + const lineText = this._buffer.translateBufferLineToString(i, true, start[0], end[0]); + result.push(lineText); + } } - } + } else { + // Get first row + const startRowEndCol = start[1] === end[1] ? end[0] : null; + result.push(this._buffer.translateBufferLineToString(start[1], true, start[0], startRowEndCol)); - // Get final row - if (start[1] !== end[1]) { - const bufferLine = this._buffer.lines.get(end[1]); - const lineText = this._buffer.translateBufferLineToString(end[1], true, 0, end[0]); - if ((bufferLine).isWrapped) { - result[result.length - 1] += lineText; - } else { - result.push(lineText); + // Get middle rows + for (let i = start[1] + 1; i <= end[1] - 1; i++) { + const bufferLine = this._buffer.lines.get(i); + const lineText = this._buffer.translateBufferLineToString(i, true); + if ((bufferLine).isWrapped) { + result[result.length - 1] += lineText; + } else { + result.push(lineText); + } + } + + // Get final row + if (start[1] !== end[1]) { + const bufferLine = this._buffer.lines.get(end[1]); + const lineText = this._buffer.translateBufferLineToString(end[1], true, 0, end[0]); + if ((bufferLine).isWrapped) { + result[result.length - 1] += lineText; + } else { + result.push(lineText); + } } } @@ -249,7 +267,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager */ private _refresh(): void { this._refreshAnimationFrame = null; - this.emit('refresh', { start: this._model.finalSelectionStart, end: this._model.finalSelectionEnd }); + this.emit('refresh', { start: this._model.finalSelectionStart, end: this._model.finalSelectionEnd, columnSelectMode: this._activeSelectionMode === SelectionMode.COLUMN }); } /** @@ -398,7 +416,11 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._onIncrementalClick(event); } else { if (event.detail === 1) { - this._onSingleClick(event); + if (this.isColumnSelectMode(event)) { + this._onColumnSelectSingleClick(event); + } else { + this._onSingleClick(event); + } } else if (event.detail === 2) { this._onDoubleClick(event); } else if (event.detail === 3) { @@ -502,6 +524,40 @@ export class SelectionManager extends EventEmitter implements ISelectionManager } } + /** + * Configures the modifier key for enabling column selection mode + */ + private _initColumnSelectModifierKeys(): string[] { + if (this._terminal.browser.isMac) { + return ['altKey']; + } + + // Linux and Windows + return ['shiftKey']; + } + + /** + * Begin a block selection + */ + private _onColumnSelectSingleClick(event: MouseEvent): void { + this._onSingleClick(event); // Perform all the normal setup actions + this._activeSelectionMode = SelectionMode.COLUMN; + } + + /** + * Checks if all required key modifiers are pressed in order to enable block + * select mode + * @param event the mouse click event + */ + public isColumnSelectMode(event: KeyboardEvent | MouseEvent): boolean { + for (let i = 0; i < this._columnSelectRequiredModifiers.length; i++) { + if (!(event)[this._columnSelectRequiredModifiers[i]]) { + return false; + } + } + return true; + } + /** * Handles the mousemove event when the mouse button is down, recording the * end of the selection and refreshing the selection. diff --git a/src/Terminal.ts b/src/Terminal.ts index 9f581461..b754b6f6 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -590,6 +590,8 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II if (!wasMondifierKeyOnlyEvent(ev)) { this.focus(); } + + self._keyUp(ev); }, true); on(this.textarea, 'keydown', (ev: KeyboardEvent) => this._keyDown(ev), true); @@ -696,7 +698,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II 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('refresh', data => this.renderer.onSelectionChanged(data.start, data.end, data.columnSelectMode)); this.selectionManager.on('newselection', text => { // If there's a new selection, put it into the textarea, focus and select it // in order to register it as a selection on the OS. This event is fired @@ -1098,6 +1100,17 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } } + /** + * Change the cursor style for different selection modes + */ + public updateCursorStyle(ev: KeyboardEvent): void { + if (this.selectionManager.isColumnSelectMode(ev)) { + this.element.classList.add('xterm-cursor-crosshair'); + } else { + this.element.classList.remove('xterm-cursor-crosshair'); + } + } + /** * Display the cursor element */ @@ -1415,6 +1428,8 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II const result = evaluateKeyboardEvent(event, this.applicationCursor, this.browser.isMac, this.options.macOptionIsMeta); + this.updateCursorStyle(event); + // if (result.key === C0.DC3) { // XOFF // this._writeStopped = true; // } else if (result.key === C0.DC1) { // XON @@ -1486,6 +1501,11 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } } + protected _keyUp(ev: KeyboardEvent): boolean { + this.updateCursorStyle(ev); + return true; + } + /** * Handle a keypress event. * Key Resources: diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts index b2a40290..2e9de389 100644 --- a/src/renderer/BaseRenderLayer.ts +++ b/src/renderer/BaseRenderLayer.ts @@ -49,7 +49,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { public onFocus(terminal: ITerminal): void {} public onCursorMove(terminal: ITerminal): void {} public onGridChanged(terminal: ITerminal, startRow: number, endRow: number): void {} - public onSelectionChanged(terminal: ITerminal, start: [number, number], end: [number, number]): void {} + public onSelectionChanged(terminal: ITerminal, start: [number, number], end: [number, number], columnSelectMode: boolean = false): void {} public onThemeChanged(terminal: ITerminal, colorSet: IColorSet): void { this._refreshCharAtlas(terminal, colorSet); diff --git a/src/renderer/Renderer.ts b/src/renderer/Renderer.ts index c41dece5..a3743289 100644 --- a/src/renderer/Renderer.ts +++ b/src/renderer/Renderer.ts @@ -141,8 +141,8 @@ export class Renderer extends EventEmitter implements IRenderer { this._runOperation(l => l.onFocus(this._terminal)); } - public onSelectionChanged(start: [number, number], end: [number, number]): void { - this._runOperation(l => l.onSelectionChanged(this._terminal, start, end)); + public onSelectionChanged(start: [number, number], end: [number, number], columnSelectMode: boolean = false): void { + this._runOperation(l => l.onSelectionChanged(this._terminal, start, end, columnSelectMode)); } public onCursorMove(): void { diff --git a/src/renderer/SelectionRenderLayer.ts b/src/renderer/SelectionRenderLayer.ts index e2212246..a9542cd7 100644 --- a/src/renderer/SelectionRenderLayer.ts +++ b/src/renderer/SelectionRenderLayer.ts @@ -37,7 +37,7 @@ export class SelectionRenderLayer extends BaseRenderLayer { } } - public onSelectionChanged(terminal: ITerminal, start: [number, number], end: [number, number]): void { + public onSelectionChanged(terminal: ITerminal, start: [number, number], end: [number, number], columnSelectMode: boolean): void { // Selection has not changed if (this._state.start === start || this._state.end === end) { return; @@ -62,21 +62,30 @@ export class SelectionRenderLayer extends BaseRenderLayer { return; } - // Draw first row - const startCol = viewportStartRow === viewportCappedStartRow ? start[0] : 0; - const startRowEndCol = viewportCappedStartRow === viewportCappedEndRow ? end[0] : terminal.cols; this._ctx.fillStyle = this._colors.selection.css; - this.fillCells(startCol, viewportCappedStartRow, startRowEndCol - startCol, 1); - // Draw middle rows - const middleRowsCount = Math.max(viewportCappedEndRow - viewportCappedStartRow - 1, 0); - this.fillCells(0, viewportCappedStartRow + 1, terminal.cols, middleRowsCount); + if (columnSelectMode) { + const startCol = viewportStartRow === viewportCappedStartRow ? start[0] : 0; + const width = end[0] - startCol; + const height = viewportCappedEndRow - viewportCappedStartRow + 1; + this.fillCells(startCol, viewportCappedStartRow, width, height); - // Draw final row - if (viewportCappedStartRow !== viewportCappedEndRow) { - // Only draw viewportEndRow if it's not the same as viewportStartRow - const endCol = viewportEndRow === viewportCappedEndRow ? end[0] : terminal.cols; - this.fillCells(0, viewportCappedEndRow, endCol, 1); + } else { + // Draw first row + const startCol = viewportStartRow === viewportCappedStartRow ? start[0] : 0; + const startRowEndCol = viewportCappedStartRow === viewportCappedEndRow ? end[0] : terminal.cols; + this.fillCells(startCol, viewportCappedStartRow, startRowEndCol - startCol, 1); + + // Draw middle rows + const middleRowsCount = Math.max(viewportCappedEndRow - viewportCappedStartRow - 1, 0); + this.fillCells(0, viewportCappedStartRow + 1, terminal.cols, middleRowsCount); + + // Draw final row + if (viewportCappedStartRow !== viewportCappedEndRow) { + // Only draw viewportEndRow if it's not the same as viewportStartRow + const endCol = viewportEndRow === viewportCappedEndRow ? end[0] : terminal.cols; + this.fillCells(0, viewportCappedEndRow, endCol, 1); + } } // Save state for next render diff --git a/src/renderer/Types.ts b/src/renderer/Types.ts index bafc9ad4..6d9b293d 100644 --- a/src/renderer/Types.ts +++ b/src/renderer/Types.ts @@ -34,7 +34,7 @@ export interface IRenderer extends IEventEmitter { onCharSizeChanged(): void; onBlur(): void; onFocus(): void; - onSelectionChanged(start: [number, number], end: [number, number]): void; + onSelectionChanged(start: [number, number], end: [number, number], columnSelectMode: boolean): void; onCursorMove(): void; onOptionsChanged(): void; clear(): void; @@ -98,7 +98,7 @@ export interface IRenderLayer { /** * Calls when the selection changes. */ - onSelectionChanged(terminal: ITerminal, start: [number, number], end: [number, number]): void; + onSelectionChanged(terminal: ITerminal, start: [number, number], end: [number, number], columnSelectMode: boolean): void; /** * Resize the render layer. diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts index 8c335dca..5f3b1c2a 100644 --- a/src/renderer/dom/DomRenderer.ts +++ b/src/renderer/dom/DomRenderer.ts @@ -217,7 +217,7 @@ export class DomRenderer extends EventEmitter implements IRenderer { this._rowContainer.classList.add(FOCUS_CLASS); } - public onSelectionChanged(start: [number, number], end: [number, number]): void { + public onSelectionChanged(start: [number, number], end: [number, number], columnSelectMode: boolean): void { // Remove all selections while (this._selectionContainer.children.length) { this._selectionContainer.removeChild(this._selectionContainer.children[0]); @@ -241,18 +241,25 @@ export class DomRenderer extends EventEmitter implements IRenderer { // Create the selections const documentFragment = document.createDocumentFragment(); - // Draw first row const startCol = viewportStartRow === viewportCappedStartRow ? start[0] : 0; - const endCol = viewportCappedStartRow === viewportCappedEndRow ? end[0] : this._terminal.cols; - documentFragment.appendChild(this._createSelectionElement(viewportCappedStartRow, startCol, endCol)); - // Draw middle rows - const middleRowsCount = viewportCappedEndRow - viewportCappedStartRow - 1; - documentFragment.appendChild(this._createSelectionElement(viewportCappedStartRow + 1, 0, this._terminal.cols, middleRowsCount)); - // Draw final row - if (viewportCappedStartRow !== viewportCappedEndRow) { - // Only draw viewportEndRow if it's not the same as viewporttartRow - const endCol = viewportEndRow === viewportCappedEndRow ? end[0] : this._terminal.cols; - documentFragment.appendChild(this._createSelectionElement(viewportCappedEndRow, 0, endCol)); + + if (columnSelectMode) { + documentFragment.appendChild( + this._createSelectionElement(viewportCappedStartRow, startCol, end[0], viewportCappedEndRow - viewportStartRow + 1) + ); + } else { + // Draw first row + const endCol = viewportCappedStartRow === viewportCappedEndRow ? end[0] : this._terminal.cols; + documentFragment.appendChild(this._createSelectionElement(viewportCappedStartRow, startCol, endCol)); + // Draw middle rows + const middleRowsCount = viewportCappedEndRow - viewportCappedStartRow - 1; + documentFragment.appendChild(this._createSelectionElement(viewportCappedStartRow + 1, 0, this._terminal.cols, middleRowsCount)); + // Draw final row + if (viewportCappedStartRow !== viewportCappedEndRow) { + // Only draw viewportEndRow if it's not the same as viewporttartRow + const endCol = viewportEndRow === viewportCappedEndRow ? end[0] : this._terminal.cols; + documentFragment.appendChild(this._createSelectionElement(viewportCappedEndRow, 0, endCol)); + } } this._selectionContainer.appendChild(documentFragment); } diff --git a/src/xterm.css b/src/xterm.css index 6e7d2f96..b435a63f 100644 --- a/src/xterm.css +++ b/src/xterm.css @@ -139,6 +139,11 @@ cursor: pointer; } +.xterm.xterm-cursor-crosshair { + /* Block selection mode */ + cursor: crosshair; +} + .xterm .xterm-accessibility, .xterm .xterm-message { position: absolute; From 2669576b6caad60e0815d3767ba7bc009a9723e4 Mon Sep 17 00:00:00 2001 From: Viraj Sinha Date: Thu, 28 Jun 2018 15:56:25 -0700 Subject: [PATCH 31/41] Adds a toggle to disable column selections on mac --- src/SelectionManager.ts | 6 +++++- src/Terminal.ts | 1 + typings/xterm.d.ts | 11 ++++++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 4f86c6de..c11f49c6 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -376,7 +376,11 @@ export class SelectionManager extends EventEmitter implements ISelectionManager * @param event The mouse event. */ public shouldForceSelection(event: MouseEvent): boolean { - return Browser.isMac ? event.altKey : event.shiftKey; + if (Browser.isMac) { + return event.altKey && this._terminal.options.macOptionClickForcesSelection; + } + + return event.shiftKey; } /** diff --git a/src/Terminal.ts b/src/Terminal.ts index b754b6f6..e2fa8bef 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -98,6 +98,7 @@ const DEFAULT_OPTIONS: ITerminalOptions = { screenReaderMode: false, debug: false, macOptionIsMeta: false, + macOptionClickForcesSelection: false, cancelEvents: false, disableStdin: false, useFlowControl: false, diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index bda984b7..94fa0432 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -124,6 +124,15 @@ declare module 'xterm' { */ macOptionIsMeta?: boolean; + /** + * Whether holding a modifier key will force normal selection behavior, + * regardless of whether the terminal is in mouse events mode. This will + * also prevent mouse events from being emitted by the terminal. For example, + * this allows you to use xterm.js' regular selection inside tmux with + * mouse mode enabled. + */ + macOptionClickForcesSelection?: boolean; + /** * (EXPERIMENTAL) The type of renderer to use, this allows using the * fallback DOM renderer when canvas is too slow for the environment. The @@ -562,7 +571,7 @@ declare module 'xterm' { * Retrieves an option's value from the terminal. * @param key The option key. */ - getOption(key: 'colors'): string[]; + getOption(key: 'columnSelectModifiers' | 'colors'): string[]; /** * Retrieves an option's value from the terminal. * @param key The option key. From 835d5a16ea711b32ba9ea0eb7a843d50a0975c10 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 29 Jun 2018 11:17:45 -0700 Subject: [PATCH 32/41] Fix build after merge --- src/SelectionManager.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SelectionManager.test.ts b/src/SelectionManager.test.ts index 70d3fc1b..bcdca458 100644 --- a/src/SelectionManager.test.ts +++ b/src/SelectionManager.test.ts @@ -4,7 +4,7 @@ */ import { assert } from 'chai'; -import { CharMeasure } from './utils/CharMeasure'; +import { CharMeasure } from './ui/CharMeasure'; import { SelectionManager, SelectionMode } from './SelectionManager'; import { SelectionModel } from './SelectionModel'; import { BufferSet } from './BufferSet'; From e628a9be76030734ae6f13d1878d33f92286cd27 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 29 Jun 2018 11:25:17 -0700 Subject: [PATCH 33/41] Fix emoji test --- src/SelectionManager.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SelectionManager.test.ts b/src/SelectionManager.test.ts index bcdca458..4ae0c08f 100644 --- a/src/SelectionManager.test.ts +++ b/src/SelectionManager.test.ts @@ -425,9 +425,9 @@ describe('SelectionManager', () => { // TODO the case this is testing works for me in the demo webapp, // but doing it programmatically fails. buffer.lines.length = 3; - buffer.lines.set(0, stringToRow('a')); - buffer.lines.set(1, stringToRow('😁')); - buffer.lines.set(2, stringToRow('c')); + buffer.lines.set(0, stringToRow('a ')); + buffer.lines.set(1, stringArrayToRow(['😁', ' '])); + buffer.lines.set(2, stringToRow('c ')); selectionManager.selectionMode = SelectionMode.COLUMN; selectionManager.model.selectionStart = [0, 0]; From ace612e575ca7cad8384c29a7fcba57a7baeac27 Mon Sep 17 00:00:00 2001 From: Viraj Sinha Date: Fri, 29 Jun 2018 20:15:17 -0700 Subject: [PATCH 34/41] Fixes for merging column select --- src/SelectionManager.ts | 56 +++++++++++++++-------------------------- src/Terminal.ts | 2 +- src/xterm.css | 2 +- typings/xterm.d.ts | 2 +- 4 files changed, 23 insertions(+), 39 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index aa0bc59e..285375e9 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -83,11 +83,6 @@ export class SelectionManager extends EventEmitter implements ISelectionManager */ protected _activeSelectionMode: SelectionMode; - /** - * The modifier keys required to trigger block select mode with left click + drag - */ - private _columnSelectRequiredModifiers: string[]; - /** * A setInterval timer that is active while the mouse is down whose callback * scrolls the viewport when necessary. @@ -120,7 +115,6 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._model = new SelectionModel(_terminal); this._activeSelectionMode = SelectionMode.NORMAL; - this._columnSelectRequiredModifiers = this._initColumnSelectModifierKeys(); } public dispose(): void { @@ -192,13 +186,17 @@ export class SelectionManager extends EventEmitter implements ISelectionManager const result: string[] = []; if (this._activeSelectionMode === SelectionMode.COLUMN) { + // Ignore zero width selections - if (start[0] !== end[0]) { - for (let i = start[1]; i <= end[1]; i++) { - const lineText = this._buffer.translateBufferLineToString(i, true, start[0], end[0]); - result.push(lineText); - } + if (start[0] === end[0]) { + return ''; } + + for (let i = start[1]; i <= end[1]; i++) { + const lineText = this._buffer.translateBufferLineToString(i, true, start[0], end[0]); + result.push(lineText); + } + } else { // Get first row const startRowEndCol = start[1] === end[1] ? end[0] : null; @@ -272,7 +270,11 @@ export class SelectionManager extends EventEmitter implements ISelectionManager */ private _refresh(): void { this._refreshAnimationFrame = null; - this.emit('refresh', { start: this._model.finalSelectionStart, end: this._model.finalSelectionEnd, columnSelectMode: this._activeSelectionMode === SelectionMode.COLUMN }); + this.emit('refresh', { + start: this._model.finalSelectionStart, + end: this._model.finalSelectionEnd, + columnSelectMode: this._activeSelectionMode === SelectionMode.COLUMN + }); } /** @@ -425,7 +427,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._onIncrementalClick(event); } else { if (event.detail === 1) { - if (this.isColumnSelectMode(event)) { + if (this.shouldColumnSelect(event)) { this._onColumnSelectSingleClick(event); } else { this._onSingleClick(event); @@ -534,19 +536,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager } /** - * Configures the modifier key for enabling column selection mode - */ - private _initColumnSelectModifierKeys(): string[] { - if (this._terminal.browser.isMac) { - return ['altKey']; - } - - // Linux and Windows - return ['shiftKey']; - } - - /** - * Begin a block selection + * Begin a column selection */ private _onColumnSelectSingleClick(event: MouseEvent): void { this._onSingleClick(event); // Perform all the normal setup actions @@ -554,17 +544,11 @@ export class SelectionManager extends EventEmitter implements ISelectionManager } /** - * Checks if all required key modifiers are pressed in order to enable block - * select mode - * @param event the mouse click event + * Returns whether the selection manager should operate in column select mode + * @param event the mouse or keyboard event */ - public isColumnSelectMode(event: KeyboardEvent | MouseEvent): boolean { - for (let i = 0; i < this._columnSelectRequiredModifiers.length; i++) { - if (!(event)[this._columnSelectRequiredModifiers[i]]) { - return false; - } - } - return true; + public shouldColumnSelect(event: KeyboardEvent | MouseEvent): boolean { + return event.altKey && !this._terminal.options.macOptionClickForcesSelection; } /** diff --git a/src/Terminal.ts b/src/Terminal.ts index 6993a2ff..f23c2a59 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1112,7 +1112,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * Change the cursor style for different selection modes */ public updateCursorStyle(ev: KeyboardEvent): void { - if (this.selectionManager.isColumnSelectMode(ev)) { + if (this.selectionManager && this.selectionManager.shouldColumnSelect(ev)) { this.element.classList.add('xterm-cursor-crosshair'); } else { this.element.classList.remove('xterm-cursor-crosshair'); diff --git a/src/xterm.css b/src/xterm.css index b435a63f..8e129f50 100644 --- a/src/xterm.css +++ b/src/xterm.css @@ -140,7 +140,7 @@ } .xterm.xterm-cursor-crosshair { - /* Block selection mode */ + /* Column selection mode */ cursor: crosshair; } diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 94fa0432..2bb70815 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -571,7 +571,7 @@ declare module 'xterm' { * Retrieves an option's value from the terminal. * @param key The option key. */ - getOption(key: 'columnSelectModifiers' | 'colors'): string[]; + getOption(key: 'colors'): string[]; /** * Retrieves an option's value from the terminal. * @param key The option key. From 3a4ad3717736f889258660bfedc99aa210c7b5b5 Mon Sep 17 00:00:00 2001 From: Viraj Sinha Date: Fri, 29 Jun 2018 20:19:18 -0700 Subject: [PATCH 35/41] Added mac platform check before we try to use mac specific config var --- src/SelectionManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 285375e9..14b22199 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -548,7 +548,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager * @param event the mouse or keyboard event */ public shouldColumnSelect(event: KeyboardEvent | MouseEvent): boolean { - return event.altKey && !this._terminal.options.macOptionClickForcesSelection; + return event.altKey && !(Browser.isMac && this._terminal.options.macOptionClickForcesSelection); } /** From 117c79e4e726aebbdadb8ab252cf1ae397a7e461 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 30 Jun 2018 10:11:48 -0700 Subject: [PATCH 36/41] Polish --- src/SelectionManager.ts | 18 ++---------------- src/Terminal.ts | 3 +-- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 14b22199..1de14e70 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -186,7 +186,6 @@ export class SelectionManager extends EventEmitter implements ISelectionManager const result: string[] = []; if (this._activeSelectionMode === SelectionMode.COLUMN) { - // Ignore zero width selections if (start[0] === end[0]) { return ''; @@ -196,7 +195,6 @@ export class SelectionManager extends EventEmitter implements ISelectionManager const lineText = this._buffer.translateBufferLineToString(i, true, start[0], end[0]); result.push(lineText); } - } else { // Get first row const startRowEndCol = start[1] === end[1] ? end[0] : null; @@ -427,11 +425,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager this._onIncrementalClick(event); } else { if (event.detail === 1) { - if (this.shouldColumnSelect(event)) { - this._onColumnSelectSingleClick(event); - } else { - this._onSingleClick(event); - } + this._onSingleClick(event); } else if (event.detail === 2) { this._onDoubleClick(event); } else if (event.detail === 3) { @@ -482,7 +476,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager private _onSingleClick(event: MouseEvent): void { this._model.selectionStartLength = 0; this._model.isSelectAllActive = false; - this._activeSelectionMode = SelectionMode.NORMAL; + this._activeSelectionMode = this.shouldColumnSelect(event) ? SelectionMode.COLUMN : SelectionMode.NORMAL; // Initialize the new selection this._model.selectionStart = this._getMouseBufferCoords(event); @@ -535,14 +529,6 @@ export class SelectionManager extends EventEmitter implements ISelectionManager } } - /** - * Begin a column selection - */ - private _onColumnSelectSingleClick(event: MouseEvent): void { - this._onSingleClick(event); // Perform all the normal setup actions - this._activeSelectionMode = SelectionMode.COLUMN; - } - /** * Returns whether the selection manager should operate in column select mode * @param event the mouse or keyboard event diff --git a/src/Terminal.ts b/src/Terminal.ts index f23c2a59..dbb2cd1e 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1509,9 +1509,8 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } } - protected _keyUp(ev: KeyboardEvent): boolean { + protected _keyUp(ev: KeyboardEvent): void { this.updateCursorStyle(ev); - return true; } /** From 054592520ef5a1c67565400d7b4324aa7caf441e Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Sun, 1 Jul 2018 18:56:58 +0100 Subject: [PATCH 37/41] Add recreate terminal after disposing to the demo --- demo/main.js | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/demo/main.js b/demo/main.js index cccf5a04..00e675f5 100644 --- a/demo/main.js +++ b/demo/main.js @@ -50,6 +50,23 @@ addDomListener(actionElements.findPrevious, 'keypress', function (e) { } }); +const disposeRecreateButtonHandler = () => { + // If the terminal exists dispose of it, otherwise recreate it + if (term) { + term.dispose(); + term = null; + window.term = null; + socket = null; + document.getElementById('dispose').innerHTML = 'Recreate Terminal'; + } + else { + createTerminal(); + document.getElementById('dispose').innerHTML = 'Dispose terminal'; + } +}; + +document.getElementById('dispose').addEventListener('click', disposeRecreateButtonHandler); + function createTerminal() { // Clean terminal while (terminalContainer.children.length) { @@ -76,15 +93,6 @@ function createTerminal() { term.fit(); term.focus(); - const buttonHandler = () => { - term.dispose(); - term = null; - window.term = null; - socket = null; - document.getElementById('dispose').removeEventListener('click', buttonHandler); - }; - document.getElementById('dispose').addEventListener('click', buttonHandler); - // fit is called within a setTimeout, cols and rows need this. setTimeout(function () { initOptions(term); From 932a8d502897d5a6cbcf2515db97d0084e20d64a Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Sun, 1 Jul 2018 19:26:50 +0100 Subject: [PATCH 38/41] Fix SelectionRenderLayer._state --- src/renderer/SelectionRenderLayer.ts | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/renderer/SelectionRenderLayer.ts b/src/renderer/SelectionRenderLayer.ts index a9542cd7..8fbcb75b 100644 --- a/src/renderer/SelectionRenderLayer.ts +++ b/src/renderer/SelectionRenderLayer.ts @@ -8,13 +8,14 @@ import { IColorSet, IRenderDimensions } from './Types'; import { BaseRenderLayer } from './BaseRenderLayer'; export class SelectionRenderLayer extends BaseRenderLayer { - private _state: {start: [number, number], end: [number, number]}; + private _state: { start: [number, number], end: [number, number], columnSelectMode?: boolean }; constructor(container: HTMLElement, zIndex: number, colors: IColorSet) { super(container, 'selection', zIndex, true, colors); this._state = { start: null, - end: null + end: null, + columnSelectMode: null }; } @@ -23,7 +24,8 @@ export class SelectionRenderLayer extends BaseRenderLayer { // Resizing the canvas discards the contents of the canvas so clear state this._state = { start: null, - end: null + end: null, + columnSelectMode: null }; } @@ -31,7 +33,8 @@ export class SelectionRenderLayer extends BaseRenderLayer { if (this._state.start && this._state.end) { this._state = { start: null, - end: null + end: null, + columnSelectMode: null }; this.clearAll(); } @@ -39,7 +42,9 @@ export class SelectionRenderLayer extends BaseRenderLayer { public onSelectionChanged(terminal: ITerminal, start: [number, number], end: [number, number], columnSelectMode: boolean): void { // Selection has not changed - if (this._state.start === start || this._state.end === end) { + if (this._areCoordinatesEqual(start, this._state.start) && + this._areCoordinatesEqual(end, this._state.end) && + columnSelectMode === this._state.columnSelectMode) { return; } @@ -91,5 +96,14 @@ export class SelectionRenderLayer extends BaseRenderLayer { // Save state for next render this._state.start = [start[0], start[1]]; this._state.end = [end[0], end[1]]; + this._state.columnSelectMode = columnSelectMode; + } + + private _areCoordinatesEqual(coord1: [number, number], coord2: [number, number]): boolean { + if (!coord1 || !coord2) { + return false; + } + + return coord1[0] === coord2[0] && coord1[1] === coord2[1]; } } From 7e393dfa8cb5d5d969ff25e2ea4c869a06f9897c Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Sun, 1 Jul 2018 22:24:51 +0100 Subject: [PATCH 39/41] Move logic to helper function --- src/renderer/SelectionRenderLayer.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/renderer/SelectionRenderLayer.ts b/src/renderer/SelectionRenderLayer.ts index 8fbcb75b..97ae67df 100644 --- a/src/renderer/SelectionRenderLayer.ts +++ b/src/renderer/SelectionRenderLayer.ts @@ -42,9 +42,7 @@ export class SelectionRenderLayer extends BaseRenderLayer { public onSelectionChanged(terminal: ITerminal, start: [number, number], end: [number, number], columnSelectMode: boolean): void { // Selection has not changed - if (this._areCoordinatesEqual(start, this._state.start) && - this._areCoordinatesEqual(end, this._state.end) && - columnSelectMode === this._state.columnSelectMode) { + if (!this._didStateChange(start, end, columnSelectMode)) { return; } @@ -99,6 +97,12 @@ export class SelectionRenderLayer extends BaseRenderLayer { this._state.columnSelectMode = columnSelectMode; } + private _didStateChange(start: [number, number], end: [number, number], columnSelectMode: boolean): boolean { + return !this._areCoordinatesEqual(start, this._state.start) || + !this._areCoordinatesEqual(end, this._state.end) || + columnSelectMode !== this._state.columnSelectMode; + } + private _areCoordinatesEqual(coord1: [number, number], coord2: [number, number]): boolean { if (!coord1 || !coord2) { return false; From ed58c42513d6444cdeb07bfb36b2d8f666502c80 Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Sun, 1 Jul 2018 22:48:38 +0100 Subject: [PATCH 40/41] Fix actions and styles --- demo/main.js | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/demo/main.js b/demo/main.js index 00e675f5..a9b11655 100644 --- a/demo/main.js +++ b/demo/main.js @@ -35,21 +35,6 @@ function setPadding() { createTerminal(); -addDomListener(paddingElement, 'change', setPadding); - -addDomListener(actionElements.findNext, 'keypress', function (e) { - if (e.key === "Enter") { - e.preventDefault(); - term.findNext(actionElements.findNext.value); - } -}); -addDomListener(actionElements.findPrevious, 'keypress', function (e) { - if (e.key === "Enter") { - e.preventDefault(); - term.findPrevious(actionElements.findPrevious.value); - } -}); - const disposeRecreateButtonHandler = () => { // If the terminal exists dispose of it, otherwise recreate it if (term) { @@ -93,6 +78,21 @@ function createTerminal() { term.fit(); term.focus(); + addDomListener(paddingElement, 'change', setPadding); + + addDomListener(actionElements.findNext, 'keypress', function (e) { + if (e.key === "Enter") { + e.preventDefault(); + term.findNext(actionElements.findNext.value); + } + }); + addDomListener(actionElements.findPrevious, 'keypress', function (e) { + if (e.key === "Enter") { + e.preventDefault(); + term.findPrevious(actionElements.findPrevious.value); + } + }); + // fit is called within a setTimeout, cols and rows need this. setTimeout(function () { initOptions(term); From af9d17760daf90834dc9ed54bb67df20114a22fd Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 2 Jul 2018 09:22:38 -0700 Subject: [PATCH 41/41] Make columnSelectMode mandatory --- src/renderer/SelectionRenderLayer.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/renderer/SelectionRenderLayer.ts b/src/renderer/SelectionRenderLayer.ts index 97ae67df..1759e112 100644 --- a/src/renderer/SelectionRenderLayer.ts +++ b/src/renderer/SelectionRenderLayer.ts @@ -8,7 +8,7 @@ import { IColorSet, IRenderDimensions } from './Types'; import { BaseRenderLayer } from './BaseRenderLayer'; export class SelectionRenderLayer extends BaseRenderLayer { - private _state: { start: [number, number], end: [number, number], columnSelectMode?: boolean }; + private _state: { start: [number, number], end: [number, number], columnSelectMode: boolean }; constructor(container: HTMLElement, zIndex: number, colors: IColorSet) { super(container, 'selection', zIndex, true, colors); @@ -72,7 +72,6 @@ export class SelectionRenderLayer extends BaseRenderLayer { const width = end[0] - startCol; const height = viewportCappedEndRow - viewportCappedStartRow + 1; this.fillCells(startCol, viewportCappedStartRow, width, height); - } else { // Draw first row const startCol = viewportStartRow === viewportCappedStartRow ? start[0] : 0;