From 2e7417032894609b939708a968cb2854a23eff8a Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Sun, 22 Dec 2024 09:13:40 +0100 Subject: [PATCH 01/32] Increased surrounding pair performance --- packages/common/src/index.ts | 1 + .../src/languages/LanguageDefinition.ts | 23 +++++++++++ .../RangeIterator.ts | 41 +++++++++++++++++++ .../getDelimiterOccurrences.ts | 35 +++++++++------- 4 files changed, 86 insertions(+), 14 deletions(-) create mode 100644 packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts 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/cursorless-engine/src/languages/LanguageDefinition.ts b/packages/cursorless-engine/src/languages/LanguageDefinition.ts index 25f8ac29fc..b7df6aba14 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 { @@ -98,6 +99,28 @@ export class LanguageDefinition { .map((match) => match.captures.find(({ name }) => name === captureName)) .filter((capture) => capture != null); } + + getMultipleCaptures( + document: TextDocument, + captureNames: SimpleScopeTypeType[], + ): StringRecord { + const matches = this.query.matches(document); + const result: StringRecord = {}; + const captureNamesSet = new Set(captureNames); + + for (const match of matches) { + for (const capture of match.captures) { + if (captureNamesSet.has(capture.name)) { + if (result[capture.name] == null) { + result[capture.name] = []; + } + result[capture.name]!.push(capture); + } + } + } + + return result; + } } /** diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts new file mode 100644 index 0000000000..77d8440587 --- /dev/null +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts @@ -0,0 +1,41 @@ +import type { Range } from "@cursorless/common"; + +/** + * An iterator that allows for efficient lookup of ranges that contain a search item. + */ +export class RangeIterator { + private index = 0; + + constructor(public items: T[]) {} + + contains(searchItem: Range): boolean { + return this.advance(searchItem); + } + + getContaining(searchItem: Range): T | undefined { + if (!this.advance(searchItem)) { + return undefined; + } + + return this.items[this.index]; + } + + 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.isBefore(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..306c2c2eda 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,7 @@ import { matchAll, Range, type TextDocument } from "@cursorless/common"; import type { LanguageDefinition } from "../../../../languages/LanguageDefinition"; import { getDelimiterRegex } from "./getDelimiterRegex"; +import { RangeIterator } from "./RangeIterator"; import type { DelimiterOccurrence, IndividualDelimiter } from "./types"; /** @@ -22,10 +23,15 @@ export function getDelimiterOccurrences( const delimiterRegex = getDelimiterRegex(individualDelimiters); - const disqualifyDelimiters = - languageDefinition?.getCaptures(document, "disqualifyDelimiter") ?? []; - const textFragments = - languageDefinition?.getCaptures(document, "textFragment") ?? []; + const captures = languageDefinition?.getMultipleCaptures(document, [ + "disqualifyDelimiter", + "textFragment", + ]); + const disqualifyDelimiters = captures?.disqualifyDelimiter ?? []; + const textFragments = captures?.textFragment ?? []; + + const disqualifyDelimitersIterator = new RangeIterator(disqualifyDelimiters); + const textFragmentsIterator = new RangeIterator(textFragments); const delimiterTextToDelimiterInfoMap = Object.fromEntries( individualDelimiters.map((individualDelimiter) => [ @@ -34,6 +40,15 @@ export function getDelimiterOccurrences( ]), ); + const isDisqualified = (range: Range): boolean => { + const delimiter = disqualifyDelimitersIterator.getContaining(range); + return delimiter != null && !delimiter.hasError(); + }; + + const getTextFragmentRange = (range: Range): Range | undefined => { + return textFragmentsIterator.getContaining(range)?.range; + }; + const text = document.getText(); return matchAll(text, delimiterRegex, (match): DelimiterOccurrence => { @@ -43,18 +58,10 @@ export function getDelimiterOccurrences( document.positionAt(match.index! + text.length), ); - const isDisqualified = disqualifyDelimiters.some( - (c) => c.range.contains(range) && !c.hasError(), - ); - - const textFragmentRange = textFragments.find((c) => - c.range.contains(range), - )?.range; - return { delimiterInfo: delimiterTextToDelimiterInfoMap[text], - isDisqualified, - textFragmentRange, + isDisqualified: isDisqualified(range), + textFragmentRange: getTextFragmentRange(range), range, }; }); From eab4aa180ce54fa3ea88358a2d9c814bf9801bd0 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Sun, 22 Dec 2024 09:36:55 +0100 Subject: [PATCH 02/32] Slight tweak --- .../SurroundingPairScopeHandler/RangeIterator.ts | 9 ++++++++- .../getDelimiterOccurrences.ts | 13 ++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts index 77d8440587..d18e321844 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts @@ -6,7 +6,14 @@ import type { Range } from "@cursorless/common"; export class RangeIterator { private index = 0; - constructor(public items: T[]) {} + constructor( + public items: T[], + sortItems = false, + ) { + if (sortItems) { + this.items.sort((a, b) => a.range.start.compareTo(b.range.start)); + } + } contains(searchItem: Range): boolean { return this.advance(searchItem); 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 306c2c2eda..b4df490f2b 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -27,11 +27,14 @@ export function getDelimiterOccurrences( "disqualifyDelimiter", "textFragment", ]); - const disqualifyDelimiters = captures?.disqualifyDelimiter ?? []; - const textFragments = captures?.textFragment ?? []; - - const disqualifyDelimitersIterator = new RangeIterator(disqualifyDelimiters); - const textFragmentsIterator = new RangeIterator(textFragments); + const disqualifyDelimitersIterator = new RangeIterator( + captures?.disqualifyDelimiter ?? [], + true, // Sort items + ); + const textFragmentsIterator = new RangeIterator( + captures?.textFragment ?? [], + true, + ); const delimiterTextToDelimiterInfoMap = Object.fromEntries( individualDelimiters.map((individualDelimiter) => [ From db385dae5484517e085303261e8dbaae382b427d Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 10:19:14 +0100 Subject: [PATCH 03/32] Cleanup --- packages/common/src/util/regex.ts | 14 ++++-- .../RangeIterator.ts | 19 +++---- .../getDelimiterOccurrences.ts | 49 ++++++++++++------- .../getSurroundingPairOccurrences.ts | 5 -- .../SurroundingPairScopeHandler/types.ts | 7 --- 5 files changed, 47 insertions(+), 47 deletions(-) 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/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts index d18e321844..ff2c690b4f 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts @@ -2,29 +2,26 @@ import type { Range } from "@cursorless/common"; /** * An iterator that allows for efficient lookup of ranges that contain a search item. + * The items must be sorted in document order. */ export class RangeIterator { private index = 0; - constructor( - public items: T[], - sortItems = false, - ) { - if (sortItems) { - this.items.sort((a, b) => a.range.start.compareTo(b.range.start)); - } - } + /** + * @param items The items to iterate over. Must be sorted in document order. + */ + constructor(public items: T[]) {} contains(searchItem: Range): boolean { return this.advance(searchItem); } getContaining(searchItem: Range): T | undefined { - if (!this.advance(searchItem)) { - return undefined; + if (this.advance(searchItem)) { + return this.items[this.index]; } - return this.items[this.index]; + return undefined; } private advance(searchItem: Range): boolean { 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 b4df490f2b..e0eedd0473 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -1,5 +1,6 @@ -import { matchAll, Range, type TextDocument } from "@cursorless/common"; +import { matchAllIterator, Range, type TextDocument } from "@cursorless/common"; import type { LanguageDefinition } from "../../../../languages/LanguageDefinition"; +import type { QueryCapture } from "../../../../languages/TreeSitterQuery/QueryCapture"; import { getDelimiterRegex } from "./getDelimiterRegex"; import { RangeIterator } from "./RangeIterator"; import type { DelimiterOccurrence, IndividualDelimiter } from "./types"; @@ -21,20 +22,14 @@ export function getDelimiterOccurrences( return []; } - const delimiterRegex = getDelimiterRegex(individualDelimiters); - const captures = languageDefinition?.getMultipleCaptures(document, [ "disqualifyDelimiter", "textFragment", ]); - const disqualifyDelimitersIterator = new RangeIterator( - captures?.disqualifyDelimiter ?? [], - true, // Sort items - ); - const textFragmentsIterator = new RangeIterator( - captures?.textFragment ?? [], - true, + const disqualifyDelimitersIterator = createRangeIterator( + captures?.disqualifyDelimiter, ); + const textFragmentsIterator = createRangeIterator(captures?.textFragment); const delimiterTextToDelimiterInfoMap = Object.fromEntries( individualDelimiters.map((individualDelimiter) => [ @@ -52,20 +47,36 @@ export function getDelimiterOccurrences( return textFragmentsIterator.getContaining(range)?.range; }; - const text = document.getText(); + const matchIterator = matchAllIterator( + document.getText(), + getDelimiterRegex(individualDelimiters), + ); + + const results: DelimiterOccurrence[] = []; - return matchAll(text, delimiterRegex, (match): DelimiterOccurrence => { + for (const match of matchIterator) { const text = match[0]; const range = new Range( document.positionAt(match.index!), document.positionAt(match.index! + text.length), ); - return { - delimiterInfo: delimiterTextToDelimiterInfoMap[text], - isDisqualified: isDisqualified(range), - textFragmentRange: getTextFragmentRange(range), - range, - }; - }); + if (!isDisqualified(range)) { + results.push({ + delimiterInfo: delimiterTextToDelimiterInfoMap[text], + textFragmentRange: getTextFragmentRange(range), + range, + }); + } + } + + return results; +} + +function createRangeIterator( + captures: QueryCapture[] | undefined, +): RangeIterator { + const items = captures ?? []; + items.sort((a, b) => a.range.start.compareTo(b.range.start)); + return new RangeIterator(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..3c9c0fced1 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/types.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/types.ts @@ -48,13 +48,6 @@ 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. From 3dcac4fa4c4e8eb7b79ef7177853d8daead2aa92 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 10:23:40 +0100 Subject: [PATCH 04/32] More clean up --- .../getDelimiterOccurrences.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) 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 e0eedd0473..522c30b502 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -38,15 +38,6 @@ export function getDelimiterOccurrences( ]), ); - const isDisqualified = (range: Range): boolean => { - const delimiter = disqualifyDelimitersIterator.getContaining(range); - return delimiter != null && !delimiter.hasError(); - }; - - const getTextFragmentRange = (range: Range): Range | undefined => { - return textFragmentsIterator.getContaining(range)?.range; - }; - const matchIterator = matchAllIterator( document.getText(), getDelimiterRegex(individualDelimiters), @@ -61,10 +52,13 @@ export function getDelimiterOccurrences( document.positionAt(match.index! + text.length), ); - if (!isDisqualified(range)) { + const delimiter = disqualifyDelimitersIterator.getContaining(range); + const isDisqualified = delimiter != null && !delimiter.hasError(); + + if (!isDisqualified) { results.push({ delimiterInfo: delimiterTextToDelimiterInfoMap[text], - textFragmentRange: getTextFragmentRange(range), + textFragmentRange: textFragmentsIterator.getContaining(range)?.range, range, }); } From 706127e54b0b5b6aa7a4a6a5a36838fc23217348 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 10:26:50 +0100 Subject: [PATCH 05/32] rename --- .../getDelimiterOccurrences.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 522c30b502..b20a676aae 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -26,10 +26,10 @@ export function getDelimiterOccurrences( "disqualifyDelimiter", "textFragment", ]); - const disqualifyDelimitersIterator = createRangeIterator( + const disqualifyDelimiters = createRangeIterator( captures?.disqualifyDelimiter, ); - const textFragmentsIterator = createRangeIterator(captures?.textFragment); + const textFragments = createRangeIterator(captures?.textFragment); const delimiterTextToDelimiterInfoMap = Object.fromEntries( individualDelimiters.map((individualDelimiter) => [ @@ -52,13 +52,13 @@ export function getDelimiterOccurrences( document.positionAt(match.index! + text.length), ); - const delimiter = disqualifyDelimitersIterator.getContaining(range); + const delimiter = disqualifyDelimiters.getContaining(range); const isDisqualified = delimiter != null && !delimiter.hasError(); if (!isDisqualified) { results.push({ delimiterInfo: delimiterTextToDelimiterInfoMap[text], - textFragmentRange: textFragmentsIterator.getContaining(range)?.range, + textFragmentRange: textFragments.getContaining(range)?.range, range, }); } From 8dd21f152416e4a7a3bb955c55b33ed9ffd28d2f Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 10:27:59 +0100 Subject: [PATCH 06/32] more rename --- .../SurroundingPairScopeHandler/getDelimiterOccurrences.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 b20a676aae..db1fce0a13 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -38,14 +38,14 @@ export function getDelimiterOccurrences( ]), ); - const matchIterator = matchAllIterator( + const regexMatches = matchAllIterator( document.getText(), getDelimiterRegex(individualDelimiters), ); const results: DelimiterOccurrence[] = []; - for (const match of matchIterator) { + for (const match of regexMatches) { const text = match[0]; const range = new Range( document.positionAt(match.index!), From 1c3f026401adca3e26d9342e16a38ed0a15036c5 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 12:04:49 +0100 Subject: [PATCH 07/32] clean up type --- .../scopeHandlers/SurroundingPairScopeHandler/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3c9c0fced1..253b8debdd 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/types.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/types.ts @@ -52,7 +52,7 @@ export interface DelimiterOccurrence { * 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; } /** From b4c8b6276f59ad49723c6599315c62effef771ac Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 12:25:51 +0100 Subject: [PATCH 08/32] Bugfix --- .../surroundingPair/textual/changePair2.yml | 23 +++++++++++++++++++ .../RangeIterator.ts | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 data/fixtures/recorded/surroundingPair/textual/changePair2.yml diff --git a/data/fixtures/recorded/surroundingPair/textual/changePair2.yml b/data/fixtures/recorded/surroundingPair/textual/changePair2.yml new file mode 100644 index 0000000000..e5bff4f0d0 --- /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, \"]\", 3];" + 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/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts index ff2c690b4f..9f19bf8029 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts @@ -33,7 +33,7 @@ export class RangeIterator { } // Search item is before the range. Since the ranges are sorted, we can stop here. - if (searchItem.end.isBefore(range.start)) { + if (searchItem.end.isBeforeOrEqual(range.start)) { return false; } From 4663d2a62b0f6284b7e97d6a603a057acdb200b9 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 12:27:08 +0100 Subject: [PATCH 09/32] Update test --- .../fixtures/recorded/surroundingPair/textual/changePair2.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data/fixtures/recorded/surroundingPair/textual/changePair2.yml b/data/fixtures/recorded/surroundingPair/textual/changePair2.yml index e5bff4f0d0..5d699c045a 100644 --- a/data/fixtures/recorded/surroundingPair/textual/changePair2.yml +++ b/data/fixtures/recorded/surroundingPair/textual/changePair2.yml @@ -11,13 +11,13 @@ command: scopeType: {type: surroundingPair, delimiter: any} usePrePhraseSnapshot: true initialState: - documentContents: "[1, 2, \"]\", 3];" + documentContents: "[1, \"]\", 2]" selections: - anchor: {line: 0, character: 1} active: {line: 0, character: 1} marks: {} finalState: - documentContents: ; + documentContents: "" selections: - anchor: {line: 0, character: 0} active: {line: 0, character: 0} From 2bf3105e7e0ae9bc69f52a3fd70a33093672bc88 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 13:22:56 +0100 Subject: [PATCH 10/32] Added cache --- .../src/languages/LanguageDefinition.ts | 29 +++++++-------- .../src/languages/LanguageDefinitionCache.ts | 36 +++++++++++++++++++ .../getDelimiterOccurrences.ts | 10 +++--- 3 files changed, 55 insertions(+), 20 deletions(-) create mode 100644 packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts diff --git a/packages/cursorless-engine/src/languages/LanguageDefinition.ts b/packages/cursorless-engine/src/languages/LanguageDefinition.ts index b7df6aba14..ea00e1cfaf 100644 --- a/packages/cursorless-engine/src/languages/LanguageDefinition.ts +++ b/packages/cursorless-engine/src/languages/LanguageDefinition.ts @@ -13,10 +13,13 @@ 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"; +const cache = new LanguageDefinitionCache(); + /** * Represents a language definition for a single language, including the * tree-sitter query used to extract scopes for the given language @@ -94,28 +97,26 @@ 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 (!cache.isValid(document)) { + cache.update(document, this.getCapturesMap(document)); + } + + return cache.get(captureName); } - getMultipleCaptures( - document: TextDocument, - captureNames: SimpleScopeTypeType[], - ): StringRecord { + /** + * 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 = {}; - const captureNamesSet = new Set(captureNames); for (const match of matches) { for (const capture of match.captures) { - if (captureNamesSet.has(capture.name)) { - if (result[capture.name] == null) { - result[capture.name] = []; - } - result[capture.name]!.push(capture); + if (result[capture.name] == null) { + result[capture.name] = []; } + result[capture.name]!.push(capture); } } diff --git a/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts b/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts new file mode 100644 index 0000000000..0f09ea2857 --- /dev/null +++ b/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts @@ -0,0 +1,36 @@ +import type { + SimpleScopeTypeType, + StringRecord, + TextDocument, +} from "@cursorless/common"; +import type { QueryCapture } from "./TreeSitterQuery/QueryCapture"; +import { ide } from ".."; + +export class LanguageDefinitionCache { + private languageId: string = ""; + private documentUri: string = ""; + private documentVersion: number = -1; + private captures: StringRecord = {}; + + isValid(document: TextDocument) { + if (ide().runMode === "test") { + return false; + } + return ( + this.languageId === document.languageId && + this.documentUri === document.uri.toString() && + this.documentVersion === document.version + ); + } + + update(document: TextDocument, captures: StringRecord) { + this.languageId = document.languageId; + 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/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts index db1fce0a13..9e902dff72 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -22,14 +22,12 @@ export function getDelimiterOccurrences( return []; } - const captures = languageDefinition?.getMultipleCaptures(document, [ - "disqualifyDelimiter", - "textFragment", - ]); const disqualifyDelimiters = createRangeIterator( - captures?.disqualifyDelimiter, + languageDefinition?.getCaptures(document, "disqualifyDelimiter"), + ); + const textFragments = createRangeIterator( + languageDefinition?.getCaptures(document, "textFragment"), ); - const textFragments = createRangeIterator(captures?.textFragment); const delimiterTextToDelimiterInfoMap = Object.fromEntries( individualDelimiters.map((individualDelimiter) => [ From bc417d8572d37e29c849bf16fdbfb8439934e2a6 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 13:53:55 +0100 Subject: [PATCH 11/32] Update cache --- .../DisabledLanguageDefinitions.ts | 4 ++++ .../src/languages/LanguageDefinition.ts | 18 ++++++++++++------ .../src/languages/LanguageDefinitionCache.ts | 13 ++++++------- .../src/languages/LanguageDefinitions.ts | 13 +++++++++++++ packages/cursorless-engine/src/runCommand.ts | 2 ++ 5 files changed, 37 insertions(+), 13 deletions(-) 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 ea00e1cfaf..a217fc54c8 100644 --- a/packages/cursorless-engine/src/languages/LanguageDefinition.ts +++ b/packages/cursorless-engine/src/languages/LanguageDefinition.ts @@ -18,13 +18,13 @@ import { TreeSitterQuery } from "./TreeSitterQuery"; import type { QueryCapture } from "./TreeSitterQuery/QueryCapture"; import { validateQueryCaptures } from "./TreeSitterQuery/validateQueryCaptures"; -const cache = new LanguageDefinitionCache(); - /** * Represents a language definition for a single language, including the * 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. @@ -32,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 @@ -97,11 +99,15 @@ export class LanguageDefinition { document: TextDocument, captureName: SimpleScopeTypeType, ): QueryCapture[] { - if (!cache.isValid(document)) { - cache.update(document, this.getCapturesMap(document)); + if (!this.cache.isValid(document)) { + this.cache.update(document, this.getCapturesMap(document)); } - return cache.get(captureName); + return this.cache.get(captureName); + } + + clearCache(): void { + this.cache.clear(); } /** diff --git a/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts b/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts index 0f09ea2857..d618ebf809 100644 --- a/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts +++ b/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts @@ -4,27 +4,26 @@ import type { TextDocument, } from "@cursorless/common"; import type { QueryCapture } from "./TreeSitterQuery/QueryCapture"; -import { ide } from ".."; export class LanguageDefinitionCache { - private languageId: string = ""; private documentUri: string = ""; private documentVersion: number = -1; private captures: StringRecord = {}; + clear() { + this.documentUri = ""; + this.documentVersion = -1; + this.captures = {}; + } + isValid(document: TextDocument) { - if (ide().runMode === "test") { - return false; - } return ( - this.languageId === document.languageId && this.documentUri === document.uri.toString() && this.documentVersion === document.version ); } update(document: TextDocument, captures: StringRecord) { - this.languageId = document.languageId; this.documentUri = document.uri.toString(); this.documentVersion = document.version; this.captures = captures; diff --git a/packages/cursorless-engine/src/languages/LanguageDefinitions.ts b/packages/cursorless-engine/src/languages/LanguageDefinitions.ts index 53e7045692..55bcd13e7b 100644 --- a/packages/cursorless-engine/src/languages/LanguageDefinitions.ts +++ b/packages/cursorless-engine/src/languages/LanguageDefinitions.ts @@ -32,6 +32,11 @@ export interface LanguageDefinitions { */ get(languageId: string): LanguageDefinition | undefined; + /** + * Clear the cache of all language definitions. This is run at the start of a command. + */ + clearCache(): void; + /** * @deprecated Only for use in legacy containing scope stage */ @@ -155,6 +160,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/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); From d8d44ed7789d20bd6e7a574002d0c34ae6ef60bb Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 15:17:18 +0100 Subject: [PATCH 12/32] Added tree --- .../surroundingPair/textual/changePair3.yml | 23 ++++++++++++ .../SurroundingPairScopeHandler/RangeNode.ts | 21 +++++++++++ .../createRangeTree.ts | 37 +++++++++++++++++++ .../getDelimiterOccurrences.ts | 26 +++++++++---- 4 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 data/fixtures/recorded/surroundingPair/textual/changePair3.yml create mode 100644 packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeNode.ts create mode 100644 packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts diff --git a/data/fixtures/recorded/surroundingPair/textual/changePair3.yml b/data/fixtures/recorded/surroundingPair/textual/changePair3.yml new file mode 100644 index 0000000000..4c900af99e --- /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/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeNode.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeNode.ts new file mode 100644 index 0000000000..6cf8826a5f --- /dev/null +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeNode.ts @@ -0,0 +1,21 @@ +import type { Range } from "@cursorless/common"; + +export class RangeNode { + children: RangeNode[] = []; + + constructor(private item: T) {} + + get range(): Range { + return this.item.range; + } + + getSmallLestContaining(separator: Range): T { + for (const child of this.children) { + if (child.item.range.contains(separator)) { + return child.getSmallLestContaining(separator); + } + } + + return this.item; + } +} diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts new file mode 100644 index 0000000000..06beec8e43 --- /dev/null +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts @@ -0,0 +1,37 @@ +import type { Range } from "@cursorless/common"; +import { RangeNode } from "./RangeNode"; + +/** + * Creates a tree of ranges from a list of ranges. This improves containing lookup time. + * @param items The ranges to create a tree from. + * @returns The root nodes of the tree. + */ +export function createRangeTree( + items: T[], +): RangeNode[] { + const results: RangeNode[] = []; + const parents: RangeNode[] = []; + + for (const item of items) { + const node = new RangeNode(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.push(node); + } else { + results.push(node); + } + + parents.push(node); + } + + return results; +} 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 9e902dff72..3fb04e746a 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,7 @@ import { matchAllIterator, Range, type TextDocument } from "@cursorless/common"; import type { LanguageDefinition } from "../../../../languages/LanguageDefinition"; import type { QueryCapture } from "../../../../languages/TreeSitterQuery/QueryCapture"; +import { createRangeTree } from "./createRangeTree"; import { getDelimiterRegex } from "./getDelimiterRegex"; import { RangeIterator } from "./RangeIterator"; import type { DelimiterOccurrence, IndividualDelimiter } from "./types"; @@ -22,11 +23,17 @@ export function getDelimiterOccurrences( return []; } - const disqualifyDelimiters = createRangeIterator( - languageDefinition?.getCaptures(document, "disqualifyDelimiter"), + const disqualifyDelimiters = new RangeIterator( + getSortedCaptures( + languageDefinition?.getCaptures(document, "disqualifyDelimiter"), + ), ); - const textFragments = createRangeIterator( - languageDefinition?.getCaptures(document, "textFragment"), + const textFragments = new RangeIterator( + createRangeTree( + getSortedCaptures( + languageDefinition?.getCaptures(document, "textFragment"), + ), + ), ); const delimiterTextToDelimiterInfoMap = Object.fromEntries( @@ -54,9 +61,12 @@ export function getDelimiterOccurrences( const isDisqualified = delimiter != null && !delimiter.hasError(); if (!isDisqualified) { + const textFragmentRange = textFragments + .getContaining(range) + ?.getSmallLestContaining(range).range; results.push({ delimiterInfo: delimiterTextToDelimiterInfoMap[text], - textFragmentRange: textFragments.getContaining(range)?.range, + textFragmentRange, range, }); } @@ -65,10 +75,10 @@ export function getDelimiterOccurrences( return results; } -function createRangeIterator( +function getSortedCaptures( captures: QueryCapture[] | undefined, -): RangeIterator { +): QueryCapture[] { const items = captures ?? []; items.sort((a, b) => a.range.start.compareTo(b.range.start)); - return new RangeIterator(items); + return items; } From 6474a91689b616e3053ddbf7a6d8a8b2301b70d0 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 15:20:58 +0100 Subject: [PATCH 13/32] Clean up --- .../getDelimiterOccurrences.ts | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) 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 3fb04e746a..c9229f1bb2 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -1,4 +1,9 @@ -import { matchAllIterator, 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 { createRangeTree } from "./createRangeTree"; @@ -24,15 +29,12 @@ export function getDelimiterOccurrences( } const disqualifyDelimiters = new RangeIterator( - getSortedCaptures( - languageDefinition?.getCaptures(document, "disqualifyDelimiter"), - ), + getSortedCaptures(languageDefinition, document, "disqualifyDelimiter"), ); const textFragments = new RangeIterator( + // We need to create a tree for text fragments since they can be nested createRangeTree( - getSortedCaptures( - languageDefinition?.getCaptures(document, "textFragment"), - ), + getSortedCaptures(languageDefinition, document, "textFragment"), ), ); @@ -76,9 +78,11 @@ export function getDelimiterOccurrences( } function getSortedCaptures( - captures: QueryCapture[] | undefined, + languageDefinition: LanguageDefinition | undefined, + document: TextDocument, + captureName: SimpleScopeTypeType, ): QueryCapture[] { - const items = captures ?? []; + const items = languageDefinition?.getCaptures(document, captureName) ?? []; items.sort((a, b) => a.range.start.compareTo(b.range.start)); return items; } From 99662784d42b7ff845f2af729160cbfa3e395f31 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 15:23:17 +0100 Subject: [PATCH 14/32] Update test --- data/fixtures/recorded/surroundingPair/textual/changePair2.yml | 2 +- data/fixtures/recorded/surroundingPair/textual/changePair3.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/data/fixtures/recorded/surroundingPair/textual/changePair2.yml b/data/fixtures/recorded/surroundingPair/textual/changePair2.yml index 5d699c045a..efe2c1e421 100644 --- a/data/fixtures/recorded/surroundingPair/textual/changePair2.yml +++ b/data/fixtures/recorded/surroundingPair/textual/changePair2.yml @@ -11,7 +11,7 @@ command: scopeType: {type: surroundingPair, delimiter: any} usePrePhraseSnapshot: true initialState: - documentContents: "[1, \"]\", 2]" + documentContents: "[1, ']', 2]" selections: - anchor: {line: 0, character: 1} active: {line: 0, character: 1} diff --git a/data/fixtures/recorded/surroundingPair/textual/changePair3.yml b/data/fixtures/recorded/surroundingPair/textual/changePair3.yml index 4c900af99e..6615e2530c 100644 --- a/data/fixtures/recorded/surroundingPair/textual/changePair3.yml +++ b/data/fixtures/recorded/surroundingPair/textual/changePair3.yml @@ -11,7 +11,7 @@ command: scopeType: {type: surroundingPair, delimiter: any} usePrePhraseSnapshot: true initialState: - documentContents: "`[1, ${\"]\"}, 3]`" + documentContents: "`[1, ${']'}, 3]`" selections: - anchor: {line: 0, character: 1} active: {line: 0, character: 1} From f035b77001a7b4dbe3224fe42a2a762ef51b7a6e Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 15:25:17 +0100 Subject: [PATCH 15/32] rename --- .../{RangeNode.ts => RangeTreeNode.ts} | 4 ++-- .../SurroundingPairScopeHandler/createRangeTree.ts | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) rename packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/{RangeNode.ts => RangeTreeNode.ts} (80%) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeNode.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts similarity index 80% rename from packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeNode.ts rename to packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts index 6cf8826a5f..2220dea90b 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeNode.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts @@ -1,7 +1,7 @@ import type { Range } from "@cursorless/common"; -export class RangeNode { - children: RangeNode[] = []; +export class RangeTreeNode { + children: RangeTreeNode[] = []; constructor(private item: T) {} diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts index 06beec8e43..13b62a1d90 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts @@ -1,5 +1,5 @@ import type { Range } from "@cursorless/common"; -import { RangeNode } from "./RangeNode"; +import { RangeTreeNode } from "./RangeTreeNode"; /** * Creates a tree of ranges from a list of ranges. This improves containing lookup time. @@ -8,12 +8,12 @@ import { RangeNode } from "./RangeNode"; */ export function createRangeTree( items: T[], -): RangeNode[] { - const results: RangeNode[] = []; - const parents: RangeNode[] = []; +): RangeTreeNode[] { + const results: RangeTreeNode[] = []; + const parents: RangeTreeNode[] = []; for (const item of items) { - const node = new RangeNode(item); + const node = new RangeTreeNode(item); while ( parents.length > 0 && From 1d7b1f0ef48183e6e16a3937792db83d28f1708c Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 15:28:27 +0100 Subject: [PATCH 16/32] Added comment --- .../SurroundingPairScopeHandler/createRangeTree.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts index 13b62a1d90..d42610c133 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts @@ -3,7 +3,7 @@ import { RangeTreeNode } from "./RangeTreeNode"; /** * Creates a tree of ranges from a list of ranges. This improves containing lookup time. - * @param items The ranges to create a tree from. + * @param items The ranges to create a tree from. They must be sorted in document order. * @returns The root nodes of the tree. */ export function createRangeTree( From e48da9938b0e97c9614da1345aa4993e9f8a3866 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 15:53:40 +0100 Subject: [PATCH 17/32] Rename lookup --- .../{RangeIterator.ts => RangeLookupList.ts} | 2 +- ...{createRangeTree.ts => RangeLookupTree.ts} | 21 ++++++++++++++++--- .../getDelimiterOccurrences.ts | 19 +++++++---------- 3 files changed, 27 insertions(+), 15 deletions(-) rename packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/{RangeIterator.ts => RangeLookupList.ts} (94%) rename packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/{createRangeTree.ts => RangeLookupTree.ts} (60%) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts similarity index 94% rename from packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts rename to packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts index 9f19bf8029..3497142cdd 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeIterator.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts @@ -4,7 +4,7 @@ import type { Range } from "@cursorless/common"; * An iterator that allows for efficient lookup of ranges that contain a search item. * The items must be sorted in document order. */ -export class RangeIterator { +export class RangeLookupList { private index = 0; /** diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts similarity index 60% rename from packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts rename to packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts index d42610c133..4bbf89043b 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/createRangeTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts @@ -1,14 +1,29 @@ import type { Range } from "@cursorless/common"; import { RangeTreeNode } from "./RangeTreeNode"; +import { RangeLookupList } from "./RangeLookupList"; + +export class RangeLookupTree { + private children: RangeLookupList>; + + constructor(items: T[]) { + this.children = createNodes(items); + } + + getSmallLestContaining(separator: Range): T | undefined { + return this.children + .getContaining(separator) + ?.getSmallLestContaining(separator); + } +} /** * Creates a tree of ranges from a list of ranges. This improves containing lookup time. * @param items The ranges to create a tree from. They must be sorted in document order. * @returns The root nodes of the tree. */ -export function createRangeTree( +function createNodes( items: T[], -): RangeTreeNode[] { +): RangeLookupList> { const results: RangeTreeNode[] = []; const parents: RangeTreeNode[] = []; @@ -33,5 +48,5 @@ export function createRangeTree( parents.push(node); } - return results; + return new RangeLookupList(results); } 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 c9229f1bb2..5a25135444 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -6,9 +6,9 @@ import { } from "@cursorless/common"; import type { LanguageDefinition } from "../../../../languages/LanguageDefinition"; import type { QueryCapture } from "../../../../languages/TreeSitterQuery/QueryCapture"; -import { createRangeTree } from "./createRangeTree"; import { getDelimiterRegex } from "./getDelimiterRegex"; -import { RangeIterator } from "./RangeIterator"; +import { RangeLookupList } from "./RangeLookupList"; +import { RangeLookupTree } from "./RangeLookupTree"; import type { DelimiterOccurrence, IndividualDelimiter } from "./types"; /** @@ -28,14 +28,12 @@ export function getDelimiterOccurrences( return []; } - const disqualifyDelimiters = new RangeIterator( + const disqualifyDelimiters = new RangeLookupList( getSortedCaptures(languageDefinition, document, "disqualifyDelimiter"), ); - const textFragments = new RangeIterator( - // We need to create a tree for text fragments since they can be nested - createRangeTree( - getSortedCaptures(languageDefinition, document, "textFragment"), - ), + // We need a tree for text fragments since they can be nested + const textFragments = new RangeLookupTree( + getSortedCaptures(languageDefinition, document, "textFragment"), ); const delimiterTextToDelimiterInfoMap = Object.fromEntries( @@ -63,9 +61,8 @@ export function getDelimiterOccurrences( const isDisqualified = delimiter != null && !delimiter.hasError(); if (!isDisqualified) { - const textFragmentRange = textFragments - .getContaining(range) - ?.getSmallLestContaining(range).range; + const textFragmentRange = + textFragments.getSmallLestContaining(range)?.range; results.push({ delimiterInfo: delimiterTextToDelimiterInfoMap[text], textFragmentRange, From 47dbf977f1cdb5ff53bb8dfb8d0bf9d6978444d0 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 15:59:52 +0100 Subject: [PATCH 18/32] Use list in tree --- .../RangeLookupTree.ts | 2 +- .../RangeTreeNode.ts | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts index 4bbf89043b..c6db9afc83 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts @@ -40,7 +40,7 @@ function createNodes( const parent = parents[parents.length - 1]; if (parent != null) { - parent.children.push(node); + parent.children.items.push(node); } else { results.push(node); } diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts index 2220dea90b..855ff33a73 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts @@ -1,19 +1,22 @@ import type { Range } from "@cursorless/common"; +import { RangeLookupList } from "./RangeLookupList"; export class RangeTreeNode { - children: RangeTreeNode[] = []; + children: RangeLookupList>; - constructor(private item: T) {} + constructor(private item: T) { + this.children = new RangeLookupList([]); + } get range(): Range { return this.item.range; } - getSmallLestContaining(separator: Range): T { - for (const child of this.children) { - if (child.item.range.contains(separator)) { - return child.getSmallLestContaining(separator); - } + getSmallLestContaining(range: Range): T { + const child = this.children.getContaining(range); + + if (child != null) { + return child.getSmallLestContaining(range); } return this.item; From e0dbc9f1431b2a1b52839bdd27f843a7129951d9 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 16:01:49 +0100 Subject: [PATCH 19/32] Update comments --- .../SurroundingPairScopeHandler/RangeLookupList.ts | 2 +- .../SurroundingPairScopeHandler/RangeLookupTree.ts | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts index 3497142cdd..16016904bb 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts @@ -8,7 +8,7 @@ export class RangeLookupList { private index = 0; /** - * @param items The items to iterate over. Must be sorted in document order. + * @param items The items to search in. Must be sorted in document order. */ constructor(public items: T[]) {} diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts index c6db9afc83..fdd44a1bd2 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts @@ -2,9 +2,16 @@ import type { Range } from "@cursorless/common"; import { RangeTreeNode } from "./RangeTreeNode"; import { RangeLookupList } from "./RangeLookupList"; +/** + * A tree of ranges that allows for efficient lookup of ranges that contain a search item. + * The items must be sorted in document order. + */ export class RangeLookupTree { private children: RangeLookupList>; + /** + * @param items The items to search in. Must be sorted in document order. + */ constructor(items: T[]) { this.children = createNodes(items); } @@ -16,11 +23,6 @@ export class RangeLookupTree { } } -/** - * Creates a tree of ranges from a list of ranges. This improves containing lookup time. - * @param items The ranges to create a tree from. They must be sorted in document order. - * @returns The root nodes of the tree. - */ function createNodes( items: T[], ): RangeLookupList> { From 694a05026dfefa1619b65a275c3f4d9ce8913888 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 16:03:49 +0100 Subject: [PATCH 20/32] clean up --- .../SurroundingPairScopeHandler/RangeTreeNode.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts index 855ff33a73..5f15491dae 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts @@ -13,12 +13,10 @@ export class RangeTreeNode { } getSmallLestContaining(range: Range): T { - const child = this.children.getContaining(range); + const child = this.children + .getContaining(range) + ?.getSmallLestContaining(range); - if (child != null) { - return child.getSmallLestContaining(range); - } - - return this.item; + return child ?? this.item; } } From 640c7745ce3f5c4922285ad775e5450fc4a4bb8c Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 16:11:15 +0100 Subject: [PATCH 21/32] More refactoring --- .../SurroundingPairScopeHandler/RangeLookupList.ts | 6 +++++- .../SurroundingPairScopeHandler/RangeLookupTree.ts | 2 +- .../SurroundingPairScopeHandler/RangeTreeNode.ts | 6 +++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts index 16016904bb..48973c07ac 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts @@ -10,7 +10,11 @@ export class RangeLookupList { /** * @param items The items to search in. Must be sorted in document order. */ - constructor(public items: T[]) {} + constructor(private items: T[]) {} + + add(item: T) { + this.items.push(item); + } contains(searchItem: Range): boolean { return this.advance(searchItem); diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts index fdd44a1bd2..86eea47375 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts @@ -42,7 +42,7 @@ function createNodes( const parent = parents[parents.length - 1]; if (parent != null) { - parent.children.items.push(node); + parent.addChildNode(node); } else { results.push(node); } diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts index 5f15491dae..a703f99841 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts @@ -2,7 +2,7 @@ import type { Range } from "@cursorless/common"; import { RangeLookupList } from "./RangeLookupList"; export class RangeTreeNode { - children: RangeLookupList>; + private children: RangeLookupList>; constructor(private item: T) { this.children = new RangeLookupList([]); @@ -12,6 +12,10 @@ export class RangeTreeNode { return this.item.range; } + addChildNode(node: RangeTreeNode) { + this.children.add(node); + } + getSmallLestContaining(range: Range): T { const child = this.children .getContaining(range) From 57aff0e5575ce5fa11525f08ec0c1a47999691d5 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 16:16:50 +0100 Subject: [PATCH 22/32] Refactoring --- .../RangeLookupTree.ts | 35 +++++++++++++++---- .../RangeTreeNode.ts | 26 -------------- 2 files changed, 29 insertions(+), 32 deletions(-) delete mode 100644 packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts index 86eea47375..d9173ad5d6 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts @@ -1,5 +1,4 @@ import type { Range } from "@cursorless/common"; -import { RangeTreeNode } from "./RangeTreeNode"; import { RangeLookupList } from "./RangeLookupList"; /** @@ -7,7 +6,7 @@ import { RangeLookupList } from "./RangeLookupList"; * The items must be sorted in document order. */ export class RangeLookupTree { - private children: RangeLookupList>; + private children: RangeLookupList>; /** * @param items The items to search in. Must be sorted in document order. @@ -25,12 +24,12 @@ export class RangeLookupTree { function createNodes( items: T[], -): RangeLookupList> { - const results: RangeTreeNode[] = []; - const parents: RangeTreeNode[] = []; +): RangeLookupList> { + const results: RangeLookupTreeNode[] = []; + const parents: RangeLookupTreeNode[] = []; for (const item of items) { - const node = new RangeTreeNode(item); + const node = new RangeLookupTreeNode(item); while ( parents.length > 0 && @@ -52,3 +51,27 @@ function createNodes( return new RangeLookupList(results); } + +class RangeLookupTreeNode { + private children: RangeLookupList>; + + constructor(private item: T) { + this.children = new RangeLookupList([]); + } + + get range(): Range { + return this.item.range; + } + + addChildNode(node: RangeLookupTreeNode) { + this.children.add(node); + } + + getSmallLestContaining(range: Range): T { + const child = this.children + .getContaining(range) + ?.getSmallLestContaining(range); + + return child ?? this.item; + } +} diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts deleted file mode 100644 index a703f99841..0000000000 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeTreeNode.ts +++ /dev/null @@ -1,26 +0,0 @@ -import type { Range } from "@cursorless/common"; -import { RangeLookupList } from "./RangeLookupList"; - -export class RangeTreeNode { - private children: RangeLookupList>; - - constructor(private item: T) { - this.children = new RangeLookupList([]); - } - - get range(): Range { - return this.item.range; - } - - addChildNode(node: RangeTreeNode) { - this.children.add(node); - } - - getSmallLestContaining(range: Range): T { - const child = this.children - .getContaining(range) - ?.getSmallLestContaining(range); - - return child ?? this.item; - } -} From 40621eabf501a3c2247228278b84ded67c9a0beb Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 26 Dec 2024 17:57:29 +0100 Subject: [PATCH 23/32] clean up --- .../SurroundingPairScopeHandler/RangeLookupTree.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts index d9173ad5d6..ee05dfb537 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts @@ -41,7 +41,7 @@ function createNodes( const parent = parents[parents.length - 1]; if (parent != null) { - parent.addChildNode(node); + parent.children.add(node); } else { results.push(node); } @@ -53,7 +53,7 @@ function createNodes( } class RangeLookupTreeNode { - private children: RangeLookupList>; + public children: RangeLookupList>; constructor(private item: T) { this.children = new RangeLookupList([]); @@ -63,10 +63,6 @@ class RangeLookupTreeNode { return this.item.range; } - addChildNode(node: RangeLookupTreeNode) { - this.children.add(node); - } - getSmallLestContaining(range: Range): T { const child = this.children .getContaining(range) From 195f94c6ec896352849eaf352bc743d74fe3ada8 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 7 Jan 2025 15:24:45 +0100 Subject: [PATCH 24/32] Add empty line in ExecuteCommand (#2702) ## Checklist - [/] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [/] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [/] I have not broken the cheatsheet --- packages/cursorless-engine/src/actions/ExecuteCommand.ts | 1 + 1 file changed, 1 insertion(+) 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); From 46aa82e8bac195593647976223e5073d22ac3ff0 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 7 Jan 2025 15:36:59 +0100 Subject: [PATCH 25/32] Update packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com> --- .../SurroundingPairScopeHandler/RangeLookupList.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts index 48973c07ac..c0b9b6e188 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts @@ -1,10 +1,13 @@ import type { Range } from "@cursorless/common"; /** - * An iterator that allows for efficient lookup of ranges that contain a search item. - * The items must be sorted in document order. + * 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 RangeLookupList { +export class OneWayRangeFinder { private index = 0; /** From 5ecf4a1a6a26a3f81c2f94c2898e5beba07637dc Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 7 Jan 2025 15:39:57 +0100 Subject: [PATCH 26/32] Merge --- .../src/languages/LanguageDefinition.ts | 2 +- .../src/languages/LanguageDefinitionCache.ts | 6 ------ .../{RangeLookupList.ts => OneWayRangeFinder.ts} | 0 .../SurroundingPairScopeHandler/RangeLookupTree.ts | 12 ++++++------ .../getDelimiterOccurrences.ts | 4 ++-- 5 files changed, 9 insertions(+), 15 deletions(-) rename packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/{RangeLookupList.ts => OneWayRangeFinder.ts} (100%) diff --git a/packages/cursorless-engine/src/languages/LanguageDefinition.ts b/packages/cursorless-engine/src/languages/LanguageDefinition.ts index a217fc54c8..3cd43d1653 100644 --- a/packages/cursorless-engine/src/languages/LanguageDefinition.ts +++ b/packages/cursorless-engine/src/languages/LanguageDefinition.ts @@ -107,7 +107,7 @@ export class LanguageDefinition { } clearCache(): void { - this.cache.clear(); + this.cache = new LanguageDefinitionCache(); } /** diff --git a/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts b/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts index d618ebf809..16d7562361 100644 --- a/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts +++ b/packages/cursorless-engine/src/languages/LanguageDefinitionCache.ts @@ -10,12 +10,6 @@ export class LanguageDefinitionCache { private documentVersion: number = -1; private captures: StringRecord = {}; - clear() { - this.documentUri = ""; - this.documentVersion = -1; - this.captures = {}; - } - isValid(document: TextDocument) { return ( this.documentUri === document.uri.toString() && diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayRangeFinder.ts similarity index 100% rename from packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupList.ts rename to packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayRangeFinder.ts diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts index ee05dfb537..c4c845e26d 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts @@ -1,12 +1,12 @@ import type { Range } from "@cursorless/common"; -import { RangeLookupList } from "./RangeLookupList"; +import { OneWayRangeFinder } from "./OneWayRangeFinder"; /** * A tree of ranges that allows for efficient lookup of ranges that contain a search item. * The items must be sorted in document order. */ export class RangeLookupTree { - private children: RangeLookupList>; + private children: OneWayRangeFinder>; /** * @param items The items to search in. Must be sorted in document order. @@ -24,7 +24,7 @@ export class RangeLookupTree { function createNodes( items: T[], -): RangeLookupList> { +): OneWayRangeFinder> { const results: RangeLookupTreeNode[] = []; const parents: RangeLookupTreeNode[] = []; @@ -49,14 +49,14 @@ function createNodes( parents.push(node); } - return new RangeLookupList(results); + return new OneWayRangeFinder(results); } class RangeLookupTreeNode { - public children: RangeLookupList>; + public children: OneWayRangeFinder>; constructor(private item: T) { - this.children = new RangeLookupList([]); + this.children = new OneWayRangeFinder([]); } get range(): Range { 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 5a25135444..9fffe09601 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -7,7 +7,7 @@ import { import type { LanguageDefinition } from "../../../../languages/LanguageDefinition"; import type { QueryCapture } from "../../../../languages/TreeSitterQuery/QueryCapture"; import { getDelimiterRegex } from "./getDelimiterRegex"; -import { RangeLookupList } from "./RangeLookupList"; +import { OneWayRangeFinder } from "./OneWayRangeFinder"; import { RangeLookupTree } from "./RangeLookupTree"; import type { DelimiterOccurrence, IndividualDelimiter } from "./types"; @@ -28,7 +28,7 @@ export function getDelimiterOccurrences( return []; } - const disqualifyDelimiters = new RangeLookupList( + const disqualifyDelimiters = new OneWayRangeFinder( getSortedCaptures(languageDefinition, document, "disqualifyDelimiter"), ); // We need a tree for text fragments since they can be nested From 4fefbe21f4188f290bc644a130b050dc687339d8 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 7 Jan 2025 15:41:23 +0100 Subject: [PATCH 27/32] Update packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com> --- .../SurroundingPairScopeHandler/RangeLookupTree.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts index c4c845e26d..cc2d59ba8c 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts @@ -15,10 +15,10 @@ export class RangeLookupTree { this.children = createNodes(items); } - getSmallLestContaining(separator: Range): T | undefined { + getSmallestContaining(separator: Range): T | undefined { return this.children .getContaining(separator) - ?.getSmallLestContaining(separator); + ?.getSmallestContaining(separator); } } From 63924c226a1ac44307892ff7b32c93c7635ca7e7 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 7 Jan 2025 15:42:54 +0100 Subject: [PATCH 28/32] typo --- .../SurroundingPairScopeHandler/RangeLookupTree.ts | 8 ++++---- .../getDelimiterOccurrences.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts index c4c845e26d..f10bbe52b4 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts @@ -15,10 +15,10 @@ export class RangeLookupTree { this.children = createNodes(items); } - getSmallLestContaining(separator: Range): T | undefined { + getSmallestContaining(separator: Range): T | undefined { return this.children .getContaining(separator) - ?.getSmallLestContaining(separator); + ?.getSmallestContaining(separator); } } @@ -63,10 +63,10 @@ class RangeLookupTreeNode { return this.item.range; } - getSmallLestContaining(range: Range): T { + getSmallestContaining(range: Range): T { const child = this.children .getContaining(range) - ?.getSmallLestContaining(range); + ?.getSmallestContaining(range); return child ?? this.item; } 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 9fffe09601..2a56b7f298 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -62,7 +62,7 @@ export function getDelimiterOccurrences( if (!isDisqualified) { const textFragmentRange = - textFragments.getSmallLestContaining(range)?.range; + textFragments.getSmallestContaining(range)?.range; results.push({ delimiterInfo: delimiterTextToDelimiterInfoMap[text], textFragmentRange, From ae2804bb3c10e4cbdf1fe2bf0969b3b1e377aad1 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 7 Jan 2025 15:44:07 +0100 Subject: [PATCH 29/32] Update packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com> --- .../SurroundingPairScopeHandler/RangeLookupTree.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts index f10bbe52b4..d001dcf264 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts @@ -2,10 +2,13 @@ import type { Range } from "@cursorless/common"; import { OneWayRangeFinder } from "./OneWayRangeFinder"; /** - * A tree of ranges that allows for efficient lookup of ranges that contain a search item. - * The items must be sorted in document order. + * 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 RangeLookupTree { +export class OneWayNestedRangeFinder { private children: OneWayRangeFinder>; /** From 1a46aa8fbfb56bacc61ec720a39c10f69d946535 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 7 Jan 2025 15:45:04 +0100 Subject: [PATCH 30/32] Rename --- .../{RangeLookupTree.ts => OneWayNestedRangeFinder.ts} | 0 .../SurroundingPairScopeHandler/getDelimiterOccurrences.ts | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) rename packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/{RangeLookupTree.ts => OneWayNestedRangeFinder.ts} (100%) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayNestedRangeFinder.ts similarity index 100% rename from packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/RangeLookupTree.ts rename to packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayNestedRangeFinder.ts 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 2a56b7f298..7f01224043 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -8,7 +8,7 @@ import type { LanguageDefinition } from "../../../../languages/LanguageDefinitio import type { QueryCapture } from "../../../../languages/TreeSitterQuery/QueryCapture"; import { getDelimiterRegex } from "./getDelimiterRegex"; import { OneWayRangeFinder } from "./OneWayRangeFinder"; -import { RangeLookupTree } from "./RangeLookupTree"; +import { OneWayNestedRangeFinder } from "./OneWayNestedRangeFinder"; import type { DelimiterOccurrence, IndividualDelimiter } from "./types"; /** @@ -32,7 +32,7 @@ export function getDelimiterOccurrences( getSortedCaptures(languageDefinition, document, "disqualifyDelimiter"), ); // We need a tree for text fragments since they can be nested - const textFragments = new RangeLookupTree( + const textFragments = new OneWayNestedRangeFinder( getSortedCaptures(languageDefinition, document, "textFragment"), ); From 174760f4609dde87f40349c0890439ee1d39c14c Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 7 Jan 2025 16:17:25 +0100 Subject: [PATCH 31/32] Added tests --- .../OneWayNestedRangeFinder.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/OneWayNestedRangeFinder.test.ts 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()); + }); +}); From c4873bbe44d89bd80a3294bedc0bee5233cc7080 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 7 Jan 2025 16:22:40 +0100 Subject: [PATCH 32/32] Added comment --- .../cursorless-engine/src/languages/LanguageDefinitions.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/cursorless-engine/src/languages/LanguageDefinitions.ts b/packages/cursorless-engine/src/languages/LanguageDefinitions.ts index 55bcd13e7b..e241383ebd 100644 --- a/packages/cursorless-engine/src/languages/LanguageDefinitions.ts +++ b/packages/cursorless-engine/src/languages/LanguageDefinitions.ts @@ -34,6 +34,12 @@ export interface LanguageDefinitions { /** * 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;