From 80a83ff8d9b8d1ab433faab857da6907d4dd36c9 Mon Sep 17 00:00:00 2001 From: Joao Moreno Date: Mon, 24 Jul 2017 18:08:11 +0200 Subject: [PATCH 1/7] wip: alt forces selection --- src/SelectionManager.ts | 50 ++++++++++++++++++++++++++--------------- src/xterm.js | 2 +- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 8f1d0e2..6da7288 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -91,9 +91,12 @@ export class SelectionManager extends EventEmitter { */ private _refreshAnimationFrame: number; - private _bufferTrimListener: any; + /** + * Whether selection is enabled. + */ + private _enabled = true; + private _mouseMoveListener: EventListener; - private _mouseDownListener: EventListener; private _mouseUpListener: EventListener; constructor( @@ -114,10 +117,16 @@ export class SelectionManager extends EventEmitter { * Initializes listener variables. */ private _initListeners() { - this._bufferTrimListener = (amount: number) => this._onTrim(amount); this._mouseMoveListener = event => this._onMouseMove(event); - this._mouseDownListener = event => this._onMouseDown(event); this._mouseUpListener = event => this._onMouseUp(event); + + this._rowContainer.addEventListener('mousedown', event => this._onMouseDown(event)); + + // Only adjust the selection on trim, shiftElements is rarely used (only in + // reverseIndex) and delete in a splice is only ever used when the same + // number of elements was just added. Given this is could actually be + // beneficial to leave the selection as is for these cases. + this._buffer.on('trim', (amount: number) => this._onTrim(amount)); } /** @@ -126,20 +135,14 @@ export class SelectionManager extends EventEmitter { */ public disable() { this.clearSelection(); - this._buffer.off('trim', this._bufferTrimListener); - this._rowContainer.removeEventListener('mousedown', this._mouseDownListener); + this._enabled = false; } /** * Enable the selection manager. */ public enable() { - // Only adjust the selection on trim, shiftElements is rarely used (only in - // reverseIndex) and delete in a splice is only ever used when the same - // number of elements was just added. Given this is could actually be - // beneficial to leave the selection as is for these cases. - this._buffer.on('trim', this._bufferTrimListener); - this._rowContainer.addEventListener('mousedown', this._mouseDownListener); + this._enabled = true; } /** @@ -314,6 +317,17 @@ export class SelectionManager extends EventEmitter { * @param event The mousedown event. */ private _onMouseDown(event: MouseEvent) { + // Ignore this event when selection is disabled + if (!this._enabled) { + if (!event.altKey) { + return; + } else { + + // Don't send the mouse down event to the current process + event.stopPropagation(); + } + } + // Only action the primary button if (event.button !== 0) { return; @@ -329,11 +343,11 @@ export class SelectionManager extends EventEmitter { this._onShiftClick(event); } else { if (event.detail === 1) { - this._onSingleClick(event); + this._onSingleClick(event); } else if (event.detail === 2) { - this._onDoubleClick(event); + this._onDoubleClick(event); } else if (event.detail === 3) { - this._onTripleClick(event); + this._onTripleClick(event); } } @@ -479,8 +493,8 @@ export class SelectionManager extends EventEmitter { // Only draw here if the selection changes. if (!previousSelectionEnd || - previousSelectionEnd[0] !== this._model.selectionEnd[0] || - previousSelectionEnd[1] !== this._model.selectionEnd[1]) { + previousSelectionEnd[0] !== this._model.selectionEnd[0] || + previousSelectionEnd[1] !== this._model.selectionEnd[1]) { this.refresh(true); } } @@ -603,7 +617,7 @@ export class SelectionManager extends EventEmitter { const start = startIndex + charOffset - leftWideCharCount; const length = Math.min(endIndex - startIndex + leftWideCharCount + rightWideCharCount + 1/*include endIndex char*/, this._terminal.cols); - return {start, length}; + return { start, length }; } /** diff --git a/src/xterm.js b/src/xterm.js index b1f2069..8685cba 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -517,7 +517,7 @@ Terminal.prototype.initGlobal = function() { on(this.element, 'copy', event => { // If mouse events are active it means the selection manager is disabled and // copy should be handled by the host program. - if (this.mouseEvents) { + if (!this.selectionManager.hasSelection) { return; } copyHandler(event, term, this.selectionManager); From 9a30b98880fe2fc511b077f20994702ed7301ce5 Mon Sep 17 00:00:00 2001 From: Joao Moreno Date: Tue, 25 Jul 2017 10:45:45 +0200 Subject: [PATCH 2/7] safeguard npe --- src/xterm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xterm.js b/src/xterm.js index 8685cba..b0ed3eb 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -517,7 +517,7 @@ Terminal.prototype.initGlobal = function() { on(this.element, 'copy', event => { // If mouse events are active it means the selection manager is disabled and // copy should be handled by the host program. - if (!this.selectionManager.hasSelection) { + if (!term.hasSelection) { return; } copyHandler(event, term, this.selectionManager); From f872ced863de8eb20f28f7819dbf4b68004c54bf Mon Sep 17 00:00:00 2001 From: Joao Moreno Date: Tue, 25 Jul 2017 11:30:03 +0200 Subject: [PATCH 3/7] oops, this is a function --- src/xterm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xterm.js b/src/xterm.js index b0ed3eb..bf7c95d 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -517,7 +517,7 @@ Terminal.prototype.initGlobal = function() { on(this.element, 'copy', event => { // If mouse events are active it means the selection manager is disabled and // copy should be handled by the host program. - if (!term.hasSelection) { + if (!term.hasSelection()) { return; } copyHandler(event, term, this.selectionManager); From 0e0ecc2d6a64af0876bebff5f5ab901bb2ed16f9 Mon Sep 17 00:00:00 2001 From: Joao Moreno Date: Tue, 25 Jul 2017 17:11:41 +0200 Subject: [PATCH 4/7] listen to shift when in Linux --- src/SelectionManager.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 6da7288..11fb65b 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -319,7 +319,7 @@ export class SelectionManager extends EventEmitter { private _onMouseDown(event: MouseEvent) { // Ignore this event when selection is disabled if (!this._enabled) { - if (!event.altKey) { + if (!(Browser.isLinux ? event.shiftKey : event.altKey)) { return; } else { @@ -339,8 +339,8 @@ export class SelectionManager extends EventEmitter { // Reset drag scroll state this._dragScrollAmount = 0; - if (event.shiftKey) { - this._onShiftClick(event); + if (this._enabled && event.shiftKey) { + this._onIncrementalClick(event); } else { if (event.detail === 1) { this._onSingleClick(event); @@ -376,11 +376,11 @@ export class SelectionManager extends EventEmitter { } /** - * Performs a shift click, setting the selection end position to the mouse + * Performs an incremental click, setting the selection end position to the mouse * position. * @param event The mouse event. */ - private _onShiftClick(event: MouseEvent): void { + private _onIncrementalClick(event: MouseEvent): void { if (this._model.selectionStart) { this._model.selectionEnd = this._getMouseBufferCoords(event); } From 9e80a1246238c4c1acfe6cc27c04607f83acf933 Mon Sep 17 00:00:00 2001 From: Joao Moreno Date: Tue, 25 Jul 2017 17:17:48 +0200 Subject: [PATCH 5/7] fix tests --- src/SelectionManager.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/SelectionManager.test.ts b/src/SelectionManager.test.ts index 668fd92..c2173d7 100644 --- a/src/SelectionManager.test.ts +++ b/src/SelectionManager.test.ts @@ -45,6 +45,7 @@ describe('SelectionManager', () => { dom = new jsdom.JSDOM(''); window = dom.window; document = window.document; + rowContainer = document.createElement('div'); terminal = { cols: 80, rows: 2 }; terminal.scrollback = 100; terminal.buffers = new BufferSet(terminal); From 6a3b39b4283d279a371b2881bd41de98b5af2350 Mon Sep 17 00:00:00 2001 From: Joao Moreno Date: Wed, 26 Jul 2017 09:07:20 +0200 Subject: [PATCH 6/7] fix context menu in disabled state --- src/SelectionManager.ts | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 11fb65b..55e9573 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -317,15 +317,10 @@ export class SelectionManager extends EventEmitter { * @param event The mousedown event. */ private _onMouseDown(event: MouseEvent) { - // Ignore this event when selection is disabled - if (!this._enabled) { - if (!(Browser.isLinux ? event.shiftKey : event.altKey)) { - return; - } else { - - // Don't send the mouse down event to the current process - event.stopPropagation(); - } + // If we have selection, we want the context menu on right click + if (event.button === 2 && this.hasSelection) { + event.stopPropagation(); + return; } // Only action the primary button @@ -333,6 +328,18 @@ export class SelectionManager extends EventEmitter { return; } + // Allow selection when using a specific modifier key, even when disabled + if (!this._enabled) { + const shouldForceSelection = Browser.isMac ? event.altKey : event.shiftKey; + + if (!shouldForceSelection) { + return; + } + + // Don't send the mouse down event to the current process, we want to select + event.stopPropagation(); + } + // Tell the browser not to start a regular selection event.preventDefault(); From 3479614c732305b4174bf438e1ac3022aa3b3aa4 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 28 Jul 2017 14:06:55 -0700 Subject: [PATCH 7/7] Disable selection override on Windows/Linux --- src/SelectionManager.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 55e9573..f6fb44d 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -317,7 +317,8 @@ export class SelectionManager extends EventEmitter { * @param event The mousedown event. */ private _onMouseDown(event: MouseEvent) { - // If we have selection, we want the context menu on right click + // If we have selection, we want the context menu on right click even if the + // terminal is in mouse mode. if (event.button === 2 && this.hasSelection) { event.stopPropagation(); return; @@ -330,7 +331,7 @@ export class SelectionManager extends EventEmitter { // Allow selection when using a specific modifier key, even when disabled if (!this._enabled) { - const shouldForceSelection = Browser.isMac ? event.altKey : event.shiftKey; + const shouldForceSelection = Browser.isMac && event.altKey; if (!shouldForceSelection) { return;