diff --git a/data/fixtures/recorded/surroundingPair/textual/changePair2.yml b/data/fixtures/recorded/surroundingPair/textual/changePair2.yml new file mode 100644 index 0000000000..efe2c1e421 --- /dev/null +++ b/data/fixtures/recorded/surroundingPair/textual/changePair2.yml @@ -0,0 +1,23 @@ +languageId: typescript +command: + version: 7 + spokenForm: change pair + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: containingScope + scopeType: {type: surroundingPair, delimiter: any} + usePrePhraseSnapshot: true +initialState: + documentContents: "[1, ']', 2]" + selections: + - anchor: {line: 0, character: 1} + active: {line: 0, character: 1} + marks: {} +finalState: + documentContents: "" + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} diff --git a/data/fixtures/recorded/surroundingPair/textual/changePair3.yml b/data/fixtures/recorded/surroundingPair/textual/changePair3.yml new file mode 100644 index 0000000000..6615e2530c --- /dev/null +++ b/data/fixtures/recorded/surroundingPair/textual/changePair3.yml @@ -0,0 +1,23 @@ +languageId: typescript +command: + version: 7 + spokenForm: change pair + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: containingScope + scopeType: {type: surroundingPair, delimiter: any} + usePrePhraseSnapshot: true +initialState: + documentContents: "`[1, ${']'}, 3]`" + selections: + - anchor: {line: 0, character: 1} + active: {line: 0, character: 1} + marks: {} +finalState: + documentContents: "``" + selections: + - anchor: {line: 0, character: 1} + active: {line: 0, character: 1} diff --git a/packages/common/src/index.ts b/packages/common/src/index.ts index 3c54d20a40..3721c82a07 100644 --- a/packages/common/src/index.ts +++ b/packages/common/src/index.ts @@ -76,6 +76,7 @@ export * from "./types/Selection"; export * from "./types/snippet.types"; export * from "./types/SpokenForm"; export * from "./types/SpokenFormType"; +export * from "./types/StringRecord"; export * from "./types/TalonSpokenForms"; export * from "./types/TestCaseFixture"; export * from "./types/TestHelpers"; diff --git a/packages/common/src/util/regex.ts b/packages/common/src/util/regex.ts index 8e61cb26a1..449bd544c9 100644 --- a/packages/common/src/util/regex.ts +++ b/packages/common/src/util/regex.ts @@ -30,16 +30,20 @@ function makeCache(func: (arg: T) => U) { export const rightAnchored = makeCache(_rightAnchored); export const leftAnchored = makeCache(_leftAnchored); +export function matchAllIterator(text: string, regex: RegExp) { + // Reset the regex to start at the beginning of string, in case the regex has + // been used before. + // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec#finding_successive_matches + regex.lastIndex = 0; + return text.matchAll(regex); +} + export function matchAll( text: string, regex: RegExp, mapfn: (v: RegExpMatchArray, k: number) => T, ) { - // Reset the regex to start at the beginning of string, in case the regex has - // been used before. - // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec#finding_successive_matches - regex.lastIndex = 0; - return Array.from(text.matchAll(regex), mapfn); + return Array.from(matchAllIterator(text, regex), mapfn); } export function testRegex(regex: RegExp, text: string): boolean { diff --git a/packages/cursorless-engine/src/actions/ExecuteCommand.ts b/packages/cursorless-engine/src/actions/ExecuteCommand.ts index e6c8a41e36..403cf160d1 100644 --- a/packages/cursorless-engine/src/actions/ExecuteCommand.ts +++ b/packages/cursorless-engine/src/actions/ExecuteCommand.ts @@ -14,6 +14,7 @@ import type { ActionReturnValue } from "./actions.types"; */ export default class ExecuteCommand { private callbackAction: CallbackAction; + constructor(rangeUpdater: RangeUpdater) { this.callbackAction = new CallbackAction(rangeUpdater); this.run = this.run.bind(this); diff --git a/packages/cursorless-engine/src/disabledComponents/DisabledLanguageDefinitions.ts b/packages/cursorless-engine/src/disabledComponents/DisabledLanguageDefinitions.ts index 20a3936c88..53f8f85d36 100644 --- a/packages/cursorless-engine/src/disabledComponents/DisabledLanguageDefinitions.ts +++ b/packages/cursorless-engine/src/disabledComponents/DisabledLanguageDefinitions.ts @@ -16,6 +16,10 @@ export class DisabledLanguageDefinitions implements LanguageDefinitions { return undefined; } + clearCache(): void { + // Do nothing + } + getNodeAtLocation( _document: TextDocument, _range: Range, diff --git a/packages/cursorless-engine/src/languages/LanguageDefinition.ts b/packages/cursorless-engine/src/languages/LanguageDefinition.ts index 25f8ac29fc..3cd43d1653 100644 --- a/packages/cursorless-engine/src/languages/LanguageDefinition.ts +++ b/packages/cursorless-engine/src/languages/LanguageDefinition.ts @@ -3,6 +3,7 @@ import type { ScopeType, SimpleScopeType, SimpleScopeTypeType, + StringRecord, TreeSitter, } from "@cursorless/common"; import { @@ -12,6 +13,7 @@ import { type TextDocument, } from "@cursorless/common"; import { TreeSitterScopeHandler } from "../processTargets/modifiers/scopeHandlers"; +import { LanguageDefinitionCache } from "./LanguageDefinitionCache"; import { TreeSitterQuery } from "./TreeSitterQuery"; import type { QueryCapture } from "./TreeSitterQuery/QueryCapture"; import { validateQueryCaptures } from "./TreeSitterQuery/validateQueryCaptures"; @@ -21,6 +23,8 @@ import { validateQueryCaptures } from "./TreeSitterQuery/validateQueryCaptures"; * tree-sitter query used to extract scopes for the given language */ export class LanguageDefinition { + private cache: LanguageDefinitionCache; + private constructor( /** * The tree-sitter query used to extract scopes for the given language. @@ -28,7 +32,9 @@ export class LanguageDefinition { * language supports using new-style tree-sitter queries */ private query: TreeSitterQuery, - ) {} + ) { + this.cache = new LanguageDefinitionCache(); + } /** * Construct a language definition for the given language id, if the language @@ -93,10 +99,34 @@ export class LanguageDefinition { document: TextDocument, captureName: SimpleScopeTypeType, ): QueryCapture[] { - return this.query - .matches(document) - .map((match) => match.captures.find(({ name }) => name === captureName)) - .filter((capture) => capture != null); + if (!this.cache.isValid(document)) { + this.cache.update(document, this.getCapturesMap(document)); + } + + return this.cache.get(captureName); + } + + clearCache(): void { + this.cache = new LanguageDefinitionCache(); + } + + /** + * This is a low level function that returns a map of all captures in the document. + */ + private getCapturesMap(document: TextDocument): StringRecord { + const matches = this.query.matches(document); + const result: StringRecord = {}; + + for (const match of matches) { + for (const capture of match.captures) { + if (result[capture.name] == null) { + result[capture.name] = []; + } + result[capture.name]!.push(capture); + } + } + + return result; } } diff --git a/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts b/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts new file mode 100644 index 0000000000..16d7562361 --- /dev/null +++ b/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts @@ -0,0 +1,29 @@ +import type { + SimpleScopeTypeType, + StringRecord, + TextDocument, +} from "@cursorless/common"; +import type { QueryCapture } from "./TreeSitterQuery/QueryCapture"; + +export class LanguageDefinitionCache { + private documentUri: string = ""; + private documentVersion: number = -1; + private captures: StringRecord = {}; + + isValid(document: TextDocument) { + return ( + this.documentUri === document.uri.toString() && + this.documentVersion === document.version + ); + } + + update(document: TextDocument, captures: StringRecord) { + this.documentUri = document.uri.toString(); + this.documentVersion = document.version; + this.captures = captures; + } + + get(captureName: SimpleScopeTypeType): QueryCapture[] { + return this.captures[captureName] ?? []; + } +} diff --git a/packages/cursorless-engine/src/languages/LanguageDefinitions.ts b/packages/cursorless-engine/src/languages/LanguageDefinitions.ts index 53e7045692..e241383ebd 100644 --- a/packages/cursorless-engine/src/languages/LanguageDefinitions.ts +++ b/packages/cursorless-engine/src/languages/LanguageDefinitions.ts @@ -32,6 +32,17 @@ export interface LanguageDefinitions { */ get(languageId: string): LanguageDefinition | undefined; + /** + * Clear the cache of all language definitions. This is run at the start of a command. + * This isn't strict necessary for normal user operations since whenever the user + * makes a change to the document the document version is updated. When + * running our test though we keep closing and reopening an untitled document. + * That test document will have the same uri and version unfortunately. Also + * to be completely sure there isn't some extension doing similar trickery + * it's just good hygiene to clear the cache before every command. + */ + clearCache(): void; + /** * @deprecated Only for use in legacy containing scope stage */ @@ -155,6 +166,14 @@ export class LanguageDefinitionsImpl return definition === LANGUAGE_UNDEFINED ? undefined : definition; } + clearCache(): void { + for (const definition of this.languageDefinitions.values()) { + if (definition !== LANGUAGE_UNDEFINED) { + definition.clearCache(); + } + } + } + public getNodeAtLocation(document: TextDocument, range: Range): SyntaxNode { return this.treeSitter.getNodeAtLocation(document, range); } diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayNestedRangeFinder.test.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayNestedRangeFinder.test.ts new file mode 100644 index 0000000000..3c2c86d3b5 --- /dev/null +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayNestedRangeFinder.test.ts @@ -0,0 +1,30 @@ +import { Range } from "@cursorless/common"; +import { OneWayNestedRangeFinder } from "./OneWayNestedRangeFinder"; +import assert from "assert"; + +const items = [ + { range: new Range(0, 0, 0, 5) }, + { range: new Range(0, 1, 0, 4) }, + { range: new Range(0, 2, 0, 3) }, + { range: new Range(0, 6, 0, 8) }, +]; + +suite("OneWayNestedRangeFinder", () => { + test("getSmallestContaining 1", () => { + const finder = new OneWayNestedRangeFinder(items); + const actual = finder.getSmallestContaining(new Range(0, 2, 0, 2)); + assert.equal(actual?.range.toString(), new Range(0, 2, 0, 3).toString()); + }); + + test("getSmallestContaining 2", () => { + const finder = new OneWayNestedRangeFinder(items); + const actual = finder.getSmallestContaining(new Range(0, 7, 0, 7)); + assert.equal(actual?.range.toString(), new Range(0, 6, 0, 8).toString()); + }); + + test("getSmallestContaining 3", () => { + const finder = new OneWayNestedRangeFinder(items); + const actual = finder.getSmallestContaining(new Range(0, 0, 0, 0)); + assert.equal(actual?.range.toString(), new Range(0, 0, 0, 5).toString()); + }); +}); diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayNestedRangeFinder.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayNestedRangeFinder.ts new file mode 100644 index 0000000000..d001dcf264 --- /dev/null +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayNestedRangeFinder.ts @@ -0,0 +1,76 @@ +import type { Range } from "@cursorless/common"; +import { OneWayRangeFinder } from "./OneWayRangeFinder"; + +/** + * Given a list of ranges (the haystack), allows the client to search for smallest range containing a range (the needle). + * Has the following requirements: + * - the haystack must be sorted in document order + * - **the needles must be in document order as well**. This enables us to avoid backtracking as you search for a sequence of items. + * - the haystack entries **may** be nested, but one haystack entry cannot partially contain another + */ +export class OneWayNestedRangeFinder { + private children: OneWayRangeFinder>; + + /** + * @param items The items to search in. Must be sorted in document order. + */ + constructor(items: T[]) { + this.children = createNodes(items); + } + + getSmallestContaining(separator: Range): T | undefined { + return this.children + .getContaining(separator) + ?.getSmallestContaining(separator); + } +} + +function createNodes( + items: T[], +): OneWayRangeFinder> { + const results: RangeLookupTreeNode[] = []; + const parents: RangeLookupTreeNode[] = []; + + for (const item of items) { + const node = new RangeLookupTreeNode(item); + + while ( + parents.length > 0 && + !parents[parents.length - 1].range.contains(item.range) + ) { + parents.pop(); + } + + const parent = parents[parents.length - 1]; + + if (parent != null) { + parent.children.add(node); + } else { + results.push(node); + } + + parents.push(node); + } + + return new OneWayRangeFinder(results); +} + +class RangeLookupTreeNode { + public children: OneWayRangeFinder>; + + constructor(private item: T) { + this.children = new OneWayRangeFinder([]); + } + + get range(): Range { + return this.item.range; + } + + getSmallestContaining(range: Range): T { + const child = this.children + .getContaining(range) + ?.getSmallestContaining(range); + + return child ?? this.item; + } +} diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayRangeFinder.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayRangeFinder.ts new file mode 100644 index 0000000000..c0b9b6e188 --- /dev/null +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayRangeFinder.ts @@ -0,0 +1,52 @@ +import type { Range } from "@cursorless/common"; + +/** + * Given a list of ranges (the haystack), allows the client to search for a sequence of ranges (the needles). + * Has the following requirements: + * - the haystack must be sorted in document order + * - **the needles must be in document order as well**. This enables us to avoid backtracking as you search for a sequence of items. + * - the haystack entries must not overlap. Adjacent is fine + */ +export class OneWayRangeFinder { + private index = 0; + + /** + * @param items The items to search in. Must be sorted in document order. + */ + constructor(private items: T[]) {} + + add(item: T) { + this.items.push(item); + } + + contains(searchItem: Range): boolean { + return this.advance(searchItem); + } + + getContaining(searchItem: Range): T | undefined { + if (this.advance(searchItem)) { + return this.items[this.index]; + } + + return undefined; + } + + private advance(searchItem: Range): boolean { + while (this.index < this.items.length) { + const range = this.items[this.index].range; + + if (range.contains(searchItem)) { + return true; + } + + // Search item is before the range. Since the ranges are sorted, we can stop here. + if (searchItem.end.isBeforeOrEqual(range.start)) { + return false; + } + + this.index++; + } + + return false; + } +} diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts index 18f42e3845..7f01224043 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -1,6 +1,14 @@ -import { matchAll, Range, type TextDocument } from "@cursorless/common"; +import { + matchAllIterator, + Range, + type SimpleScopeTypeType, + type TextDocument, +} from "@cursorless/common"; import type { LanguageDefinition } from "../../../../languages/LanguageDefinition"; +import type { QueryCapture } from "../../../../languages/TreeSitterQuery/QueryCapture"; import { getDelimiterRegex } from "./getDelimiterRegex"; +import { OneWayRangeFinder } from "./OneWayRangeFinder"; +import { OneWayNestedRangeFinder } from "./OneWayNestedRangeFinder"; import type { DelimiterOccurrence, IndividualDelimiter } from "./types"; /** @@ -20,12 +28,13 @@ export function getDelimiterOccurrences( return []; } - const delimiterRegex = getDelimiterRegex(individualDelimiters); - - const disqualifyDelimiters = - languageDefinition?.getCaptures(document, "disqualifyDelimiter") ?? []; - const textFragments = - languageDefinition?.getCaptures(document, "textFragment") ?? []; + const disqualifyDelimiters = new OneWayRangeFinder( + getSortedCaptures(languageDefinition, document, "disqualifyDelimiter"), + ); + // We need a tree for text fragments since they can be nested + const textFragments = new OneWayNestedRangeFinder( + getSortedCaptures(languageDefinition, document, "textFragment"), + ); const delimiterTextToDelimiterInfoMap = Object.fromEntries( individualDelimiters.map((individualDelimiter) => [ @@ -34,28 +43,43 @@ export function getDelimiterOccurrences( ]), ); - const text = document.getText(); + const regexMatches = matchAllIterator( + document.getText(), + getDelimiterRegex(individualDelimiters), + ); + + const results: DelimiterOccurrence[] = []; - return matchAll(text, delimiterRegex, (match): DelimiterOccurrence => { + for (const match of regexMatches) { const text = match[0]; const range = new Range( document.positionAt(match.index!), document.positionAt(match.index! + text.length), ); - const isDisqualified = disqualifyDelimiters.some( - (c) => c.range.contains(range) && !c.hasError(), - ); + const delimiter = disqualifyDelimiters.getContaining(range); + const isDisqualified = delimiter != null && !delimiter.hasError(); - const textFragmentRange = textFragments.find((c) => - c.range.contains(range), - )?.range; - - return { - delimiterInfo: delimiterTextToDelimiterInfoMap[text], - isDisqualified, - textFragmentRange, - range, - }; - }); + if (!isDisqualified) { + const textFragmentRange = + textFragments.getSmallestContaining(range)?.range; + results.push({ + delimiterInfo: delimiterTextToDelimiterInfoMap[text], + textFragmentRange, + range, + }); + } + } + + return results; +} + +function getSortedCaptures( + languageDefinition: LanguageDefinition | undefined, + document: TextDocument, + captureName: SimpleScopeTypeType, +): QueryCapture[] { + const items = languageDefinition?.getCaptures(document, captureName) ?? []; + items.sort((a, b) => a.range.start.compareTo(b.range.start)); + return items; } diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts index b1a573877c..027476ab7a 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts @@ -25,15 +25,10 @@ export function getSurroundingPairOccurrences( for (const occurrence of delimiterOccurrences) { const { delimiterInfo: { delimiterName, side, isSingleLine }, - isDisqualified, textFragmentRange, range, } = occurrence; - if (isDisqualified) { - continue; - } - let openingDelimiters = openingDelimiterOccurrences.get(delimiterName); if (isSingleLine) { diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/types.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/types.ts index 923f0e250a..253b8debdd 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/types.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/types.ts @@ -48,18 +48,11 @@ export interface DelimiterOccurrence { */ range: Range; - /** - * If `true`, this delimiter is disqualified from being considered as a - * surrounding pair delimiter, because it has been tagged as such based on a - * parse tree query. - */ - isDisqualified: boolean; - /** * If the delimiter is part of a text fragment, eg a string or comment, this * will be the range of the text fragment. */ - textFragmentRange?: Range; + textFragmentRange: Range | undefined; } /** diff --git a/packages/cursorless-engine/src/runCommand.ts b/packages/cursorless-engine/src/runCommand.ts index f30bf185be..bf78495cd7 100644 --- a/packages/cursorless-engine/src/runCommand.ts +++ b/packages/cursorless-engine/src/runCommand.ts @@ -71,6 +71,8 @@ export async function runCommand( commandRunner = decorator.wrapCommandRunner(readableHatMap, commandRunner); } + languageDefinitions.clearCache(); + const response = await commandRunner.run(commandComplete); return await unwrapLegacyCommandResponse(command, response);