From efb939d171409acd3d6723d97c7cc0ab48727432 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 14 Jun 2022 06:51:44 -0700 Subject: [PATCH] Fix column selection issues The following parts were not respecting the column selection edge case where the start column was greater than the end column while the start row was less than the end row: - Webgl renderer - DOM renderer - Selection service (ie. getSelection(), I don't think this was a regression) Fixes #3855 --- addons/xterm-addon-webgl/src/WebglRenderer.ts | 8 ++++++-- src/browser/renderer/dom/DomRenderer.ts | 3 ++- src/browser/renderer/dom/DomRendererRowFactory.ts | 8 ++++++-- src/browser/services/SelectionService.ts | 6 +++++- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts index 8edda780..ef31a959 100644 --- a/addons/xterm-addon-webgl/src/WebglRenderer.ts +++ b/addons/xterm-addon-webgl/src/WebglRenderer.ts @@ -455,8 +455,12 @@ export class WebglRenderer extends Disposable implements IRenderer { } y -= this._terminal.buffer.active.viewportY; if (this._model.selection.columnSelectMode) { - return x >= this._model.selection.startCol && y >= this._model.selection.viewportCappedStartRow && - x < this._model.selection.endCol && y < this._model.selection.viewportCappedEndRow; + if (this._model.selection.startCol <= this._model.selection.endCol) { + return x >= this._model.selection.startCol && y >= this._model.selection.viewportCappedStartRow && + x < this._model.selection.endCol && y <= this._model.selection.viewportCappedEndRow; + } + return x < this._model.selection.startCol && y >= this._model.selection.viewportCappedStartRow && + x >= this._model.selection.endCol && y <= this._model.selection.viewportCappedEndRow; } return (y > this._model.selection.viewportStartRow && y < this._model.selection.viewportEndRow) || (this._model.selection.viewportStartRow === this._model.selection.viewportEndRow && y === this._model.selection.viewportStartRow && x >= this._model.selection.startCol && x < this._model.selection.endCol) || diff --git a/src/browser/renderer/dom/DomRenderer.ts b/src/browser/renderer/dom/DomRenderer.ts index a966e6f9..248f37ac 100644 --- a/src/browser/renderer/dom/DomRenderer.ts +++ b/src/browser/renderer/dom/DomRenderer.ts @@ -304,8 +304,9 @@ export class DomRenderer extends Disposable implements IRenderer { const documentFragment = document.createDocumentFragment(); if (columnSelectMode) { + const isXFlipped = start[0] > end[0]; documentFragment.appendChild( - this._createSelectionElement(viewportCappedStartRow, start[0], end[0], viewportCappedEndRow - viewportCappedStartRow + 1) + this._createSelectionElement(viewportCappedStartRow, isXFlipped ? end[0] : start[0], isXFlipped ? start[0] : end[0], viewportCappedEndRow - viewportCappedStartRow + 1) ); } else { // Draw first row diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index c37fd485..fadf5032 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -322,8 +322,12 @@ export class DomRendererRowFactory { return false; } if (this._columnSelectMode) { - return x >= start[0] && y >= start[1] && - x < end[0] && y < end[1]; + if (start[0] <= end[0]) { + return x >= start[0] && y >= start[1] && + x < end[0] && y <= end[1]; + } + return x < start[0] && y >= start[1] && + x >= end[0] && y <= end[1]; } return (y > start[1] && y < end[1]) || (start[1] === end[1] && y === start[1] && x >= start[0] && x < end[0]) || diff --git a/src/browser/services/SelectionService.ts b/src/browser/services/SelectionService.ts index 57ba048f..6b5c1425 100644 --- a/src/browser/services/SelectionService.ts +++ b/src/browser/services/SelectionService.ts @@ -207,8 +207,12 @@ export class SelectionService extends Disposable implements ISelectionService { return ''; } + // For column selection it's not enough to rely on final selection's swapping of reversed + // values, it also needs the x coordinates to swap independently of the y coordinate is needed + const startCol = start[0] < end[0] ? start[0] : end[0]; + const endCol = start[0] < end[0] ? end[0] : start[0]; for (let i = start[1]; i <= end[1]; i++) { - const lineText = buffer.translateBufferLineToString(i, true, start[0], end[0]); + const lineText = buffer.translateBufferLineToString(i, true, startCol, endCol); result.push(lineText); } } else {