From feb2b958a44622f5e6a1bdc75b4b292f7ace99c7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 30 Sep 2017 06:13:44 -0400 Subject: [PATCH 1/8] Mostly emoji selection Selection is not aware of characters that have a size greater than 1. When the emoji is at the start of the selection it still doesn't work. Fixes #1015 --- src/Buffer.ts | 38 ++++++++++++++++++++++++++------- src/SelectionManager.ts | 47 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 2d23980d..585855db 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -208,9 +208,19 @@ export class Buffer implements IBuffer { public translateBufferLineToString(lineIndex: number, trimRight: boolean, startCol: number = 0, endCol: number = null): string { // Get full line let lineString = ''; - let widthAdjustedStartCol = startCol; - let widthAdjustedEndCol = endCol; const line = this.lines.get(lineIndex); + if (!line) { + return ''; + } + + // Initialize column and index values. Column values represent the actual + // cell column, indexes represent the index in the string. Indexes are + // needed here because some chars are 0 characters long (eg. after wide + // chars) and some chars are longer than 1 characters long (eg. emojis). + let startIndex = startCol; + endCol = endCol || line.length; + let endIndex = endCol; + for (let i = 0; i < line.length; i++) { const char = line[i]; lineString += char[CHAR_DATA_CHAR_INDEX]; @@ -218,29 +228,41 @@ export class Buffer implements IBuffer { // column indexes if (char[CHAR_DATA_WIDTH_INDEX] === 0) { if (startCol >= i) { - widthAdjustedStartCol--; + startIndex -= char[CHAR_DATA_CHAR_INDEX].length; } if (endCol >= i) { - widthAdjustedEndCol--; + endIndex -= char[CHAR_DATA_CHAR_INDEX].length; + } + } else { + // Adjust the columns to take glyphs that are represented by multiple + // code points into account. + if (char[CHAR_DATA_CHAR_INDEX].length > 1) { + if (startCol >= i) { + startIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; + } + if (endCol >= i) { + endIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; + } } } + // TODO: startCol needs to be emoji-aware, currently each emoji code point is + // consuming addition space in the selection text } // Calculate the final end col by trimming whitespace on the right of the // line if needed. - let finalEndCol = widthAdjustedEndCol || line.length; if (trimRight) { const rightWhitespaceIndex = lineString.search(/\s+$/); if (rightWhitespaceIndex !== -1) { - finalEndCol = Math.min(finalEndCol, rightWhitespaceIndex); + endIndex = Math.min(endIndex, rightWhitespaceIndex); } // Return the empty string if only trimmed whitespace is selected - if (finalEndCol <= widthAdjustedStartCol) { + if (endIndex <= startIndex) { return ''; } } - return lineString.substring(widthAdjustedStartCol, finalEndCol); + return lineString.substring(startIndex, endIndex); } /** diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 28ed1522..65f6a57e 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -11,7 +11,7 @@ import { EventEmitter } from './EventEmitter'; import { ITerminal, ICircularList, ISelectionManager, IBuffer } from './Interfaces'; import { SelectionModel } from './SelectionModel'; import { LineData } from './Types'; -import { CHAR_DATA_WIDTH_INDEX } from './Buffer'; +import { CHAR_DATA_WIDTH_INDEX, CHAR_DATA_CHAR_INDEX } from './Buffer'; /** * The number of pixels the mouse needs to be above or below the viewport in @@ -281,6 +281,9 @@ export class SelectionManager extends EventEmitter implements ISelectionManager // Convert to 0-based coords[0]--; coords[1]--; + + console.log('coords', coords); + // Convert viewport coords to buffer coords coords[1] += this._terminal.buffer.ydisp; return coords; @@ -545,9 +548,16 @@ 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 charIndex--; + // } + } else if (char[CHAR_DATA_CHAR_INDEX].length > 1) { + console.log('char length > 1', char[CHAR_DATA_CHAR_INDEX], char[CHAR_DATA_CHAR_INDEX].length); + // Emojis take up multiple characters, so adjust accordingly + charIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; } } + console.log('character index: ', charIndex); return charIndex; } @@ -606,20 +616,47 @@ export class SelectionManager extends EventEmitter implements ISelectionManager endCol++; } // Expand the string in both directions until a space is hit - while (startIndex > 0 && !this._isCharWordSeparator(line.charAt(startIndex - 1))) { - if (bufferLine[startCol - 1][CHAR_DATA_WIDTH_INDEX] === 0) { + let nextCharData = startIndex > 0 ? bufferLine[startCol - 1] : null; + + + +// TODO: Need to make sure that characters whose strings are longer than 1 get compensated for +// Double click words should expand to the spaces. + + + // while (startIndex > 0 && !this._isCharWordSeparator(line.charAt(startIndex - 1))) { + console.log('start char: ' + bufferLine[startCol]); + console.log('scan backwards'); + console.log(' startIndex:',startIndex); + while (startIndex > 0 && !this._isCharWordSeparator(bufferLine[startCol - 1][CHAR_DATA_CHAR_INDEX])) { + const char = bufferLine[startCol - 1]; + console.log(' char: ' + char); + if (char[CHAR_DATA_WIDTH_INDEX] === 0) { // If the next character is a wide char, record it and skip the column leftWideCharCount++; startCol--; + } else if (char[CHAR_DATA_CHAR_INDEX].length > 1) { + startIndex -= char[CHAR_DATA_CHAR_INDEX].length - 1; +console.log('x', char[CHAR_DATA_CHAR_INDEX], char[CHAR_DATA_CHAR_INDEX].length); } startIndex--; startCol--; } - while (endIndex + 1 < line.length && !this._isCharWordSeparator(line.charAt(endIndex + 1))) { - if (bufferLine[endCol + 1][CHAR_DATA_WIDTH_INDEX] === 2) { + console.log('scan forwards'); + // while (endIndex + 1 < line.length && !this._isCharWordSeparator(line.charAt(endIndex + 1))) { + console.log(' first checking: ',bufferLine[endCol + 1]); + console.log(' endIndex:',endIndex); + console.log(' line:',line); + console.log(' line.length:',line.length); + while (endIndex + 1 < line.length && !this._isCharWordSeparator(bufferLine[endCol + 1][CHAR_DATA_CHAR_INDEX])) { + const char = bufferLine[endCol + 1]; + console.log(' char: ' + char); + if (char[CHAR_DATA_WIDTH_INDEX] === 2) { // If the next character is a wide char, record it and skip the column rightWideCharCount++; endCol++; + } else if (char[CHAR_DATA_CHAR_INDEX].length > 1) { + startIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; } endIndex++; endCol++; From fa86f31f36fbef234d4dffa987c0331e38062258 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 30 Sep 2017 06:30:00 -0400 Subject: [PATCH 2/8] Fix copying emoji characters on the edge of selection Fixes #1015 --- src/Buffer.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 585855db..9c5c717f 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -237,16 +237,14 @@ export class Buffer implements IBuffer { // Adjust the columns to take glyphs that are represented by multiple // code points into account. if (char[CHAR_DATA_CHAR_INDEX].length > 1) { - if (startCol >= i) { + if (startCol > i) { startIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; } - if (endCol >= i) { + if (endCol > i) { endIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; } } } - // TODO: startCol needs to be emoji-aware, currently each emoji code point is - // consuming addition space in the selection text } // Calculate the final end col by trimming whitespace on the right of the From 3e29a03f874202d74e464e4083565ccefe9977a1 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 30 Sep 2017 07:54:52 -0400 Subject: [PATCH 3/8] Fix rendering issues for emojis that come out as wide chars The fix is to disallow non-single cell width chars to be overlapping --- src/renderer/TextRenderLayer.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/renderer/TextRenderLayer.ts b/src/renderer/TextRenderLayer.ts index e319efe7..eb163c1c 100644 --- a/src/renderer/TextRenderLayer.ts +++ b/src/renderer/TextRenderLayer.ts @@ -183,10 +183,15 @@ export class TextRenderLayer extends BaseRenderLayer { } /** - * Whether a character is overlapping to the - * next cell. + * Whether a character is overlapping to the next cell. */ private _isOverlapping(charData: CharData): boolean { + // Only single cell characters can be overlapping, rendering issues can + // occur without this check + if (charData[CHAR_DATA_WIDTH_INDEX] !== 1) { + return false; + } + // We assume that any ascii character will not overlap const code = charData[CHAR_DATA_CODE_INDEX]; if (code < 256) { From 4828c4a6faa15d5d4609b4921d28254bdd3fa6eb Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 30 Sep 2017 07:59:06 -0400 Subject: [PATCH 4/8] Clean up logs/comments --- src/SelectionManager.ts | 32 +++++--------------------------- 1 file changed, 5 insertions(+), 27 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 65f6a57e..9ba66efd 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -282,8 +282,6 @@ export class SelectionManager extends EventEmitter implements ISelectionManager coords[0]--; coords[1]--; - console.log('coords', coords); - // Convert viewport coords to buffer coords coords[1] += this._terminal.buffer.ydisp; return coords; @@ -550,14 +548,11 @@ export class SelectionManager extends EventEmitter implements ISelectionManager if (char[CHAR_DATA_WIDTH_INDEX] === 0) { // Wide characters aren't included in the line string so decrement the index charIndex--; - // } } else if (char[CHAR_DATA_CHAR_INDEX].length > 1) { - console.log('char length > 1', char[CHAR_DATA_CHAR_INDEX], char[CHAR_DATA_CHAR_INDEX].length); // Emojis take up multiple characters, so adjust accordingly charIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; } } - console.log('character index: ', charIndex); return charIndex; } @@ -616,47 +611,30 @@ export class SelectionManager extends EventEmitter implements ISelectionManager endCol++; } // Expand the string in both directions until a space is hit - let nextCharData = startIndex > 0 ? bufferLine[startCol - 1] : null; - - - -// TODO: Need to make sure that characters whose strings are longer than 1 get compensated for -// Double click words should expand to the spaces. - - - // while (startIndex > 0 && !this._isCharWordSeparator(line.charAt(startIndex - 1))) { - console.log('start char: ' + bufferLine[startCol]); - console.log('scan backwards'); - console.log(' startIndex:',startIndex); while (startIndex > 0 && !this._isCharWordSeparator(bufferLine[startCol - 1][CHAR_DATA_CHAR_INDEX])) { const char = bufferLine[startCol - 1]; - console.log(' char: ' + char); if (char[CHAR_DATA_WIDTH_INDEX] === 0) { // If the next character is a wide char, record it and skip the column leftWideCharCount++; startCol--; } 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 startIndex -= char[CHAR_DATA_CHAR_INDEX].length - 1; -console.log('x', char[CHAR_DATA_CHAR_INDEX], char[CHAR_DATA_CHAR_INDEX].length); } startIndex--; startCol--; } - console.log('scan forwards'); - // while (endIndex + 1 < line.length && !this._isCharWordSeparator(line.charAt(endIndex + 1))) { - console.log(' first checking: ',bufferLine[endCol + 1]); - console.log(' endIndex:',endIndex); - console.log(' line:',line); - console.log(' line.length:',line.length); while (endIndex + 1 < line.length && !this._isCharWordSeparator(bufferLine[endCol + 1][CHAR_DATA_CHAR_INDEX])) { const char = bufferLine[endCol + 1]; - console.log(' char: ' + char); if (char[CHAR_DATA_WIDTH_INDEX] === 2) { // If the next character is a wide char, record it and skip the column rightWideCharCount++; endCol++; } else if (char[CHAR_DATA_CHAR_INDEX].length > 1) { - startIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; + // If the next character's string is longer than 1 char (eg. emoji), + // adjust the index + endIndex += char[CHAR_DATA_CHAR_INDEX].length - 1; } endIndex++; endCol++; From 58259f8bd965ec23fc8e07a588bb8a03c48ed895 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 30 Sep 2017 08:30:19 -0400 Subject: [PATCH 5/8] Fix select word on wide characters --- src/Buffer.ts | 4 ++-- src/SelectionManager.ts | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 9c5c717f..8922c3d6 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -228,10 +228,10 @@ export class Buffer implements IBuffer { // column indexes if (char[CHAR_DATA_WIDTH_INDEX] === 0) { if (startCol >= i) { - startIndex -= char[CHAR_DATA_CHAR_INDEX].length; + startIndex--; } if (endCol >= i) { - endIndex -= char[CHAR_DATA_CHAR_INDEX].length; + endIndex--; } } else { // Adjust the columns to take glyphs that are represented by multiple diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 9ba66efd..f8464d09 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -10,7 +10,7 @@ import { CircularList } from './utils/CircularList'; import { EventEmitter } from './EventEmitter'; import { ITerminal, ICircularList, ISelectionManager, IBuffer } from './Interfaces'; import { SelectionModel } from './SelectionModel'; -import { LineData } from './Types'; +import { LineData, CharData } from './Types'; import { CHAR_DATA_WIDTH_INDEX, CHAR_DATA_CHAR_INDEX } from './Buffer'; /** @@ -611,7 +611,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager endCol++; } // Expand the string in both directions until a space is hit - while (startIndex > 0 && !this._isCharWordSeparator(bufferLine[startCol - 1][CHAR_DATA_CHAR_INDEX])) { + while (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 @@ -625,7 +625,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager startIndex--; startCol--; } - while (endIndex + 1 < line.length && !this._isCharWordSeparator(bufferLine[endCol + 1][CHAR_DATA_CHAR_INDEX])) { + while (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 @@ -674,8 +674,13 @@ export class SelectionManager extends EventEmitter implements ISelectionManager * word logic. * @param char The character to check. */ - private _isCharWordSeparator(char: string): boolean { - return WORD_SEPARATORS.indexOf(char) >= 0; + private _isCharWordSeparator(charData: CharData): boolean { + // Zero width characters are never separators as they are always to the + // right of wide characters + if (charData[CHAR_DATA_WIDTH_INDEX] === 0) { + return false; + } + return WORD_SEPARATORS.indexOf(charData[CHAR_DATA_CHAR_INDEX]) >= 0; } /** From e0c28bc7d2fffd1019c82e865b343c665cb084b7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 30 Sep 2017 09:32:33 -0400 Subject: [PATCH 6/8] 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 }; } From da4e3e6e9ffce221c5cb0e67a6fe413de159a218 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 30 Sep 2017 09:45:11 -0400 Subject: [PATCH 7/8] Improve documentation on word selection --- src/SelectionManager.ts | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 27aeafff..da484b85 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -656,8 +656,27 @@ export class SelectionManager extends EventEmitter implements ISelectionManager } } - const start = startIndex + charOffset - leftWideCharCount + leftLongCharOffset; - const length = Math.min(endIndex - startIndex + leftWideCharCount + rightWideCharCount - leftLongCharOffset - rightLongCharOffset + 1/*include endIndex char*/, this._terminal.cols); + // Incremenet the end index so it is at the start of the next character + endIndex++; + + // Calculate the start _column_, converting the the string indexes back to + // column coordinates. + const start = + startIndex // The index of the selection's start char in the line string + + charOffset // The difference between the initial char's column and index + - leftWideCharCount // The number of wide chars left of the initial char + + leftLongCharOffset; // The number of additional chars left of the initial char added by columns with strings longer than 1 (emojis) + + // Calculate the length in _columns_, converting the the string indexes back + // to column coordinates. + const length = Math.min(this._terminal.cols, // Disallow lengths larger than the terminal cols + endIndex // The index of the selection's end char in the line string + - startIndex // The index of the selection's start char in the line string + + leftWideCharCount // The number of wide chars left of the initial char + + rightWideCharCount // The number of wide chars right of the initial char (inclusive) + - leftLongCharOffset // The number of additional chars left of the initial char added by columns with strings longer than 1 (emojis) + - rightLongCharOffset); // The number of additional chars right of the initial char (inclusive) added by columns with strings longer than 1 (emojis) + return { start, length }; } From b1214063d23491522a421c3b7b5b4ea119fb296c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 30 Sep 2017 10:00:18 -0400 Subject: [PATCH 8/8] Add more complex emoji selection tests --- src/SelectionManager.test.ts | 74 ++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 7 deletions(-) diff --git a/src/SelectionManager.test.ts b/src/SelectionManager.test.ts index 77312467..8c6ea3e1 100644 --- a/src/SelectionManager.test.ts +++ b/src/SelectionManager.test.ts @@ -211,14 +211,9 @@ describe('SelectionManager', () => { assert.equal(selectionManager.selectionText, ' '); }); it('should treat emojis using the zero-width-joiner as a single word', () => { + // Note that the first 3 emojis include the invisible ZWJ char buffer.lines.set(0, stringArrayToRow([ - ' ', - '👨‍', // Note that the first 3 emojis include the invisible ZWJ char - '👩‍', - '👧‍', - '👦', - ' ', - 'a' + ' ', '👨‍', '👩‍', '👧‍', '👦', ' ', 'a' ])); // The a is here to prevent the space being trimmed in selectionText selectionManager.selectWordAt([0, 0]); assert.equal(selectionManager.selectionText, ' '); @@ -235,6 +230,71 @@ describe('SelectionManager', () => { selectionManager.selectWordAt([5, 0]); assert.equal(selectionManager.selectionText, ' '); }); + it('should treat emojis and characters joined together as a word', () => { + buffer.lines.set(0, stringToRow(' ⚽ab cd⚽ ef⚽gh')); // 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, '⚽ab'); + selectionManager.selectWordAt([2, 0]); + assert.equal(selectionManager.selectionText, '⚽ab'); + selectionManager.selectWordAt([3, 0]); + assert.equal(selectionManager.selectionText, '⚽ab'); + selectionManager.selectWordAt([4, 0]); + assert.equal(selectionManager.selectionText, ' '); + selectionManager.selectWordAt([5, 0]); + assert.equal(selectionManager.selectionText, 'cd⚽'); + selectionManager.selectWordAt([6, 0]); + assert.equal(selectionManager.selectionText, 'cd⚽'); + selectionManager.selectWordAt([7, 0]); + assert.equal(selectionManager.selectionText, 'cd⚽'); + selectionManager.selectWordAt([8, 0]); + assert.equal(selectionManager.selectionText, ' '); + selectionManager.selectWordAt([9, 0]); + assert.equal(selectionManager.selectionText, 'ef⚽gh'); + selectionManager.selectWordAt([10, 0]); + assert.equal(selectionManager.selectionText, 'ef⚽gh'); + selectionManager.selectWordAt([11, 0]); + assert.equal(selectionManager.selectionText, 'ef⚽gh'); + selectionManager.selectWordAt([12, 0]); + assert.equal(selectionManager.selectionText, 'ef⚽gh'); + selectionManager.selectWordAt([13, 0]); + assert.equal(selectionManager.selectionText, 'ef⚽gh'); + }); + it('should treat complex emojis and characters joined together as a word', () => { + // This emoji is the flag for England and is made up of: 1F3F4 E0067 E0062 E0065 E006E E0067 E007F + buffer.lines.set(0, stringArrayToRow([ + ' ', '🏴󠁧󠁢󠁥󠁮󠁧󠁿', 'a', 'b', ' ', 'c', 'd', '🏴󠁧󠁢󠁥󠁮󠁧󠁿', ' ', 'e', 'f', '🏴󠁧󠁢󠁥󠁮󠁧󠁿', 'g', 'h', ' ', '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, '🏴󠁧󠁢󠁥󠁮󠁧󠁿ab'); + selectionManager.selectWordAt([2, 0]); + assert.equal(selectionManager.selectionText, '🏴󠁧󠁢󠁥󠁮󠁧󠁿ab'); + selectionManager.selectWordAt([3, 0]); + assert.equal(selectionManager.selectionText, '🏴󠁧󠁢󠁥󠁮󠁧󠁿ab'); + selectionManager.selectWordAt([4, 0]); + assert.equal(selectionManager.selectionText, ' '); + selectionManager.selectWordAt([5, 0]); + assert.equal(selectionManager.selectionText, 'cd🏴󠁧󠁢󠁥󠁮󠁧󠁿'); + selectionManager.selectWordAt([6, 0]); + assert.equal(selectionManager.selectionText, 'cd🏴󠁧󠁢󠁥󠁮󠁧󠁿'); + selectionManager.selectWordAt([7, 0]); + assert.equal(selectionManager.selectionText, 'cd🏴󠁧󠁢󠁥󠁮󠁧󠁿'); + selectionManager.selectWordAt([8, 0]); + assert.equal(selectionManager.selectionText, ' '); + selectionManager.selectWordAt([9, 0]); + assert.equal(selectionManager.selectionText, 'ef🏴󠁧󠁢󠁥󠁮󠁧󠁿gh'); + selectionManager.selectWordAt([10, 0]); + assert.equal(selectionManager.selectionText, 'ef🏴󠁧󠁢󠁥󠁮󠁧󠁿gh'); + selectionManager.selectWordAt([11, 0]); + assert.equal(selectionManager.selectionText, 'ef🏴󠁧󠁢󠁥󠁮󠁧󠁿gh'); + selectionManager.selectWordAt([12, 0]); + assert.equal(selectionManager.selectionText, 'ef🏴󠁧󠁢󠁥󠁮󠁧󠁿gh'); + selectionManager.selectWordAt([13, 0]); + assert.equal(selectionManager.selectionText, 'ef🏴󠁧󠁢󠁥󠁮󠁧󠁿gh'); + }); }); });