Proper handling of surrounding pair delimiter prefixes (#2705)

AndreasArvidsson · pokey · phillco · web-flow · commit 0b9cdc3c2fec · 2025-01-10T18:13:15.000Z
On main `"take inside"` on `"command server"` will only select `command serve` because `r"` is considered a delimiter. This pull request fixes this. ## Checklist - [x] 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 --------- Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com> Co-authored-by: Phil Cohen <phillip@phillip.io>
diff --git a/cursorless-talon/src/spoken_forms.py b/cursorless-talon/src/spoken_forms.py
@@ -160,6 +160,7 @@ def handle_new_values(csv_name: str, values: list[SpokenFormEntry]):
                 "private.switchStatementSubject",
                 "textFragment",
                 "disqualifyDelimiter",
+                "pairDelimiter",
             ],
             default_list_name="scope_type",
         ),
diff --git a/data/fixtures/recorded/surroundingPair/parseTree/python/changeInside.yml b/data/fixtures/recorded/surroundingPair/parseTree/python/changeInside.yml
@@ -0,0 +1,22 @@
+languageId: python
+command:
+  version: 7
+  spokenForm: change inside
+  action:
+    name: clearAndSetSelection
+    target:
+      type: primitive
+      modifiers:
+        - {type: interiorOnly}
+  usePrePhraseSnapshot: true
+initialState:
+  documentContents: r'command server'
+  selections:
+    - anchor: {line: 0, character: 2}
+      active: {line: 0, character: 2}
+  marks: {}
+finalState:
+  documentContents: r''
+  selections:
+    - anchor: {line: 0, character: 2}
+      active: {line: 0, character: 2}
diff --git a/data/fixtures/recorded/surroundingPair/parseTree/python/changePair2.yml b/data/fixtures/recorded/surroundingPair/parseTree/python/changePair2.yml
@@ -11,7 +11,7 @@ command:
           scopeType: {type: surroundingPair, delimiter: any}
   usePrePhraseSnapshot: true
 initialState:
-  documentContents: "\" r\""
+  documentContents: "' r'"
   selections:
     - anchor: {line: 0, character: 0}
       active: {line: 0, character: 0}
diff --git a/data/fixtures/recorded/surroundingPair/parseTree/python/changePair3.yml b/data/fixtures/recorded/surroundingPair/parseTree/python/changePair3.yml
@@ -0,0 +1,23 @@
+languageId: python
+command:
+  version: 7
+  spokenForm: change pair
+  action:
+    name: clearAndSetSelection
+    target:
+      type: primitive
+      modifiers:
+        - type: containingScope
+          scopeType: {type: surroundingPair, delimiter: any}
+  usePrePhraseSnapshot: true
+initialState:
+  documentContents: r'command server'
+  selections:
+    - anchor: {line: 0, character: 2}
+      active: {line: 0, character: 2}
+  marks: {}
+finalState:
+  documentContents: ""
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
diff --git a/data/fixtures/scopes/python/pairDelimiter.scope b/data/fixtures/scopes/python/pairDelimiter.scope
@@ -0,0 +1,72 @@
+"server"
+'server'
+"""server"""
+'''server'''
+r" r"
+r' r'
+r""" r"""
+r''' r'''
+---
+
+[#1 Content] =
+[#1 Domain] = 4:0-4:2
+  >--<
+4| r" r"
+
+[#1 Removal] = 4:0-4:3
+  >---<
+4| r" r"
+
+[#1 Trailing delimiter] = 4:2-4:3
+    >-<
+4| r" r"
+
+[#1 Insertion delimiter] = " "
+
+
+[#2 Content] =
+[#2 Domain] = 5:0-5:2
+  >--<
+5| r' r'
+
+[#2 Removal] = 5:0-5:3
+  >---<
+5| r' r'
+
+[#2 Trailing delimiter] = 5:2-5:3
+    >-<
+5| r' r'
+
+[#2 Insertion delimiter] = " "
+
+
+[#3 Content] =
+[#3 Domain] = 6:0-6:4
+  >----<
+6| r""" r"""
+
+[#3 Removal] = 6:0-6:5
+  >-----<
+6| r""" r"""
+
+[#3 Trailing delimiter] = 6:4-6:5
+      >-<
+6| r""" r"""
+
+[#3 Insertion delimiter] = " "
+
+
+[#4 Content] =
+[#4 Domain] = 7:0-7:4
+  >----<
+7| r''' r'''
+
+[#4 Removal] = 7:0-7:5
+  >-----<
+7| r''' r'''
+
+[#4 Trailing delimiter] = 7:4-7:5
+      >-<
+7| r''' r'''
+
+[#4 Insertion delimiter] = " "
diff --git a/packages/common/src/scopeSupportFacets/python.ts b/packages/common/src/scopeSupportFacets/python.ts
@@ -14,6 +14,7 @@ export const pythonScopeSupport: LanguageScopeSupportFacetMap = {
   namedFunction: supported,
   anonymousFunction: supported,
   disqualifyDelimiter: supported,
+  pairDelimiter: supported,
 
   "argument.actual": supported,
   "argument.actual.iteration": supported,
diff --git a/packages/common/src/scopeSupportFacets/scopeSupportFacetInfos.ts b/packages/common/src/scopeSupportFacets/scopeSupportFacetInfos.ts
@@ -298,6 +298,11 @@ export const scopeSupportFacetInfos: Record<
       "Used to disqualify a token from being treated as a surrounding pair delimiter. This will usually be operators containing `>` or `<`, eg `<`, `<=`, `->`, etc",
     scopeType: "disqualifyDelimiter",
   },
+  pairDelimiter: {
+    description:
+      "A pair delimiter, eg parentheses, brackets, braces, quotes, etc",
+    scopeType: "pairDelimiter",
+  },
 
   "branch.if": {
     description: "An if/elif/else branch",
diff --git a/packages/common/src/scopeSupportFacets/scopeSupportFacets.types.ts b/packages/common/src/scopeSupportFacets/scopeSupportFacets.types.ts
@@ -80,6 +80,7 @@ export const scopeSupportFacets = [
   "textFragment.string.multiLine",
 
   "disqualifyDelimiter",
+  "pairDelimiter",
 
   "branch.if",
   "branch.if.iteration",
diff --git a/packages/common/src/types/command/PartialTargetDescriptor.types.ts b/packages/common/src/types/command/PartialTargetDescriptor.types.ts
@@ -205,6 +205,7 @@ export const simpleScopeTypeTypes = [
   // Private scope types
   "textFragment",
   "disqualifyDelimiter",
+  "pairDelimiter",
 ] as const;
 
 export function isSimpleScopeType(
diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/queryPredicateOperators.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/queryPredicateOperators.ts
@@ -62,6 +62,21 @@ class HasMultipleChildrenOfType extends QueryPredicateOperator<HasMultipleChildr
   }
 }
 
+/**
+ * A predicate operator that returns true if the nodes text matched the regular expression
+ */
+class Match extends QueryPredicateOperator<Match> {
+  name = "match?" as const;
+  schema = z.tuple([q.node, q.string]);
+
+  run(nodeInfo: MutableQueryCapture, pattern: string) {
+    const { document, range } = nodeInfo;
+    const regex = new RegExp(pattern, "ds");
+    const text = document.getText(range);
+    return regex.test(text);
+  }
+}
+
 class ChildRange extends QueryPredicateOperator<ChildRange> {
   name = "child-range!" as const;
   schema = z.union([
@@ -277,4 +292,5 @@ export const queryPredicateOperators = [
   new InsertionDelimiter(),
   new SingleOrMultilineDelimiter(),
   new HasMultipleChildrenOfType(),
+  new Match(),
 ];
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/delimiterMaps.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/delimiterMaps.ts
@@ -14,17 +14,6 @@ interface Options {
    * salient example is strings.
    */
   isSingleLine?: boolean;
-
-  /**
-   * This field can be used to force us to treat the side of the delimiter as
-   * unknown. We usually infer this from the fact that the opening and closing
-   * delimiters are the same, but in some cases they are different, but the side
-   * is actually still unknown. In particular, this is the case for Python
-   * string prefixes, where if we see the prefix it doesn't necessarily mean
-   * that it's an opening delimiter. For example, in  `" r"`, note that the `r`
-   * is just part of the string, not a prefix of the opening delimiter.
-   */
-  isUnknownSide?: boolean;
 }
 
 type DelimiterMap = Record<
@@ -52,38 +41,6 @@ const delimiterToText: DelimiterMap = Object.freeze({
   squareBrackets: ["[", "]"],
 });
 
-// https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals
-const pythonPrefixes = [
-  // Base case without a prefix
-  "",
-  // string prefixes
-  "r",
-  "u",
-  "R",
-  "U",
-  "f",
-  "F",
-  "fr",
-  "Fr",
-  "fR",
-  "FR",
-  "rf",
-  "rF",
-  "Rf",
-  "RF",
-  // byte prefixes
-  "b",
-  "B",
-  "br",
-  "Br",
-  "bR",
-  "BR",
-  "rb",
-  "rB",
-  "Rb",
-  "RB",
-];
-
 // FIXME: Probably remove these as part of
 // https://github.com/cursorless-dev/cursorless/issues/1812#issuecomment-1691493746
 const delimiterToTextOverrides: Record<string, Partial<DelimiterMap>> = {
@@ -102,26 +59,8 @@ const delimiterToTextOverrides: Record<string, Partial<DelimiterMap>> = {
   },
 
   python: {
-    singleQuotes: [
-      pythonPrefixes.map((prefix) => `${prefix}'`),
-      "'",
-      { isSingleLine: true, isUnknownSide: true },
-    ],
-    doubleQuotes: [
-      pythonPrefixes.map((prefix) => `${prefix}"`),
-      '"',
-      { isSingleLine: true, isUnknownSide: true },
-    ],
-    tripleSingleQuotes: [
-      pythonPrefixes.map((prefix) => `${prefix}'''`),
-      "'''",
-      { isUnknownSide: true },
-    ],
-    tripleDoubleQuotes: [
-      pythonPrefixes.map((prefix) => `${prefix}"""`),
-      '"""',
-      { isUnknownSide: true },
-    ],
+    tripleSingleQuotes: ["'''", "'''"],
+    tripleDoubleQuotes: ['"""', '"""'],
   },
 
   ruby: {
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getDelimiterOccurrences.ts
@@ -27,6 +27,9 @@ export function getDelimiterOccurrences(
   const disqualifyDelimiters = new OneWayRangeFinder(
     getSortedCaptures(capturesMap.disqualifyDelimiter),
   );
+  const pairDelimiters = new OneWayRangeFinder(
+    getSortedCaptures(capturesMap.pairDelimiter),
+  );
   const textFragments = new OneWayNestedRangeFinder(
     getSortedCaptures(capturesMap.textFragment),
   );
@@ -47,28 +50,35 @@ export function getDelimiterOccurrences(
 
   for (const match of regexMatches) {
     const text = match[0];
-    const range = new Range(
+    const matchRange = new Range(
       document.positionAt(match.index!),
       document.positionAt(match.index! + text.length),
     );
 
-    const delimiter = disqualifyDelimiters.getContaining(range);
-    const isDisqualified = delimiter != null && !delimiter.hasError();
+    const disqualifiedDelimiter = ifNoErrors(
+      disqualifyDelimiters.getContaining(matchRange),
+    );
 
-    if (!isDisqualified) {
-      const textFragmentRange =
-        textFragments.getSmallestContaining(range)?.range;
-      results.push({
-        delimiterInfo: delimiterTextToDelimiterInfoMap[text],
-        textFragmentRange,
-        range,
-      });
+    if (disqualifiedDelimiter != null) {
+      continue;
     }
+
+    results.push({
+      delimiterInfo: delimiterTextToDelimiterInfoMap[text],
+      textFragmentRange: textFragments.getSmallestContaining(matchRange)?.range,
+      range:
+        ifNoErrors(pairDelimiters.getContaining(matchRange))?.range ??
+        matchRange,
+    });
   }
 
   return results;
 }
 
+function ifNoErrors(capture?: QueryCapture): QueryCapture | undefined {
+  return capture != null && !capture.hasError() ? capture : undefined;
+}
+
 function getSortedCaptures(items?: QueryCapture[]): QueryCapture[] {
   if (items == null) {
     return [];
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getIndividualDelimiters.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/getIndividualDelimiters.ts
@@ -21,7 +21,7 @@ import type { IndividualDelimiter } from "./types";
 export function getIndividualDelimiters(
   delimiter: SurroundingPairName,
   languageId: string,
-) {
+): IndividualDelimiter[] {
   const delimiters = complexDelimiterMap[
     delimiter as ComplexSurroundingPairName
   ] ?? [delimiter];
@@ -36,7 +36,7 @@ function getSimpleIndividualDelimiters(
   return delimiters.flatMap((delimiterName) => {
     const [leftDelimiter, rightDelimiter, options] =
       delimiterToText[delimiterName];
-    const { isSingleLine = false, isUnknownSide = false } = options ?? {};
+    const { isSingleLine = false } = options ?? {};
 
     // Allow for the fact that a delimiter might have multiple ways to indicate
     // its opening / closing
@@ -54,9 +54,6 @@ function getSimpleIndividualDelimiters(
       const isRight = rightDelimiters.includes(text);
 
       const side = (() => {
-        if (isUnknownSide) {
-          return "unknown";
-        }
         if (isLeft && !isRight) {
           return "left";
         }
diff --git a/packages/cursorless-engine/src/scopeProviders/ScopeInfoProvider.ts b/packages/cursorless-engine/src/scopeProviders/ScopeInfoProvider.ts
@@ -158,6 +158,7 @@ function isLanguageSpecific(scopeType: ScopeType): boolean {
     case "environment":
     case "textFragment":
     case "disqualifyDelimiter":
+    case "pairDelimiter":
       return true;
 
     case "character":
diff --git a/packages/cursorless-engine/src/spokenForms/defaultSpokenFormMapCore.ts b/packages/cursorless-engine/src/spokenForms/defaultSpokenFormMapCore.ts
@@ -103,6 +103,7 @@ export const defaultSpokenFormMapCore: DefaultSpokenFormMapDefinition = {
     string: isPrivate("parse tree string"),
     textFragment: isPrivate("text fragment"),
     disqualifyDelimiter: isPrivate("disqualify delimiter"),
+    pairDelimiter: isPrivate("pair delimiter"),
     ["private.fieldAccess"]: isPrivate("access"),
     ["private.switchStatementSubject"]: isPrivate("subject"),
   },
diff --git a/queries/python.scm b/queries/python.scm
@@ -652,3 +652,8 @@ operator: [
 (function_definition
   "->" @disqualifyDelimiter
 )
+
+(
+  (string_start) @pairDelimiter
+  (#match? @pairDelimiter "^[a-zA-Z]+")
+)
