From 603d6132b6bd091695b82ffdec213cfb52e87068 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Sat, 25 Jan 2025 05:20:25 +0100 Subject: [PATCH 1/2] Prevent overlapping pairs --- .../parseTreeParity/takeOutside13.yml | 23 ------ .../parseTreeParity/takeOutside16.yml | 23 ------ .../parseTreeParity/takeOutside25.yml | 23 ------ .../surroundingPair/textual/takeOutside13.yml | 23 ------ .../surroundingPair/textual/takeOutside16.yml | 23 ------ .../surroundingPair/textual/takeOutside25.yml | 23 ------ .../scopes/textual/surroundingPair4.scope | 44 ++++++++++ .../scopes/textual/surroundingPair5.scope | 38 +++++++++ .../scopes/textual/surroundingPair6.scope | 22 +++++ .../getSurroundingPairOccurrences.ts | 82 ++++++++----------- 10 files changed, 139 insertions(+), 185 deletions(-) delete mode 100644 data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside13.yml delete mode 100644 data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside16.yml delete mode 100644 data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside25.yml delete mode 100644 data/fixtures/recorded/surroundingPair/textual/takeOutside13.yml delete mode 100644 data/fixtures/recorded/surroundingPair/textual/takeOutside16.yml delete mode 100644 data/fixtures/recorded/surroundingPair/textual/takeOutside25.yml create mode 100644 data/fixtures/scopes/textual/surroundingPair4.scope create mode 100644 data/fixtures/scopes/textual/surroundingPair5.scope create mode 100644 data/fixtures/scopes/textual/surroundingPair6.scope diff --git a/data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside13.yml b/data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside13.yml deleted file mode 100644 index 538c341d66..0000000000 --- a/data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside13.yml +++ /dev/null @@ -1,23 +0,0 @@ -languageId: typescript -command: - version: 6 - spokenForm: change pair - action: - name: clearAndSetSelection - target: - type: primitive - modifiers: - - type: containingScope - scopeType: {type: surroundingPair, delimiter: any} - usePrePhraseSnapshot: false -initialState: - documentContents: ( [ ) ] - selections: - - anchor: {line: 0, character: 3} - active: {line: 0, character: 3} - marks: {} -finalState: - documentContents: "( " - selections: - - anchor: {line: 0, character: 3} - active: {line: 0, character: 3} diff --git a/data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside16.yml b/data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside16.yml deleted file mode 100644 index 34506e5999..0000000000 --- a/data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside16.yml +++ /dev/null @@ -1,23 +0,0 @@ -languageId: typescript -command: - version: 6 - spokenForm: change pair - action: - name: clearAndSetSelection - target: - type: primitive - modifiers: - - type: containingScope - scopeType: {type: surroundingPair, delimiter: any} - usePrePhraseSnapshot: false -initialState: - documentContents: ( [ ) ] - selections: - - anchor: {line: 0, character: 8} - active: {line: 0, character: 8} - marks: {} -finalState: - documentContents: "( " - selections: - - anchor: {line: 0, character: 3} - active: {line: 0, character: 3} diff --git a/data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside25.yml b/data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside25.yml deleted file mode 100644 index 5f6fa046f2..0000000000 --- a/data/fixtures/recorded/surroundingPair/parseTreeParity/takeOutside25.yml +++ /dev/null @@ -1,23 +0,0 @@ -languageId: typescript -command: - version: 6 - spokenForm: change pair - action: - name: clearAndSetSelection - target: - type: primitive - modifiers: - - type: containingScope - scopeType: {type: surroundingPair, delimiter: any} - usePrePhraseSnapshot: false -initialState: - documentContents: ([)] - 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/data/fixtures/recorded/surroundingPair/textual/takeOutside13.yml b/data/fixtures/recorded/surroundingPair/textual/takeOutside13.yml deleted file mode 100644 index a65d873177..0000000000 --- a/data/fixtures/recorded/surroundingPair/textual/takeOutside13.yml +++ /dev/null @@ -1,23 +0,0 @@ -languageId: plaintext -command: - version: 6 - spokenForm: change pair - action: - name: clearAndSetSelection - target: - type: primitive - modifiers: - - type: containingScope - scopeType: {type: surroundingPair, delimiter: any} - usePrePhraseSnapshot: false -initialState: - documentContents: ( [ ) ] - selections: - - anchor: {line: 0, character: 3} - active: {line: 0, character: 3} - marks: {} -finalState: - documentContents: "( " - selections: - - anchor: {line: 0, character: 3} - active: {line: 0, character: 3} diff --git a/data/fixtures/recorded/surroundingPair/textual/takeOutside16.yml b/data/fixtures/recorded/surroundingPair/textual/takeOutside16.yml deleted file mode 100644 index 90de980545..0000000000 --- a/data/fixtures/recorded/surroundingPair/textual/takeOutside16.yml +++ /dev/null @@ -1,23 +0,0 @@ -languageId: plaintext -command: - version: 6 - spokenForm: change pair - action: - name: clearAndSetSelection - target: - type: primitive - modifiers: - - type: containingScope - scopeType: {type: surroundingPair, delimiter: any} - usePrePhraseSnapshot: false -initialState: - documentContents: ( [ ) ] - selections: - - anchor: {line: 0, character: 8} - active: {line: 0, character: 8} - marks: {} -finalState: - documentContents: "( " - selections: - - anchor: {line: 0, character: 3} - active: {line: 0, character: 3} diff --git a/data/fixtures/recorded/surroundingPair/textual/takeOutside25.yml b/data/fixtures/recorded/surroundingPair/textual/takeOutside25.yml deleted file mode 100644 index 591aa1a697..0000000000 --- a/data/fixtures/recorded/surroundingPair/textual/takeOutside25.yml +++ /dev/null @@ -1,23 +0,0 @@ -languageId: plaintext -command: - version: 6 - spokenForm: change pair - action: - name: clearAndSetSelection - target: - type: primitive - modifiers: - - type: containingScope - scopeType: {type: surroundingPair, delimiter: any} - usePrePhraseSnapshot: false -initialState: - documentContents: ([)] - 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/data/fixtures/scopes/textual/surroundingPair4.scope b/data/fixtures/scopes/textual/surroundingPair4.scope new file mode 100644 index 0000000000..cc17bcceab --- /dev/null +++ b/data/fixtures/scopes/textual/surroundingPair4.scope @@ -0,0 +1,44 @@ +(a < b) +(c > d) +--- + +[#1 Content] = +[#1 Removal] = +[#1 Domain] = 0:0-0:7 + >-------< +0| (a < b) + +[#1 Interior] = 0:1-0:6 + >-----< +0| (a < b) + +[#1 Boundary L] = 0:0-0:1 + >-< +0| (a < b) + +[#1 Boundary R] = 0:6-0:7 + >-< +0| (a < b) + +[#1 Insertion delimiter] = " " + + +[#2 Content] = +[#2 Removal] = +[#2 Domain] = 1:0-1:7 + >-------< +1| (c > d) + +[#2 Interior] = 1:1-1:6 + >-----< +1| (c > d) + +[#2 Boundary L] = 1:0-1:1 + >-< +1| (c > d) + +[#2 Boundary R] = 1:6-1:7 + >-< +1| (c > d) + +[#2 Insertion delimiter] = " " diff --git a/data/fixtures/scopes/textual/surroundingPair5.scope b/data/fixtures/scopes/textual/surroundingPair5.scope new file mode 100644 index 0000000000..155d2c9269 --- /dev/null +++ b/data/fixtures/scopes/textual/surroundingPair5.scope @@ -0,0 +1,38 @@ +( [ ) ] +--- + +[Content] = +[Domain] = 0:0-0:7 + >-------< +0| ( [ ) ] + +[Removal] = 0:0-0:9 + >---------< +0| ( [ ) ] + +[Trailing delimiter] = 0:7-0:9 + >--< +0| ( [ ) ] + +[Interior: Content] = 0:3-0:4 + >-< +0| ( [ ) ] +[Interior: Removal] = 0:1-0:6 + >-----< +0| ( [ ) ] + +[Boundary L: Content] = 0:0-0:1 + >-< +0| ( [ ) ] +[Boundary L: Removal] = 0:0-0:3 + >---< +0| ( [ ) ] + +[Boundary R: Content] = 0:6-0:7 + >-< +0| ( [ ) ] +[Boundary R: Removal] = 0:6-0:9 + >---< +0| ( [ ) ] + +[Insertion delimiter] = " " diff --git a/data/fixtures/scopes/textual/surroundingPair6.scope b/data/fixtures/scopes/textual/surroundingPair6.scope new file mode 100644 index 0000000000..e98d23e5a1 --- /dev/null +++ b/data/fixtures/scopes/textual/surroundingPair6.scope @@ -0,0 +1,22 @@ +([)] +--- + +[Content] = +[Removal] = +[Domain] = 0:0-0:3 + >---< +0| ([)] + +[Interior] = 0:1-0:2 + >-< +0| ([)] + +[Boundary L] = 0:0-0:1 + >-< +0| ([)] + +[Boundary R] = 0:2-0:3 + >-< +0| ([)] + +[Insertion delimiter] = " " 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 027476ab7a..0c0c2b8d42 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts @@ -1,5 +1,5 @@ -import type { SimpleSurroundingPairName } from "@cursorless/common"; -import { DefaultMap } from "@cursorless/common"; +import type { Range } from "@cursorless/common"; +import findLastIndex from "lodash-es/findLastIndex"; import type { DelimiterOccurrence, SurroundingPairOccurrence } from "./types"; /** @@ -13,14 +13,7 @@ export function getSurroundingPairOccurrences( delimiterOccurrences: DelimiterOccurrence[], ): SurroundingPairOccurrence[] { const result: SurroundingPairOccurrence[] = []; - - /** - * A map from delimiter names to occurrences of the opening delimiter - */ - const openingDelimiterOccurrences = new DefaultMap< - SimpleSurroundingPairName, - DelimiterOccurrence[] - >(() => []); + const openingDelimitersStack: DelimiterOccurrence[] = []; for (const occurrence of delimiterOccurrences) { const { @@ -29,48 +22,29 @@ export function getSurroundingPairOccurrences( range, } = occurrence; - let openingDelimiters = openingDelimiterOccurrences.get(delimiterName); - - if (isSingleLine) { - // If single line, remove all opening delimiters that are not on the same line - // as occurrence - openingDelimiters = openingDelimiters.filter( - (openingDelimiter) => - openingDelimiter.range.start.line === range.start.line, - ); - openingDelimiterOccurrences.set(delimiterName, openingDelimiters); - } - - /** - * A list of opening delimiters that are relevant to the current occurrence. - * We exclude delimiters that are not in the same text fragment range as the - * current occurrence. - */ - const relevantOpeningDelimiters = openingDelimiters.filter( - (openingDelimiter) => - (textFragmentRange == null && - openingDelimiter.textFragmentRange == null) || - (textFragmentRange != null && - openingDelimiter.textFragmentRange != null && - openingDelimiter.textFragmentRange.isRangeEqual(textFragmentRange)), - ); - - if ( - side === "left" || - (side === "unknown" && relevantOpeningDelimiters.length % 2 === 0) - ) { - openingDelimiters.push(occurrence); + if (side === "left") { + openingDelimitersStack.push(occurrence); } else { - const openingDelimiter = relevantOpeningDelimiters.at(-1); + const openingDelimiterIndex = findLastIndex( + openingDelimitersStack, + (o) => + o.delimiterInfo.delimiterName === delimiterName && + isSameTextFragment(o.textFragmentRange, textFragmentRange) && + isValidLine(isSingleLine, o.range, range), + ); - if (openingDelimiter == null) { + 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); + } continue; } - openingDelimiters.splice( - openingDelimiters.lastIndexOf(openingDelimiter), - 1, - ); + const openingDelimiter = openingDelimitersStack[openingDelimiterIndex]; + + // Pop stack up to and including the opening delimiter + openingDelimitersStack.length = openingDelimiterIndex; result.push({ delimiterName: delimiterName, @@ -82,3 +56,17 @@ export function getSurroundingPairOccurrences( return result; } + +function isSameTextFragment( + a: Range | undefined, + b: Range | undefined, +): boolean { + if (a == null || b == null) { + return a === b; + } + return a.isRangeEqual(b); +} + +function isValidLine(isSingleLine: boolean, a: Range, b: Range): boolean { + return !isSingleLine || a.start.line === b.start.line; +} From 163f3ea56e1f32b5295d1c5946bb9b3149e5647c Mon Sep 17 00:00:00 2001 From: Phil Cohen Date: Sat, 25 Jan 2025 10:37:07 -0800 Subject: [PATCH 2/2] Update packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts --- .../getSurroundingPairOccurrences.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0c0c2b8d42..7f33695a9c 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getSurroundingPairOccurrences.ts @@ -34,7 +34,7 @@ export function getSurroundingPairOccurrences( ); if (openingDelimiterIndex === -1) { - // When side is unknown and we can't find an opening delimiter, that means this is the opening delimiter. + // 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); }