From 181cd2012cf7d280a382c8ad164b850ce3d1035c Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 13 Jul 2017 12:32:11 +0200 Subject: [PATCH] improve whitespace detection when searching for word at position, #29102 --- src/vs/editor/common/model/wordHelper.ts | 23 +++++++++++---- .../api/extHostDocumentData.test.ts | 28 +++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/vs/editor/common/model/wordHelper.ts b/src/vs/editor/common/model/wordHelper.ts index 8ced8f8c91f..05305e0c187 100644 --- a/src/vs/editor/common/model/wordHelper.ts +++ b/src/vs/editor/common/model/wordHelper.ts @@ -57,10 +57,6 @@ export function ensureValidWordDefinition(wordDefinition?: RegExp): RegExp { function getWordAtPosFast(column: number, wordDefinition: RegExp, text: string, textOffset: number): IWordAtPosition { // find whitespace enclosed text around column and match from there - if (wordDefinition.test(' ')) { - return getWordAtPosSlow(column, wordDefinition, text, textOffset); - } - let pos = column - 1 - textOffset; let start = text.lastIndexOf(' ', pos - 1) + 1; let end = text.indexOf(' ', pos); @@ -113,10 +109,25 @@ function getWordAtPosSlow(column: number, wordDefinition: RegExp, text: string, } export function getWordAtText(column: number, wordDefinition: RegExp, text: string, textOffset: number): IWordAtPosition { - const result = getWordAtPosFast(column, wordDefinition, text, textOffset); + + // if `words` can contain whitespace character we have to use the slow variant + // otherwise we use the fast variant of finding a word + wordDefinition.lastIndex = 0; + let match = wordDefinition.exec(text); + if (!match) { + return null; + } + // todo@joh the `match` could already be the (first) word + const ret = match[0].indexOf(' ') >= 0 + // did match a word which contains a space character -> use slow word find + ? getWordAtPosSlow(column, wordDefinition, text, textOffset) + // sane word definition -> use fast word find + : getWordAtPosFast(column, wordDefinition, text, textOffset); + // both (getWordAtPosFast and getWordAtPosSlow) leave the wordDefinition-RegExp // in an undefined state and to not confuse other users of the wordDefinition // we reset the lastIndex wordDefinition.lastIndex = 0; - return result; + + return ret; } diff --git a/src/vs/workbench/test/electron-browser/api/extHostDocumentData.test.ts b/src/vs/workbench/test/electron-browser/api/extHostDocumentData.test.ts index 12e8380dfc3..a4c48b2f95a 100644 --- a/src/vs/workbench/test/electron-browser/api/extHostDocumentData.test.ts +++ b/src/vs/workbench/test/electron-browser/api/extHostDocumentData.test.ts @@ -269,6 +269,34 @@ suite('ExtHostDocumentData', () => { range = data.document.getWordRangeAtPosition(new Position(0, 11), /yy/); assert.equal(range, undefined); }); + + test('getWordRangeAtPosition doesn\'t quite use the regex as expected, #29102', function () { + data = new ExtHostDocumentData(undefined, URI.file(''), [ + 'some text here', + '/** foo bar */', + 'function() {', + ' "far boo"', + '}' + ], '\n', 'text', 1, false); + + let range = data.document.getWordRangeAtPosition(new Position(0, 0), /\/\*.+\*\//); + assert.equal(range, undefined); + + range = data.document.getWordRangeAtPosition(new Position(1, 0), /\/\*.+\*\//); + assert.equal(range.start.line, 1); + assert.equal(range.start.character, 0); + assert.equal(range.end.line, 1); + assert.equal(range.end.character, 14); + + range = data.document.getWordRangeAtPosition(new Position(3, 0), /("|').*\1/); + assert.equal(range, undefined); + + range = data.document.getWordRangeAtPosition(new Position(3, 1), /("|').*\1/); + assert.equal(range.start.line, 3); + assert.equal(range.start.character, 1); + assert.equal(range.end.line, 3); + assert.equal(range.end.character, 10); + }); }); enum AssertDocumentLineMappingDirection { -- GitLab