diff --git a/data/fixtures/recorded/surroundingPair/parseTree/ruby/changePair2.yml b/data/fixtures/recorded/surroundingPair/parseTree/ruby/changePair2.yml new file mode 100644 index 0000000000..da9708eab0 --- /dev/null +++ b/data/fixtures/recorded/surroundingPair/parseTree/ruby/changePair2.yml @@ -0,0 +1,23 @@ +languageId: ruby +command: + version: 7 + spokenForm: change pair + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: containingScope + scopeType: {type: surroundingPair, delimiter: any} + usePrePhraseSnapshot: false +initialState: + documentContents: "%Q(hello)" + selections: + - anchor: {line: 0, character: 3} + active: {line: 0, character: 3} + marks: {} +finalState: + documentContents: "" + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} diff --git a/data/fixtures/recorded/surroundingPair/parseTree/ruby/changePair3.yml b/data/fixtures/recorded/surroundingPair/parseTree/ruby/changePair3.yml new file mode 100644 index 0000000000..71519bff01 --- /dev/null +++ b/data/fixtures/recorded/surroundingPair/parseTree/ruby/changePair3.yml @@ -0,0 +1,23 @@ +languageId: ruby +command: + version: 7 + spokenForm: change pair + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: containingScope + scopeType: {type: surroundingPair, delimiter: any} + usePrePhraseSnapshot: false +initialState: + documentContents: foo(%Q(hello)) + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 4} + marks: {} +finalState: + documentContents: foo() + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 4} 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 80e8a4a1af..1ec74624e6 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts @@ -34,12 +34,12 @@ export function getDelimiterOccurrences( getSortedCaptures(capturesMap.textFragment), ); - const delimiterTextToDelimiterInfoMap = Object.fromEntries( - individualDelimiters.map((individualDelimiter) => [ - individualDelimiter.text, - individualDelimiter, - ]), - ); + const delimiterTextToDelimiterInfoMap = individualDelimiters.reduce< + Record + >((acc, individualDelimiter) => { + (acc[individualDelimiter.text] ??= []).push(individualDelimiter); + return acc; + }, {}); const regexMatches = matchAllIterator( document.getText(), @@ -64,7 +64,7 @@ export function getDelimiterOccurrences( } results.push({ - delimiterInfo: delimiterTextToDelimiterInfoMap[text], + delimiterInfos: delimiterTextToDelimiterInfoMap[text], textFragmentRange: textFragments.getSmallestContaining(matchRange)?.range, range: ifNoErrors(pairDelimiters.getContaining(matchRange))?.range ?? 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 7f33695a9c..0a031d0421 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts @@ -1,6 +1,21 @@ import type { Range } from "@cursorless/common"; import findLastIndex from "lodash-es/findLastIndex"; -import type { DelimiterOccurrence, SurroundingPairOccurrence } from "./types"; +import type { + DelimiterOccurrence, + IndividualDelimiter, + SurroundingPairOccurrence, +} from "./types"; + +interface OpeningDelimiterStackOccurrence { + delimiterInfo: IndividualDelimiter; + range: Range; + textFragmentRange: Range | undefined; +} + +interface OpeningDelimiterMatch { + delimiterInfo: IndividualDelimiter; + openingDelimiterIndex: number; +} /** * Given a list of occurrences of delimiters, returns a list of occurrences of @@ -13,50 +28,91 @@ export function getSurroundingPairOccurrences( delimiterOccurrences: DelimiterOccurrence[], ): SurroundingPairOccurrence[] { const result: SurroundingPairOccurrence[] = []; - const openingDelimitersStack: DelimiterOccurrence[] = []; + const openingDelimitersStack: OpeningDelimiterStackOccurrence[] = []; for (const occurrence of delimiterOccurrences) { - const { - delimiterInfo: { delimiterName, side, isSingleLine }, - textFragmentRange, - range, - } = occurrence; - - if (side === "left") { - openingDelimitersStack.push(occurrence); - } else { - const openingDelimiterIndex = findLastIndex( - openingDelimitersStack, - (o) => - o.delimiterInfo.delimiterName === delimiterName && - isSameTextFragment(o.textFragmentRange, textFragmentRange) && - isValidLine(isSingleLine, o.range, range), + // One token can represent multiple delimiters (eg ")" could close + // `parentheses` or Ruby `%Q(`), so pick the best closing interpretation + // based on currently open delimiters. + const closestOpeningDelimiterMatch = getClosestOpeningDelimiterMatch( + occurrence, + openingDelimitersStack, + ); + + if (closestOpeningDelimiterMatch == null) { + const openingDelimiterInfo = occurrence.delimiterInfos.find( + ({ side }) => side === "left" || side === "unknown", ); - if (openingDelimiterIndex === -1) { - // When side is unknown and we can't find an opening delimiter, that means this *is* the opening delimiter. - if (side === "unknown") { - openingDelimitersStack.push(occurrence); - } + // Pure closing delimiters with no matching opener are ignored. + if (openingDelimiterInfo == null) { continue; } - const openingDelimiter = openingDelimitersStack[openingDelimiterIndex]; - - // Pop stack up to and including the opening delimiter - openingDelimitersStack.length = openingDelimiterIndex; - - result.push({ - delimiterName: delimiterName, - openingDelimiterRange: openingDelimiter.range, - closingDelimiterRange: range, + // If this token can't close anything, treat it as an opener. + openingDelimitersStack.push({ + delimiterInfo: openingDelimiterInfo, + range: occurrence.range, + textFragmentRange: occurrence.textFragmentRange, }); + continue; } + + const { delimiterInfo, openingDelimiterIndex } = + closestOpeningDelimiterMatch; + const openingDelimiter = openingDelimitersStack[openingDelimiterIndex]; + + // Pop stack up to and including the opening delimiter + openingDelimitersStack.length = openingDelimiterIndex; + + result.push({ + delimiterName: delimiterInfo.delimiterName, + openingDelimiterRange: openingDelimiter.range, + closingDelimiterRange: occurrence.range, + }); } return result; } +// When multiple interpretations are possible, choose the one whose opener is +// closest on the stack, which preserves normal nesting behavior. +function getClosestOpeningDelimiterMatch( + occurrence: DelimiterOccurrence, + openingDelimitersStack: OpeningDelimiterStackOccurrence[], +): OpeningDelimiterMatch | undefined { + let closestMatch: OpeningDelimiterMatch | undefined; + + for (const delimiterInfo of occurrence.delimiterInfos) { + if (delimiterInfo.side === "left") { + continue; + } + + const openingDelimiterIndex = findLastIndex( + openingDelimitersStack, + (o) => + o.delimiterInfo.delimiterName === delimiterInfo.delimiterName && + isSameTextFragment(o.textFragmentRange, occurrence.textFragmentRange) && + isValidLine(delimiterInfo.isSingleLine, o.range, occurrence.range), + ); + + // No opening delimiter found for this interpretation, so skip it + if (openingDelimiterIndex === -1) { + continue; + } + + // If this is the closest opening delimiter so far, remember it + if ( + closestMatch == null || + openingDelimiterIndex > closestMatch.openingDelimiterIndex + ) { + closestMatch = { delimiterInfo, openingDelimiterIndex }; + } + } + + return closestMatch; +} + function isSameTextFragment( a: Range | undefined, b: Range | undefined, 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 458ee23142..520e9630fc 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/types.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/types.ts @@ -38,9 +38,9 @@ export interface IndividualDelimiter { */ export interface DelimiterOccurrence { /** - * Information about the delimiter itself + * Possible delimiter interpretations for this text occurrence */ - delimiterInfo: IndividualDelimiter; + delimiterInfos: IndividualDelimiter[]; /** * The range of the delimiter in the document