From e0c28bc7d2fffd1019c82e865b343c665cb084b7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 30 Sep 2017 09:32:33 -0400 Subject: [PATCH] Fix selecting words containing ZWJ emojis --- src/SelectionManager.test.ts | 53 +++++++++++++++++++++++++++++++++++- src/SelectionManager.ts | 33 ++++++++++++++++------ 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/SelectionManager.test.ts b/src/SelectionManager.test.ts index e5bdc117..77312467 100644 --- a/src/SelectionManager.test.ts +++ b/src/SelectionManager.test.ts @@ -12,7 +12,7 @@ import { SelectionManager } from './SelectionManager'; import { SelectionModel } from './SelectionModel'; import { BufferSet } from './BufferSet'; import { MockTerminal } from './utils/TestUtils.test'; -import { LineData } from './Types'; +import { LineData, CharData } from './Types'; class TestSelectionManager extends SelectionManager { constructor( @@ -66,6 +66,10 @@ describe('SelectionManager', () => { return result; } + function stringArrayToRow(chars: string[]): LineData { + return chars.map(c => [0, c, 1, c.charCodeAt(0)]); + } + describe('_selectWordAt', () => { it('should expand selection for normal width chars', () => { buffer.lines.set(0, stringToRow('foo bar')); @@ -185,6 +189,53 @@ describe('SelectionManager', () => { selectionManager.selectWordAt([15, 0]); assert.equal(selectionManager.selectionText, 'ij"'); }); + describe('emoji', () => { + it('should treat a single emoji as a word when wrapped in spaces', () => { + buffer.lines.set(0, stringToRow(' ⚽ a')); // The a is here to prevent the space being trimmed in selectionText + selectionManager.selectWordAt([0, 0]); + assert.equal(selectionManager.selectionText, ' '); + selectionManager.selectWordAt([1, 0]); + assert.equal(selectionManager.selectionText, '⚽'); + selectionManager.selectWordAt([2, 0]); + assert.equal(selectionManager.selectionText, ' '); + }); + it('should treat multiple emojis as a word when wrapped in spaces', () => { + buffer.lines.set(0, stringToRow(' ⚽⚽ a')); // The a is here to prevent the space being trimmed in selectionText + selectionManager.selectWordAt([0, 0]); + assert.equal(selectionManager.selectionText, ' '); + selectionManager.selectWordAt([1, 0]); + assert.equal(selectionManager.selectionText, '⚽⚽'); + selectionManager.selectWordAt([2, 0]); + assert.equal(selectionManager.selectionText, '⚽⚽'); + selectionManager.selectWordAt([3, 0]); + assert.equal(selectionManager.selectionText, ' '); + }); + it('should treat emojis using the zero-width-joiner as a single word', () => { + buffer.lines.set(0, stringArrayToRow([ + ' ', + '👨‍', // Note that the first 3 emojis include the invisible ZWJ char + '👩‍', + '👧‍', + '👦', + ' ', + 'a' + ])); // The a is here to prevent the space being trimmed in selectionText + selectionManager.selectWordAt([0, 0]); + assert.equal(selectionManager.selectionText, ' '); + // ZWJ emojis do not combine in the terminal so the family emoji used here consumed 4 cells + // The selection text should retain ZWJ chars despite not combining on the terminal + selectionManager.selectWordAt([1, 0]); + assert.equal(selectionManager.selectionText, '👨‍👩‍👧‍👦'); + selectionManager.selectWordAt([2, 0]); + assert.equal(selectionManager.selectionText, '👨‍👩‍👧‍👦'); + selectionManager.selectWordAt([3, 0]); + assert.equal(selectionManager.selectionText, '👨‍👩‍👧‍👦'); + selectionManager.selectWordAt([4, 0]); + assert.equal(selectionManager.selectionText, '👨‍👩‍👧‍👦'); + selectionManager.selectWordAt([5, 0]); + assert.equal(selectionManager.selectionText, ' '); + }); + }); }); describe('_selectLineAt', () => { diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index f8464d09..27aeafff 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -546,10 +546,13 @@ export class SelectionManager extends EventEmitter implements ISelectionManager for (let i = 0; coords[0] >= i; i++) { const char = bufferLine[i]; if (char[CHAR_DATA_WIDTH_INDEX] === 0) { - // Wide characters aren't included in the line string so decrement the index + // Wide characters aren't included in the line string so decrement the + // index so the index is back on the wide character. charIndex--; - } else if (char[CHAR_DATA_CHAR_INDEX].length > 1) { - // Emojis take up multiple characters, so adjust accordingly + } else if (char[CHAR_DATA_CHAR_INDEX].length > 1 && coords[0] !== i) { + // Emojis take up multiple characters, so adjust accordingly. For these + // we don't want ot include the character at the column as we're + // returning the start index in the string, not the end index. charIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; } } @@ -577,13 +580,15 @@ export class SelectionManager extends EventEmitter implements ISelectionManager const line = this._buffer.translateBufferLineToString(coords[1], false); // Get actual index, taking into consideration wide characters - let endIndex = this._convertViewportColToCharacterIndex(bufferLine, coords); - let startIndex = endIndex; + let startIndex = this._convertViewportColToCharacterIndex(bufferLine, coords); + let endIndex = startIndex; // Record offset to be used later const charOffset = coords[0] - startIndex; let leftWideCharCount = 0; let rightWideCharCount = 0; + let leftLongCharOffset = 0; + let rightLongCharOffset = 0; if (line.charAt(startIndex) === ' ') { // Expand until non-whitespace is hit @@ -600,6 +605,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager // character is hit, it is recorded and the column index is adjusted. let startCol = coords[0]; let endCol = coords[0]; + // Consider the initial position, skip it and increment the wide char // variable if (bufferLine[startCol][CHAR_DATA_WIDTH_INDEX] === 0) { @@ -610,8 +616,15 @@ export class SelectionManager extends EventEmitter implements ISelectionManager rightWideCharCount++; endCol++; } + + // Adjust the end index for characters whose length are > 1 (emojis) + if (bufferLine[endCol][CHAR_DATA_CHAR_INDEX].length > 1) { + rightLongCharOffset += bufferLine[endCol][CHAR_DATA_CHAR_INDEX].length - 1; + endIndex += bufferLine[endCol][CHAR_DATA_CHAR_INDEX].length - 1; + } + // Expand the string in both directions until a space is hit - while (startIndex > 0 && !this._isCharWordSeparator(bufferLine[startCol - 1])) { + while (startCol > 0 && startIndex > 0 && !this._isCharWordSeparator(bufferLine[startCol - 1])) { const char = bufferLine[startCol - 1]; if (char[CHAR_DATA_WIDTH_INDEX] === 0) { // If the next character is a wide char, record it and skip the column @@ -620,12 +633,13 @@ export class SelectionManager extends EventEmitter implements ISelectionManager } else if (char[CHAR_DATA_CHAR_INDEX].length > 1) { // If the next character's string is longer than 1 char (eg. emoji), // adjust the index + leftLongCharOffset += char[CHAR_DATA_CHAR_INDEX].length - 1; startIndex -= char[CHAR_DATA_CHAR_INDEX].length - 1; } startIndex--; startCol--; } - while (endIndex + 1 < line.length && !this._isCharWordSeparator(bufferLine[endCol + 1])) { + while (endCol < bufferLine.length && endIndex + 1 < line.length && !this._isCharWordSeparator(bufferLine[endCol + 1])) { const char = bufferLine[endCol + 1]; if (char[CHAR_DATA_WIDTH_INDEX] === 2) { // If the next character is a wide char, record it and skip the column @@ -634,6 +648,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager } else if (char[CHAR_DATA_CHAR_INDEX].length > 1) { // If the next character's string is longer than 1 char (eg. emoji), // adjust the index + rightLongCharOffset += char[CHAR_DATA_CHAR_INDEX].length - 1; endIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; } endIndex++; @@ -641,8 +656,8 @@ export class SelectionManager extends EventEmitter implements ISelectionManager } } - const start = startIndex + charOffset - leftWideCharCount; - const length = Math.min(endIndex - startIndex + leftWideCharCount + rightWideCharCount + 1/*include endIndex char*/, this._terminal.cols); + const start = startIndex + charOffset - leftWideCharCount + leftLongCharOffset; + const length = Math.min(endIndex - startIndex + leftWideCharCount + rightWideCharCount - leftLongCharOffset - rightLongCharOffset + 1/*include endIndex char*/, this._terminal.cols); return { start, length }; }