Token scope prefers $ over other symbols (#2048)

Fixes #1321 ## 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
cursorless-dev · Nov 28, 2023 · 3fe5d81 · 3fe5d81
1 parent 41cf78d
commit 3fe5d81
Show file tree

Hide file tree

Showing 5 changed files with 149 additions and 45 deletions.
diff --git a/...ges/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CharacterScopeHandler.ts b/...ges/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CharacterScopeHandler.ts
@@ -1,10 +1,10 @@
+import { Direction, ScopeType } from "@cursorless/common";
 import { imap } from "itertools";
 import { NestedScopeHandler } from ".";
-import { generateMatchesInRange } from "../../../util/getMatchesInRange";
-import { Direction, ScopeType } from "@cursorless/common";
 import { getMatcher } from "../../../tokenizer";
-import { testRegex } from "../../../util/regex";
+import { generateMatchesInRange } from "../../../util/getMatchesInRange";
 import { PlainTarget } from "../../targets";
+import { isPreferredOverHelper } from "./isPreferredOverHelper";
 import type { TargetScope } from "./scope.types";
 
 /**
@@ -14,6 +14,7 @@ import type { TargetScope } from "./scope.types";
  */
 const SPLIT_REGEX = /\p{L}\p{M}*|[\p{N}\p{P}\p{S}\p{Z}\p{C}]/gu;
 
+const PREFERRED_SYMBOLS_REGEX = /[$]/g;
 const NONWHITESPACE_REGEX = /\p{L}\p{M}*|[\p{N}\p{P}\p{S}]/gu;
 
 export class CharacterScopeHandler extends NestedScopeHandler {
@@ -49,33 +50,13 @@ export class CharacterScopeHandler extends NestedScopeHandler {
     scopeA: TargetScope,
     scopeB: TargetScope,
   ): boolean | undefined {
-    const {
-      editor: { document },
-    } = scopeA;
     const { identifierMatcher } = getMatcher(this.languageId);
-
-    const aText = document.getText(scopeA.domain);
-    const bText = document.getText(scopeB.domain);
-
-    // Regexes indicating preferences.  We prefer identifiers, then
-    // nonwhitespace.
-    const matchers = [identifierMatcher, NONWHITESPACE_REGEX];
-
-    for (const matcher of matchers) {
-      // NB: Don't directly use `test` here because global regexes are stateful
-      // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec#finding_successive_matches
-      const aMatchesRegex = testRegex(matcher, aText);
-      const bMatchesRegex = testRegex(matcher, bText);
-
-      if (aMatchesRegex && !bMatchesRegex) {
-        return true;
-      }
-
-      if (bMatchesRegex && !aMatchesRegex) {
-        return false;
-      }
-    }
-
-    return undefined;
+    // Regexes indicating preferences. We prefer identifiers, preferred
+    // symbols, then nonwhitespace.
+    return isPreferredOverHelper(scopeA, scopeB, [
+      identifierMatcher,
+      PREFERRED_SYMBOLS_REGEX,
+      NONWHITESPACE_REGEX,
+    ]);
   }
 }
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TokenScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TokenScopeHandler.ts
@@ -1,12 +1,14 @@
+import { Direction } from "@cursorless/common";
 import { imap } from "itertools";
 import { NestedScopeHandler } from ".";
-import { generateMatchesInRange } from "../../../util/getMatchesInRange";
-import { Direction } from "@cursorless/common";
 import { getMatcher } from "../../../tokenizer";
-import { testRegex } from "../../../util/regex";
+import { generateMatchesInRange } from "../../../util/getMatchesInRange";
 import { TokenTarget } from "../../targets";
+import { isPreferredOverHelper } from "./isPreferredOverHelper";
 import type { TargetScope } from "./scope.types";
 
+const PREFERRED_SYMBOLS_REGEX = /[$]/g;
+
 export class TokenScopeHandler extends NestedScopeHandler {
   public readonly scopeType = { type: "token" } as const;
   public readonly iterationScopeType = { type: "line" } as const;
@@ -37,17 +39,12 @@ export class TokenScopeHandler extends NestedScopeHandler {
     scopeA: TargetScope,
     scopeB: TargetScope,
   ): boolean | undefined {
-    const {
-      editor: { document },
-    } = scopeA;
-    const { identifierMatcher } = getMatcher(document.languageId);
-
-    // NB: Don't directly use `test` here because global regexes are stateful
-    // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec#finding_successive_matches
-    return testRegex(identifierMatcher, document.getText(scopeA.domain))
-      ? true
-      : testRegex(identifierMatcher, document.getText(scopeB.domain))
-      ? false
-      : undefined;
+    const { identifierMatcher } = getMatcher(this.languageId);
+    // Regexes indicating preferences. We prefer identifiers then preferred
+    // symbols.
+    return isPreferredOverHelper(scopeA, scopeB, [
+      identifierMatcher,
+      PREFERRED_SYMBOLS_REGEX,
+    ]);
   }
 }
diff --git a/...ges/cursorless-engine/src/processTargets/modifiers/scopeHandlers/isPreferredOverHelper.ts b/...ges/cursorless-engine/src/processTargets/modifiers/scopeHandlers/isPreferredOverHelper.ts
@@ -0,0 +1,28 @@
+import { testRegex } from "../../../util/regex";
+import type { TargetScope } from "./scope.types";
+
+export function isPreferredOverHelper(
+  scopeA: TargetScope,
+  scopeB: TargetScope,
+  matchers: RegExp[],
+): boolean | undefined {
+  const textA = scopeA.editor.document.getText(scopeA.domain);
+  const textB = scopeB.editor.document.getText(scopeB.domain);
+
+  for (const matcher of matchers) {
+    // NB: Don't directly use `test` here because global regexes are stateful
+    // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec#finding_successive_matches
+    const aMatchesRegex = testRegex(matcher, textA);
+    const bMatchesRegex = testRegex(matcher, textB);
+
+    if (aMatchesRegex && !bMatchesRegex) {
+      return true;
+    }
+
+    if (bMatchesRegex && !aMatchesRegex) {
+      return false;
+    }
+  }
+
+  return undefined;
+}
diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/character/changeChar.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/character/changeChar.yml
@@ -0,0 +1,49 @@
+languageId: plaintext
+command:
+  version: 6
+  spokenForm: change char
+  action:
+    name: clearAndSetSelection
+    target:
+      type: primitive
+      modifiers:
+        - type: containingScope
+          scopeType: {type: character}
+  usePrePhraseSnapshot: true
+initialState:
+  documentContents: |-
+    $.
+    .$
+    $a
+    a$
+    $$
+  selections:
+    - anchor: {line: 4, character: 1}
+      active: {line: 4, character: 1}
+    - anchor: {line: 3, character: 1}
+      active: {line: 3, character: 1}
+    - anchor: {line: 2, character: 1}
+      active: {line: 2, character: 1}
+    - anchor: {line: 1, character: 1}
+      active: {line: 1, character: 1}
+    - anchor: {line: 0, character: 1}
+      active: {line: 0, character: 1}
+  marks: {}
+finalState:
+  documentContents: |-
+    .
+    .
+    $
+    $
+    $
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+    - anchor: {line: 1, character: 1}
+      active: {line: 1, character: 1}
+    - anchor: {line: 2, character: 1}
+      active: {line: 2, character: 1}
+    - anchor: {line: 3, character: 0}
+      active: {line: 3, character: 0}
+    - anchor: {line: 4, character: 1}
+      active: {line: 4, character: 1}
diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/changeToken.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/changeToken.yml
@@ -0,0 +1,49 @@
+languageId: plaintext
+command:
+  version: 6
+  spokenForm: change token
+  action:
+    name: clearAndSetSelection
+    target:
+      type: primitive
+      modifiers:
+        - type: containingScope
+          scopeType: {type: token}
+  usePrePhraseSnapshot: true
+initialState:
+  documentContents: |-
+    $.
+    .$
+    $a
+    a$
+    $$
+  selections:
+    - anchor: {line: 4, character: 1}
+      active: {line: 4, character: 1}
+    - anchor: {line: 3, character: 1}
+      active: {line: 3, character: 1}
+    - anchor: {line: 2, character: 1}
+      active: {line: 2, character: 1}
+    - anchor: {line: 1, character: 1}
+      active: {line: 1, character: 1}
+    - anchor: {line: 0, character: 1}
+      active: {line: 0, character: 1}
+  marks: {}
+finalState:
+  documentContents: |-
+    .
+    .
+    $
+    $
+    $
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+    - anchor: {line: 1, character: 1}
+      active: {line: 1, character: 1}
+    - anchor: {line: 2, character: 1}
+      active: {line: 2, character: 1}
+    - anchor: {line: 3, character: 0}
+      active: {line: 3, character: 0}
+    - anchor: {line: 4, character: 1}
+      active: {line: 4, character: 1}