From 5ccc0fd97bc5056c460acf167044d2c3eb0c7c64 Mon Sep 17 00:00:00 2001 From: Maddy <12754347+MaddyDev@users.noreply.github.com> Date: Thu, 23 Jan 2020 07:28:12 -0800 Subject: [PATCH] Implemented match case and whole wrd (#8904) * Implemented match case and whole wrd * removed unused ref * renamed test method * escape \ * refactored search * added more tests * updated tests appying match case and whole word to spcl characters * regex update * spcl to special * non-capturing group added to the regex * test --- .../browser/models/modelInterfaces.ts | 2 +- .../notebook/browser/notebookEditor.ts | 6 +- .../notebook/find/notebookFindModel.ts | 72 +++++++----- .../notebookFindModel.test.ts | 103 ++++++++++++++++-- .../workbench/contrib/notebook/test/stubs.ts | 2 +- 5 files changed, 146 insertions(+), 39 deletions(-) diff --git a/src/sql/workbench/contrib/notebook/browser/models/modelInterfaces.ts b/src/sql/workbench/contrib/notebook/browser/models/modelInterfaces.ts index eb47d4cbb5..57ffc64946 100644 --- a/src/sql/workbench/contrib/notebook/browser/models/modelInterfaces.ts +++ b/src/sql/workbench/contrib/notebook/browser/models/modelInterfaces.ts @@ -430,7 +430,7 @@ export interface INotebookFindModel { findPrevious(): Promise; /** search the notebook model for the given exp up to maxMatch occurances */ - find(exp: string, maxMatches?: number): Promise; + find(exp: string, matchCase?: boolean, wholeWord?: boolean, maxMatches?: number): Promise; /** clear the results of the find */ clearFind(): void; diff --git a/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts b/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts index eec8b5e7b8..9cda0b57d9 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebookEditor.ts @@ -282,12 +282,12 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle } } - if (e.searchString) { + if (e.searchString || e.matchCase || e.wholeWord) { this._findDecorations.clearDecorations(); if (this._notebookModel) { if (this._findState.searchString) { let findScope = this._findDecorations.getFindScope(); - if (this._findState.searchString === this.notebookFindModel.findExpression && findScope !== null) { + if (this._findState.searchString === this.notebookFindModel.findExpression && findScope !== null && !e.matchCase && !e.wholeWord) { if (findScope) { this._updateFinderMatchState(); this._findState.changeMatchInfo( @@ -300,7 +300,7 @@ export class NotebookEditor extends BaseEditor implements IFindNotebookControlle } else { this.notebookInput.notebookFindModel.clearDecorations(); this.notebookFindModel.findExpression = this._findState.searchString; - this.notebookInput.notebookFindModel.find(this._findState.searchString, NOTEBOOK_MAX_MATCHES).then(findRange => { + this.notebookInput.notebookFindModel.find(this._findState.searchString, this._findState.matchCase, this._findState.wholeWord, NOTEBOOK_MAX_MATCHES).then(findRange => { if (findRange) { this.updatePosition(findRange); } else if (this.notebookFindModel.findMatches.length > 0) { diff --git a/src/sql/workbench/contrib/notebook/find/notebookFindModel.ts b/src/sql/workbench/contrib/notebook/find/notebookFindModel.ts index ec17c31606..511c537b38 100644 --- a/src/sql/workbench/contrib/notebook/find/notebookFindModel.ts +++ b/src/sql/workbench/contrib/notebook/find/notebookFindModel.ts @@ -490,13 +490,13 @@ export class NotebookFindModel extends Disposable implements INotebookFindModel } } - find(exp: string, maxMatches?: number): Promise { + find(exp: string, matchCase?: boolean, wholeWord?: boolean, maxMatches?: number): Promise { this._findArray = new Array(); this._onFindCountChange.fire(this._findArray.length); if (exp) { for (let i = 0; i < this.notebookModel.cells.length; i++) { const item = this.notebookModel.cells[i]; - const result = this.searchFn(item, exp, maxMatches); + const result = this.searchFn(item, exp, matchCase, wholeWord, maxMatches); if (result) { this._findArray.push(...result); this._onFindCountChange.fire(this._findArray.length); @@ -523,43 +523,65 @@ export class NotebookFindModel extends Disposable implements INotebookFindModel return this.findArray; } - private searchFn(cell: ICellModel, exp: string, maxMatches?: number): NotebookRange[] { + private searchFn(cell: ICellModel, exp: string, matchCase: boolean = false, wholeWord: boolean = false, maxMatches?: number): NotebookRange[] { let findResults: NotebookRange[] = []; let cellVal = cell.cellType === 'markdown' ? this.cleanUpCellSource(cell.source) : cell.source; - let index: number; - let start: number; - let end: number; if (cellVal) { + if (typeof cellVal === 'string') { - index = 0; - while (cellVal.substr(index).toLocaleLowerCase().indexOf(exp.toLocaleLowerCase()) > -1) { - start = cellVal.substr(index).toLocaleLowerCase().indexOf(exp.toLocaleLowerCase()) + index; - end = start + exp.length; - let range = new NotebookRange(cell, 0, start, 0, end); - findResults = findResults.concat(range); - index = end; - } + let findStartResults = this.search(cellVal, exp, matchCase, wholeWord, maxMatches); + findStartResults.forEach(start => { + let range = new NotebookRange(cell, 0, start, 0, start + exp.length); + findResults.push(range); + }); + } else { for (let j = 0; j < cellVal.length; j++) { - index = 0; let cellValFormatted = cell.cellType === 'markdown' ? this.cleanMarkdownLinks(cellVal[j]) : cellVal[j]; - while (cellValFormatted.substr(index).toLocaleLowerCase().indexOf(exp.toLocaleLowerCase()) > -1) { - start = cellValFormatted.substr(index).toLocaleLowerCase().indexOf(exp.toLocaleLowerCase()) + index + 1; - end = start + exp.length; + let findStartResults = this.search(cellValFormatted, exp, matchCase, wholeWord, maxMatches - findResults.length); + findStartResults.forEach(start => { // lineNumber: j+1 since notebook editors aren't zero indexed. - let range = new NotebookRange(cell, j + 1, start, j + 1, end); - findResults = findResults.concat(range); - index = end; - } - if (findResults.length >= maxMatches) { - break; - } + let range = new NotebookRange(cell, j + 1, start, j + 1, start + exp.length); + findResults.push(range); + }); } } } return findResults; } + // escape the special characters in a regex string + escapeRegExp(text: string): string { + return text.replace(/[-[\]{}()*+!<=:?.\/\\^$|#\s,]/g, '\\$&'); + } + + search(input: string, exp: string, matchCase: boolean = false, wholeWord: boolean = false, maxMatches?: number): number[] { + let index: number = 0; + let start: number; + let findResults: number[] = []; + if (!matchCase) { + input = input.toLocaleLowerCase(); + exp = exp.toLocaleLowerCase(); + } + let searchText: string = input.substr(index); + while (findResults.length < maxMatches && searchText.indexOf(exp) > -1) { + if (wholeWord) { + // word with no special characters around \\bword\\b, word that begins or ends with special character \\sword\\s + let wholeWordRegex = new RegExp(`(?:\\b|\\s)${this.escapeRegExp(exp)}(?:\\b|\\s)`); + start = searchText.search(wholeWordRegex) + 1; + if (start < 1) { + break; + } + } else { + start = searchText.indexOf(exp) + index + 1; + } + findResults.push(start); + index = start + exp.length; + searchText = input.substr(index - 1); + } + return findResults; + } + // In markdown links are defined as [Link Text](https://url/of/the/text). when searching for text we shouldn't // look for the values inside the (), below regex replaces that with just the Link Text. cleanMarkdownLinks(cellSrc: string): string { diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts index 671ecf8b10..55692f3615 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts @@ -32,12 +32,12 @@ import { NotebookEditorContentManager } from 'sql/workbench/contrib/notebook/bro let expectedNotebookContent: nb.INotebookContents = { cells: [{ cell_type: CellTypes.Code, - source: 'insert into t1 values (c1, c2) \ninsert into markdown values (*hello worls*)', + source: ['insert into t1 values (c1, c2) ', 'INSERT into markdown values (*hello world*)'], metadata: { language: 'python' }, execution_count: 1 }, { cell_type: CellTypes.Markdown, - source: 'I am *markdown*', + source: ['I am *markdown insertImportant*'], metadata: { language: 'python' }, execution_count: 1 }], @@ -110,23 +110,23 @@ suite('Notebook Find Model', function (): void { test('Should find results in the notebook', async function (): Promise { //initialize find let notebookFindModel = new NotebookFindModel(model); - await notebookFindModel.find('markdown', max_find_count); + await notebookFindModel.find('markdown', false, false, max_find_count); - assert(notebookFindModel.findMatches, new Error('Find in notebook failed.')); + assert(notebookFindModel.findMatches, 'Find in notebook failed.'); assert.equal(notebookFindModel.findMatches.length, 2, 'Find couldnt find all occurances'); }); test('Should not find results in the notebook', async function (): Promise { //initialize find let notebookFindModel = new NotebookFindModel(model); - await notebookFindModel.find('notFound', max_find_count); + await notebookFindModel.find('notFound', false, false, max_find_count); assert.equal(notebookFindModel.findMatches.length, 0, 'Find failed'); }); test('Should match find result ranges', async function (): Promise { let notebookFindModel = new NotebookFindModel(model); - await notebookFindModel.find('markdown', max_find_count); + await notebookFindModel.find('markdown', false, false, max_find_count); let expectedFindRange1 = new NotebookRange(model.cells[0], 2, 13, 2, 21); assert.deepEqual(notebookFindModel.findMatches[0].range, expectedFindRange1, 'Find in markdown range is wrong :\n' + JSON.stringify(expectedFindRange1) + '\n ' + JSON.stringify(notebookFindModel.findMatches[0].range)); @@ -155,7 +155,7 @@ suite('Notebook Find Model', function (): void { await initNotebookModel(markdownContent); let notebookFindModel = new NotebookFindModel(model); - await notebookFindModel.find('best', max_find_count); + await notebookFindModel.find('best', false, false, max_find_count); assert.equal(notebookFindModel.findMatches.length, 1, 'Find failed on markdown link'); @@ -184,11 +184,97 @@ suite('Notebook Find Model', function (): void { await initNotebookModel(codeContent); //initialize find let notebookFindModel = new NotebookFindModel(model); - await notebookFindModel.find('x', max_find_count); + await notebookFindModel.find('x', false, false, max_find_count); assert.equal(notebookFindModel.findMatches.length, 3, 'Find failed'); }); + test('Should find results correctly with & without matching case selection', async function (): Promise { + //initialize find + let notebookFindModel = new NotebookFindModel(model); + await notebookFindModel.find('insert', false, false, max_find_count); + + assert(notebookFindModel.findMatches, 'Find in notebook failed.'); + assert.equal(notebookFindModel.findMatches.length, 3, 'Find couldnt find all occurances'); + + await notebookFindModel.find('insert', true, false, max_find_count); + assert.equal(notebookFindModel.findMatches.length, 2, 'Find failed to apply match case while searching'); + + }); + + test('Should find results with matching whole word in the notebook', async function (): Promise { + //initialize find + let notebookFindModel = new NotebookFindModel(model); + + await notebookFindModel.find('insert', true, true, max_find_count); + assert.equal(notebookFindModel.findMatches.length, 1, 'Find failed to apply whole word filter while searching'); + + }); + + test('Should find special characters in the search term without problems', async function (): Promise { + let codeContent: nb.INotebookContents = { + cells: [{ + cell_type: CellTypes.Code, + source: ['import x', 'x.init()', '//am just adding a bunch of {special} !!!!$}'], + metadata: { language: 'python' }, + execution_count: 1 + }], + metadata: { + kernelspec: { + name: 'python', + language: 'python' + } + }, + nbformat: 4, + nbformat_minor: 5 + }; + await initNotebookModel(codeContent); + //initialize find + let notebookFindModel = new NotebookFindModel(model); + // test for string with special character + await notebookFindModel.find('{special}', true, true, max_find_count); + assert.equal(notebookFindModel.findMatches.length, 1, 'Find failed for search term with special character'); + // test for only special character !! + await notebookFindModel.find('!!', false, false, max_find_count); + assert.equal(notebookFindModel.findMatches.length, 2, 'Find failed for special character'); + + // test for only special character combination + await notebookFindModel.find('!!!$}', false, false, max_find_count); + assert.equal(notebookFindModel.findMatches.length, 1, 'Find failed for special character combination'); + }); + + test('Should find // characters in the search term correctly', async function (): Promise { + let codeContent: nb.INotebookContents = { + cells: [{ + cell_type: CellTypes.Code, + source: ['import x', 'x.init()', '//am just adding a bunch of {special} !test$}'], + metadata: { language: 'python' }, + execution_count: 1 + }], + metadata: { + kernelspec: { + name: 'python', + language: 'python' + } + }, + nbformat: 4, + nbformat_minor: 5 + }; + await initNotebookModel(codeContent); + //initialize find + let notebookFindModel = new NotebookFindModel(model); + + await notebookFindModel.find('/', true, false, max_find_count); + assert.equal(notebookFindModel.findMatches.length, 2, 'Find failed to find number of / occurrences'); + + await notebookFindModel.find('//', true, false, max_find_count); + assert.equal(notebookFindModel.findMatches.length, 1, 'Find failed to find number of // occurrences'); + + await notebookFindModel.find('//', true, true, max_find_count); + assert.equal(notebookFindModel.findMatches.length, 0, 'Find failed to apply match whole word for //'); + }); + + async function initNotebookModel(contents: nb.INotebookContents): Promise { let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(contents)); @@ -200,4 +286,3 @@ suite('Notebook Find Model', function (): void { } }); - diff --git a/src/sql/workbench/contrib/notebook/test/stubs.ts b/src/sql/workbench/contrib/notebook/test/stubs.ts index 1e2fd09d58..0479c5ddb1 100644 --- a/src/sql/workbench/contrib/notebook/test/stubs.ts +++ b/src/sql/workbench/contrib/notebook/test/stubs.ts @@ -135,7 +135,7 @@ export class NotebookFindModelStub implements INotebookFindModel { findPrevious(): Promise { throw new Error('Method not implemented.'); } - find(exp: string, maxMatches?: number): Promise { + find(exp: string, matchCase?: boolean, wholeWord?: boolean, maxMatches?: number): Promise { throw new Error('Method not implemented.'); } clearFind(): void {