From e652db05e1ba47725249a6ff543628aeeb32fad7 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sat, 23 Nov 2024 16:47:46 +0000 Subject: [PATCH] Desktop: Fixes #11409: Goto Anything fails for long strings (#11436) --- packages/app-desktop/plugins/GotoAnything.tsx | 18 +++++++++------ .../screens/SearchScreen/SearchResults.tsx | 4 ++-- .../components/screens/SearchScreen/index.tsx | 3 ++- packages/lib/BaseApplication.ts | 4 ++-- packages/lib/services/search/SearchEngine.ts | 22 +++++++++++++------ packages/lib/string-utils.ts | 4 ++++ 6 files changed, 36 insertions(+), 19 deletions(-) diff --git a/packages/app-desktop/plugins/GotoAnything.tsx b/packages/app-desktop/plugins/GotoAnything.tsx index b57e3b09e..aed524e3e 100644 --- a/packages/app-desktop/plugins/GotoAnything.tsx +++ b/packages/app-desktop/plugins/GotoAnything.tsx @@ -13,7 +13,7 @@ import Folder from '@joplin/lib/models/Folder'; import Note from '@joplin/lib/models/Note'; import ItemList from '../gui/ItemList'; import HelpButton from '../gui/HelpButton'; -import { surroundKeywords, nextWhitespaceIndex, removeDiacritics } from '@joplin/lib/string-utils'; +import { surroundKeywords, nextWhitespaceIndex, removeDiacritics, escapeRegExp, KeywordType } from '@joplin/lib/string-utils'; import { mergeOverlappingIntervals } from '@joplin/lib/ArrayUtils'; import markupLanguageUtils from '@joplin/lib/utils/markupLanguageUtils'; import focusEditorIfEditorCommand from '@joplin/lib/services/commands/focusEditorIfEditorCommand'; @@ -56,7 +56,7 @@ interface State { query: string; results: GotoAnythingSearchResult[]; selectedItemId: string; - keywords: string[]; + keywords: (string | ComplexTerm)[]; listType: number; showHelp: boolean; resultsInBody: boolean; @@ -376,9 +376,13 @@ class DialogComponent extends React.PureComponent { results = results.filter(r => !!notesById[r.id]) .map(r => ({ ...r, title: notesById[r.id].title })); - const normalizedKeywords = (await this.keywords(searchQuery)).map( - ({ valueRegex }: ComplexTerm) => new RegExp(removeDiacritics(valueRegex), 'ig'), - ); + const keywordRegexes = (await this.keywords(searchQuery)).map(term => { + if (typeof term === 'string') { + return new RegExp(escapeRegExp(term), 'ig'); + } else { + return new RegExp(removeDiacritics(term.valueRegex), 'ig'); + } + }); for (let i = 0; i < results.length; i++) { const row = results[i]; @@ -393,7 +397,7 @@ class DialogComponent extends React.PureComponent { const normalizedBody = removeDiacritics(body); // Iterate over all matches in the body for each search keyword - for (const keywordRegex of normalizedKeywords) { + for (const keywordRegex of keywordRegexes) { for (const match of normalizedBody.matchAll(keywordRegex)) { // Populate 'indices' with [begin index, end index] of each note fragment // Begins at the regex matching index, ends at the next whitespace after seeking 15 characters to the right @@ -547,7 +551,7 @@ class DialogComponent extends React.PureComponent { const wrapKeywordMatches = (unescapedContent: string) => { return surroundKeywords( - this.state.keywords, + this.state.keywords as KeywordType, unescapedContent, ``, '', diff --git a/packages/app-mobile/components/screens/SearchScreen/SearchResults.tsx b/packages/app-mobile/components/screens/SearchScreen/SearchResults.tsx index c23ece678..ddb65406b 100644 --- a/packages/app-mobile/components/screens/SearchScreen/SearchResults.tsx +++ b/packages/app-mobile/components/screens/SearchScreen/SearchResults.tsx @@ -7,13 +7,13 @@ import useQueuedAsyncEffect from '@joplin/lib/hooks/useQueuedAsyncEffect'; import { NoteEntity } from '@joplin/lib/services/database/types'; import SearchEngineUtils from '@joplin/lib/services/search/SearchEngineUtils'; import Note from '@joplin/lib/models/Note'; -import SearchEngine from '@joplin/lib/services/search/SearchEngine'; +import SearchEngine, { ComplexTerm } from '@joplin/lib/services/search/SearchEngine'; import { ProgressBar } from 'react-native-paper'; import shim from '@joplin/lib/shim'; interface Props { query: string; - onHighlightedWordsChange: (highlightedWords: string[])=> void; + onHighlightedWordsChange: (highlightedWords: (ComplexTerm | string)[])=> void; ftsEnabled: number; } diff --git a/packages/app-mobile/components/screens/SearchScreen/index.tsx b/packages/app-mobile/components/screens/SearchScreen/index.tsx index 4e365585d..137ccb3c0 100644 --- a/packages/app-mobile/components/screens/SearchScreen/index.tsx +++ b/packages/app-mobile/components/screens/SearchScreen/index.tsx @@ -11,6 +11,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import IconButton from '../../IconButton'; import SearchResults from './SearchResults'; import AccessibleView from '../../accessibility/AccessibleView'; +import { ComplexTerm } from '@joplin/lib/services/search/SearchEngine'; interface Props { themeId: number; @@ -73,7 +74,7 @@ const SearchScreenComponent: React.FC = props => { setQuery(''); }, []); - const onHighlightedWordsChange = useCallback((words: string[]) => { + const onHighlightedWordsChange = useCallback((words: (ComplexTerm | string)[]) => { props.dispatch({ type: 'SET_HIGHLIGHTED', words, diff --git a/packages/lib/BaseApplication.ts b/packages/lib/BaseApplication.ts index 2d9f07a80..074c7f3b6 100644 --- a/packages/lib/BaseApplication.ts +++ b/packages/lib/BaseApplication.ts @@ -39,7 +39,7 @@ const SyncTargetAmazonS3 = require('./SyncTargetAmazonS3.js'); import EncryptionService from './services/e2ee/EncryptionService'; import ResourceFetcher from './services/ResourceFetcher'; import SearchEngineUtils from './services/search/SearchEngineUtils'; -import SearchEngine, { ProcessResultsRow } from './services/search/SearchEngine'; +import SearchEngine, { ComplexTerm, ProcessResultsRow } from './services/search/SearchEngine'; import RevisionService from './services/RevisionService'; import ResourceService from './services/ResourceService'; import DecryptionWorker from './services/DecryptionWorker'; @@ -240,7 +240,7 @@ export default class BaseApplication { }); let notes: NoteEntity[] = []; - let highlightedWords: string[] = []; + let highlightedWords: (ComplexTerm | string)[] = []; let searchResults: ProcessResultsRow[] = []; if (parentId) { diff --git a/packages/lib/services/search/SearchEngine.ts b/packages/lib/services/search/SearchEngine.ts index 8f762dbd5..e0891f5f8 100644 --- a/packages/lib/services/search/SearchEngine.ts +++ b/packages/lib/services/search/SearchEngine.ts @@ -63,11 +63,22 @@ export interface ComplexTerm { } export interface Terms { + // This `string | ComplexTerm` type that ends up propagating throughout the app is a bit of a + // mess, but it reflects how it was originally done in plain JS. Ideally it should be refactor + // to use a simple type. _: (string | ComplexTerm)[]; title: (string | ComplexTerm)[]; body: (string | ComplexTerm)[]; } +export interface ParsedQuery { + termCount: number; + keys: string[]; + terms: Terms; // text terms + allTerms: Term[]; + any: boolean; +} + export default class SearchEngine { public static instance_: SearchEngine = null; @@ -521,7 +532,7 @@ export default class SearchEngine { return regexString; } - public async parseQuery(query: string) { + public async parseQuery(query: string): Promise { const trimQuotes = (str: string) => str.startsWith('"') ? str.substr(1, str.length - 2) : str; @@ -606,14 +617,11 @@ export default class SearchEngine { }; } - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - public allParsedQueryTerms(parsedQuery: any) { + public allParsedQueryTerms(parsedQuery: ParsedQuery) { if (!parsedQuery || !parsedQuery.termCount) return []; - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - let output: any[] = []; - for (const col in parsedQuery.terms) { - if (!parsedQuery.terms.hasOwnProperty(col)) continue; + let output: typeof parsedQuery.terms._ = []; + for (const col of Object.keys(parsedQuery.terms) as (keyof Terms)[]) { output = output.concat(parsedQuery.terms[col]); } return output; diff --git a/packages/lib/string-utils.ts b/packages/lib/string-utils.ts index 63d4bbbe5..7028f255d 100644 --- a/packages/lib/string-utils.ts +++ b/packages/lib/string-utils.ts @@ -100,6 +100,10 @@ export function removeDiacritics(str: string) { return str; } +export function escapeRegExp(keyword: string) { + return keyword.replace(/[.*+\-?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string +} + export function wrap(text: string, indent: string, width: number) { const wrap_ = require('word-wrap');