From 6f6378aaf2d2b59a7c0a980f22380d567163957c Mon Sep 17 00:00:00 2001 From: Gerald Date: Fri, 29 Jan 2021 12:20:12 +0800 Subject: [PATCH 1/5] fix length calculation of wide unicode chars --- addons/xterm-addon-search/src/SearchAddon.ts | 49 +++++++++++-------- .../test/SearchAddon.api.ts | 8 +++ 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index efc12662a3..68e05d5f36 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { Terminal, IDisposable, ITerminalAddon, ISelectionPosition } from 'xterm'; +import { Terminal, IBufferLine, IDisposable, ITerminalAddon, ISelectionPosition } from 'xterm'; export interface ISearchOptions { regex?: boolean; @@ -21,6 +21,7 @@ export interface ISearchResult { term: string; col: number; row: number; + length: number; } const NON_WORD_CHARACTERS = ' ~!@#$%^&*()+`-=[]{}|\;:"\',./<>?'; @@ -324,34 +325,42 @@ export class SearchAddon implements ITerminalAddon { } const line = terminal.buffer.active.getLine(row); + let { length } = term; if (line) { - for (let i = 0; i < resultIndex; i++) { - const cell = line.getCell(i); - if (!cell) { - break; - } - // Adjust the searchIndex to normalize emoji into single chars - const char = cell.getChars(); - if (char.length > 1) { - resultIndex -= char.length - 1; - } - // Adjust the searchIndex for empty characters following wide unicode - // chars (eg. CJK) - const charWidth = cell.getWidth(); - if (charWidth === 0) { - resultIndex++; - } - } + resultIndex = this._stringLengthToBufferSize(line, resultIndex); + length = this._stringLengthToBufferSize(line, length, resultIndex); } return { term, col: resultIndex, - row + row, + length }; } } + private _stringLengthToBufferSize(line: IBufferLine, length: number, start: number = 0): number { + for (let i = 0; i < length; i++) { + const cell = line.getCell(start + i); + if (!cell) { + break; + } + // Adjust the searchIndex to normalize emoji into single chars + const char = cell.getChars(); + if (char.length > 1) { + length -= char.length - 1; + } + // Adjust the searchIndex for empty characters following wide unicode + // chars (eg. CJK) + const nextCell = line.getCell(start + i + 1); + if (nextCell && nextCell.getWidth() === 0) { + length++; + } + } + return length; + } + /** * Translates a buffer line to a string, including subsequent lines if they are wraps. * Wide characters will count as two columns in the resulting string. This @@ -390,7 +399,7 @@ export class SearchAddon implements ITerminalAddon { terminal.clearSelection(); return false; } - terminal.select(result.col, result.row, result.term.length); + terminal.select(result.col, result.row, result.length); // If it is not in the viewport then we scroll else it just gets selected if (result.row >= (terminal.buffer.active.viewportY + terminal.rows) || result.row < terminal.buffer.active.viewportY) { let scroll = result.row - terminal.buffer.active.viewportY; diff --git a/addons/xterm-addon-search/test/SearchAddon.api.ts b/addons/xterm-addon-search/test/SearchAddon.api.ts index 3707aecd12..4622c0ece4 100644 --- a/addons/xterm-addon-search/test/SearchAddon.api.ts +++ b/addons/xterm-addon-search/test/SearchAddon.api.ts @@ -106,6 +106,14 @@ describe('Search Tests', function(): void { assert.deepEqual(await page.evaluate(`window.term.getSelection()`), 'abc'); }); + it('Search for result bounding with wide unicode chars', async () => { + await writeSync(page, '中文xxx'); + assert.deepEqual(await page.evaluate(`window.search.findNext('中')`), true); + assert.deepEqual(await page.evaluate(`window.term.getSelection()`), '中'); + assert.deepEqual(await page.evaluate(`window.search.findNext('xxx')`), true); + assert.deepEqual(await page.evaluate(`window.term.getSelection()`), 'xxx'); + }); + describe('Regression tests', () => { describe('#2444 wrapped line content not being found', () => { let fixture: string; From ae037bdac08df8a1fcb4cb5488d69e4d3b0e181d Mon Sep 17 00:00:00 2001 From: Gerald Date: Mon, 22 Feb 2021 15:37:46 +0800 Subject: [PATCH 2/5] fix: translate buffer col to string offset --- addons/xterm-addon-search/src/SearchAddon.ts | 55 ++++++++++++++----- .../test/SearchAddon.api.ts | 15 ++++- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index 68e05d5f36..5ca902bd1f 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -21,7 +21,7 @@ export interface ISearchResult { term: string; col: number; row: number; - length: number; + size: number; } const NON_WORD_CHARACTERS = ' ~!@#$%^&*()+`-=[]{}|\;:"\',./<>?'; @@ -283,6 +283,7 @@ export class SearchAddon implements ITerminalAddon { } } + const offset = this._bufferColsToStringOffset(row, col); const searchTerm = searchOptions.caseSensitive ? term : term.toLowerCase(); const searchStringLine = searchOptions.caseSensitive ? stringLine : stringLine.toLowerCase(); @@ -291,26 +292,26 @@ export class SearchAddon implements ITerminalAddon { const searchRegex = RegExp(searchTerm, 'g'); let foundTerm: RegExpExecArray | null; if (isReverseSearch) { - // This loop will get the resultIndex of the _last_ regex match in the range 0..col - while (foundTerm = searchRegex.exec(searchStringLine.slice(0, col))) { + // This loop will get the resultIndex of the _last_ regex match in the range 0..offset + while (foundTerm = searchRegex.exec(searchStringLine.slice(0, offset))) { resultIndex = searchRegex.lastIndex - foundTerm[0].length; term = foundTerm[0]; searchRegex.lastIndex -= (term.length - 1); } } else { - foundTerm = searchRegex.exec(searchStringLine.slice(col)); + foundTerm = searchRegex.exec(searchStringLine.slice(offset)); if (foundTerm && foundTerm[0].length > 0) { - resultIndex = col + (searchRegex.lastIndex - foundTerm[0].length); + resultIndex = offset + (searchRegex.lastIndex - foundTerm[0].length); term = foundTerm[0]; } } } else { if (isReverseSearch) { - if (col - searchTerm.length >= 0) { - resultIndex = searchStringLine.lastIndexOf(searchTerm, col - searchTerm.length); + if (offset - searchTerm.length >= 0) { + resultIndex = searchStringLine.lastIndexOf(searchTerm, offset - searchTerm.length); } } else { - resultIndex = searchStringLine.indexOf(searchTerm, col); + resultIndex = searchStringLine.indexOf(searchTerm, offset); } } @@ -325,17 +326,18 @@ export class SearchAddon implements ITerminalAddon { } const line = terminal.buffer.active.getLine(row); - let { length } = term; + let size = term.length; + let col = 0; if (line) { - resultIndex = this._stringLengthToBufferSize(line, resultIndex); - length = this._stringLengthToBufferSize(line, length, resultIndex); + col = this._stringLengthToBufferSize(line, resultIndex); + size = this._stringLengthToBufferSize(line, term.length, col); } return { term, - col: resultIndex, + col, row, - length + size }; } } @@ -361,6 +363,31 @@ export class SearchAddon implements ITerminalAddon { return length; } + private _bufferColsToStringOffset(startRow: number, cols: number): number { + const terminal = this._terminal!; + let lineIndex = startRow; + let offset = 0; + let line = terminal.buffer.active.getLine(lineIndex); + while (cols > 0 && line) { + for (let i = 0; i < cols && i < terminal.cols; i++) { + const cell = line.getCell(i); + if (!cell) { + break; + } + if (cell.getWidth()) { + offset += cell.getChars().length; + } + } + lineIndex++; + line = terminal.buffer.active.getLine(lineIndex); + if (line && !line.isWrapped) { + break; + } + cols -= terminal.cols; + } + return offset; + } + /** * Translates a buffer line to a string, including subsequent lines if they are wraps. * Wide characters will count as two columns in the resulting string. This @@ -399,7 +426,7 @@ export class SearchAddon implements ITerminalAddon { terminal.clearSelection(); return false; } - terminal.select(result.col, result.row, result.length); + terminal.select(result.col, result.row, result.size); // If it is not in the viewport then we scroll else it just gets selected if (result.row >= (terminal.buffer.active.viewportY + terminal.rows) || result.row < terminal.buffer.active.viewportY) { let scroll = result.row - terminal.buffer.active.viewportY; diff --git a/addons/xterm-addon-search/test/SearchAddon.api.ts b/addons/xterm-addon-search/test/SearchAddon.api.ts index 4622c0ece4..eb9a468098 100644 --- a/addons/xterm-addon-search/test/SearchAddon.api.ts +++ b/addons/xterm-addon-search/test/SearchAddon.api.ts @@ -107,11 +107,20 @@ describe('Search Tests', function(): void { }); it('Search for result bounding with wide unicode chars', async () => { - await writeSync(page, '中文xxx'); + await writeSync(page, '中文xx𝄞𝄞'); assert.deepEqual(await page.evaluate(`window.search.findNext('中')`), true); assert.deepEqual(await page.evaluate(`window.term.getSelection()`), '中'); - assert.deepEqual(await page.evaluate(`window.search.findNext('xxx')`), true); - assert.deepEqual(await page.evaluate(`window.term.getSelection()`), 'xxx'); + assert.deepEqual(await page.evaluate(`window.search.findNext('xx')`), true); + assert.deepEqual(await page.evaluate(`window.term.getSelection()`), 'xx'); + assert.deepEqual(await page.evaluate(`window.search.findNext('𝄞')`), true); + assert.deepEqual(await page.evaluate(`window.term.getSelection()`), '𝄞'); + assert.deepEqual(await page.evaluate(`window.search.findNext('𝄞')`), true); + assert.deepEqual(await page.evaluate(`window.term.getSelectionPosition()`), { + startRow: 0, + endRow: 0, + startColumn: 7, + endColumn: 8 + }); }); describe('Regression tests', () => { From db42dcfae2d63ef0da3d98b05cd65348f1abc5c4 Mon Sep 17 00:00:00 2001 From: Gerald Date: Tue, 23 Feb 2021 12:24:00 +0800 Subject: [PATCH 3/5] fix: translate string offset to buffer index in wrapped line --- addons/xterm-addon-search/src/SearchAddon.ts | 95 ++++++++++++-------- 1 file changed, 56 insertions(+), 39 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index 5ca902bd1f..fec9e81964 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -35,7 +35,7 @@ export class SearchAddon implements ITerminalAddon { * We memoize the calls into an array that has a time based ttl. * _linesCache is also invalidated when the terminal cursor moves. */ - private _linesCache: string[] | undefined; + private _linesCache: [string, number[]][] | undefined; private _linesCacheTimeoutId = 0; private _cursorMoveListener: IDisposable | undefined; private _resizeListener: IDisposable | undefined; @@ -258,7 +258,7 @@ export class SearchAddon implements ITerminalAddon { */ protected _findInLine(term: string, searchPosition: ISearchPosition, searchOptions: ISearchOptions = {}, isReverseSearch: boolean = false): ISearchResult | undefined { const terminal = this._terminal!; - let row = searchPosition.startRow; + const row = searchPosition.startRow; const col = searchPosition.startCol; // Ignore wrapped lines, only consider on unwrapped line (first row of command string). @@ -275,13 +275,14 @@ export class SearchAddon implements ITerminalAddon { searchPosition.startCol += terminal.cols; return this._findInLine(term, searchPosition, searchOptions); } - let stringLine = this._linesCache ? this._linesCache[row] : void 0; - if (stringLine === void 0) { - stringLine = this._translateBufferLineToStringWithWrap(row, true); + let cache = this._linesCache?.[row]; + if (!cache) { + cache = this._translateBufferLineToStringWithWrap(row, true); if (this._linesCache) { - this._linesCache[row] = stringLine; + this._linesCache[row] = cache; } } + const [stringLine, offsets] = cache; const offset = this._bufferColsToStringOffset(row, col); const searchTerm = searchOptions.caseSensitive ? term : term.toLowerCase(); @@ -316,51 +317,57 @@ export class SearchAddon implements ITerminalAddon { } if (resultIndex >= 0) { - // Adjust the row number and search index if needed since a "line" of text can span multiple rows - if (resultIndex >= terminal.cols) { - row += Math.floor(resultIndex / terminal.cols); - resultIndex = resultIndex % terminal.cols; - } if (searchOptions.wholeWord && !this._isWholeWord(resultIndex, searchStringLine, term)) { return; } - const line = terminal.buffer.active.getLine(row); - let size = term.length; - - let col = 0; - if (line) { - col = this._stringLengthToBufferSize(line, resultIndex); - size = this._stringLengthToBufferSize(line, term.length, col); + // Adjust the row number and search index if needed since a "line" of text can span multiple rows + let startRowOffset = 0; + while (startRowOffset < offsets.length - 1 && resultIndex >= offsets[startRowOffset + 1]) { + startRowOffset++; } + let endRowOffset = startRowOffset; + while (endRowOffset < offsets.length - 1 && resultIndex + term.length >= offsets[endRowOffset + 1]) { + endRowOffset++; + } + const startColOffset = resultIndex - offsets[startRowOffset]; + const endColOffset = resultIndex + term.length - offsets[endRowOffset]; + const startColIndex = this._stringLengthToBufferSize(row + startRowOffset, startColOffset); + const endColIndex = this._stringLengthToBufferSize(row + endRowOffset, endColOffset); + const size = endColIndex - startColIndex + terminal.cols * (endRowOffset - startRowOffset); + return { term, - col, - row, + col: startColIndex, + row: row + startRowOffset, size }; } } - private _stringLengthToBufferSize(line: IBufferLine, length: number, start: number = 0): number { - for (let i = 0; i < length; i++) { - const cell = line.getCell(start + i); + private _stringLengthToBufferSize(row: number, offset: number): number { + const line = this._terminal!.buffer.active.getLine(row); + if (!line) { + return 0; + } + for (let i = 0; i < offset; i++) { + const cell = line.getCell(i); if (!cell) { break; } // Adjust the searchIndex to normalize emoji into single chars const char = cell.getChars(); if (char.length > 1) { - length -= char.length - 1; + offset -= char.length - 1; } // Adjust the searchIndex for empty characters following wide unicode // chars (eg. CJK) - const nextCell = line.getCell(start + i + 1); + const nextCell = line.getCell(i + 1); if (nextCell && nextCell.getWidth() === 0) { - length++; + offset++; } } - return length; + return offset; } private _bufferColsToStringOffset(startRow: number, cols: number): number { @@ -396,23 +403,33 @@ export class SearchAddon implements ITerminalAddon { * @param line The line being translated. * @param trimRight Whether to trim whitespace to the right. */ - private _translateBufferLineToStringWithWrap(lineIndex: number, trimRight: boolean): string { + private _translateBufferLineToStringWithWrap(lineIndex: number, trimRight: boolean): [string, number[]] { const terminal = this._terminal!; - let lineString = ''; - let lineWrapsToNext: boolean; - - do { + const strings = []; + const offsets = [0]; + let line = terminal.buffer.active.getLine(lineIndex); + while (line) { const nextLine = terminal.buffer.active.getLine(lineIndex + 1); - lineWrapsToNext = nextLine ? nextLine.isWrapped : false; - const line = terminal.buffer.active.getLine(lineIndex); - if (!line) { + const lineWrapsToNext = nextLine ? nextLine.isWrapped : false; + let string = line.translateToString(!lineWrapsToNext && trimRight); + if (lineWrapsToNext && nextLine) { + const lastCell = line.getCell(line.length - 1); + const lastCellIsNull = lastCell && lastCell.getCode() === 0 && lastCell.getWidth() === 1; + // a wide character wrapped to the next line + if (lastCellIsNull && nextLine.getCell(0)?.getWidth() === 2) { + string = string.slice(0, -1); + } + } + strings.push(string); + if (lineWrapsToNext) { + offsets.push(offsets[offsets.length - 1] + string.length); + } else { break; } - lineString += line.translateToString(!lineWrapsToNext && trimRight).substring(0, terminal.cols); lineIndex++; - } while (lineWrapsToNext); - - return lineString; + line = nextLine; + } + return [strings.join(''), offsets]; } /** From 0fcd87085ba475b31f783474ebe89666bea87559 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 22 Dec 2021 09:26:19 -0800 Subject: [PATCH 4/5] Name line cache entry type --- addons/xterm-addon-search/src/SearchAddon.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index c6494c5856..155d50702d 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -24,6 +24,8 @@ export interface ISearchResult { size: number; } +type LineCacheEntry = [lineAsString: string, offsets: number[]]; + const NON_WORD_CHARACTERS = ' ~!@#$%^&*()+`-=[]{}|\\;:"\',./<>?'; const LINES_CACHE_TIME_TO_LIVE = 15 * 1000; // 15 secs @@ -35,7 +37,7 @@ export class SearchAddon implements ITerminalAddon { * We memoize the calls into an array that has a time based ttl. * _linesCache is also invalidated when the terminal cursor moves. */ - private _linesCache: [string, number[]][] | undefined; + private _linesCache: LineCacheEntry[] | undefined; private _linesCacheTimeoutId = 0; private _cursorMoveListener: IDisposable | undefined; private _resizeListener: IDisposable | undefined; @@ -403,7 +405,7 @@ export class SearchAddon implements ITerminalAddon { * @param line The line being translated. * @param trimRight Whether to trim whitespace to the right. */ - private _translateBufferLineToStringWithWrap(lineIndex: number, trimRight: boolean): [string, number[]] { + private _translateBufferLineToStringWithWrap(lineIndex: number, trimRight: boolean): LineCacheEntry { const terminal = this._terminal!; const strings = []; const offsets = [0]; From f6853607076baa1408f38745affcbc46b2adf35a Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 22 Dec 2021 09:29:32 -0800 Subject: [PATCH 5/5] Add docs to line cache entry items --- addons/xterm-addon-search/src/SearchAddon.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index 155d50702d..4651f6c7f4 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -24,7 +24,16 @@ export interface ISearchResult { size: number; } -type LineCacheEntry = [lineAsString: string, offsets: number[]]; +type LineCacheEntry = [ + /** + * The string representation of a line (as opposed to the buffer cell representation). + */ + lineAsString: string, + /** + * The offsets where each line starts when the entry describes a wrapped line. + */ + lineOffsets: number[] +]; const NON_WORD_CHARACTERS = ' ~!@#$%^&*()+`-=[]{}|\\;:"\',./<>?'; const LINES_CACHE_TIME_TO_LIVE = 15 * 1000; // 15 secs @@ -408,7 +417,7 @@ export class SearchAddon implements ITerminalAddon { private _translateBufferLineToStringWithWrap(lineIndex: number, trimRight: boolean): LineCacheEntry { const terminal = this._terminal!; const strings = []; - const offsets = [0]; + const lineOffsets = [0]; let line = terminal.buffer.active.getLine(lineIndex); while (line) { const nextLine = terminal.buffer.active.getLine(lineIndex + 1); @@ -424,14 +433,14 @@ export class SearchAddon implements ITerminalAddon { } strings.push(string); if (lineWrapsToNext) { - offsets.push(offsets[offsets.length - 1] + string.length); + lineOffsets.push(lineOffsets[lineOffsets.length - 1] + string.length); } else { break; } lineIndex++; line = nextLine; } - return [strings.join(''), offsets]; + return [strings.join(''), lineOffsets]; } /**