From 3fe5d819a238e4dd2e0599580e27a6f0799d3c40 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 28 Nov 2023 12:48:57 +0100 Subject: [PATCH] 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 --- .../scopeHandlers/CharacterScopeHandler.ts | 41 +++++----------- .../scopeHandlers/TokenScopeHandler.ts | 27 +++++----- .../scopeHandlers/isPreferredOverHelper.ts | 28 +++++++++++ .../recorded/scopes/character/changeChar.yml | 49 +++++++++++++++++++ .../recorded/scopes/token/changeToken.yml | 49 +++++++++++++++++++ 5 files changed, 149 insertions(+), 45 deletions(-) create mode 100644 packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/isPreferredOverHelper.ts create mode 100644 packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/character/changeChar.yml create mode 100644 packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/changeToken.yml diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CharacterScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CharacterScopeHandler.ts index 58b8fb4041..c864357cc8 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CharacterScopeHandler.ts +++ b/packages/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 index a8011827b6..9b49df4c2b 100644 --- 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/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/isPreferredOverHelper.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/isPreferredOverHelper.ts new file mode 100644 index 0000000000..3920e8e18d --- /dev/null +++ b/packages/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 new file mode 100644 index 0000000000..7a2f20f5dd --- /dev/null +++ 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 new file mode 100644 index 0000000000..75b224194d --- /dev/null +++ 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}