avoid allocating hats to the first letter of a token (#1723)

We could get much fancier than this,
but after running this with a day it appears to help some,
and it is nice and simple.

I propose that we declare that it fixes #1658,
at least for now.



## Checklist

- [/] 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: Andreas Arvidsson <[email protected]>
Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>

Author: 3 people

data/fixtures/recorded/hatTokenMap/takeEach.yml

@@ -0,0 +1,23 @@
+languageId: plaintext
+command:
+  version: 5
+  spokenForm: take each
+  action: {name: setSelection}
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: e}
+  usePrePhraseSnapshot: false
+initialState:
+  documentContents: hello world
+  selections:
+    - anchor: {line: 0, character: 11}
+      active: {line: 0, character: 11}
+  marks:
+    default.e:
+      start: {line: 0, character: 0}
+      end: {line: 0, character: 5}
+finalState:
+  documentContents: hello world
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 5}

data/fixtures/recorded/testCaseRecorder/takeEach.yml

@@ -0,0 +1,24 @@
+languageId: plaintext
+command:
+  version: 7
+  spokenForm: take each
+  action:
+    name: setSelection
+    target:
+      type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: e}
+  usePrePhraseSnapshot: false
+initialState:
+  documentContents: hello world
+  selections:
+    - anchor: {line: 0, character: 11}
+      active: {line: 0, character: 11}
+  marks:
+    default.e:
+      start: {line: 0, character: 0}
+      end: {line: 0, character: 5}
+finalState:
+  documentContents: hello world
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 5}

packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/WordScopeHandler/WordTokenizer.ts

@@ -4,10 +4,8 @@ import { getMatcher } from "../../../../tokenizer";
 const CAMEL_REGEX = /\p{Lu}?\p{Ll}+|\p{Lu}+(?!\p{Ll})|\p{N}+/gu;
 
 /**
- * This class just encapsulates the word-splitting logic from
- * {@link WordScopeHandler}.  We could probably just inline it into that class,
- * but for now we need it here because we can't yet properly mock away vscode
- * for the unit tests in subtoken.test.ts.
+ * This class encapsulates word-splitting logic.
+ * It is used by the {@link WordScopeHandler} and the hat allocator.
  */
 export class WordTokenizer {
   private wordRegex: RegExp;

packages/cursorless-engine/src/test/fixtures/subtoken.fixture.ts

@@ -84,4 +84,8 @@ export const subtokenFixture: Fixture[] = [
     input: "_quickBrownFox_",
     expectedOutput: ["quick", "Brown", "Fox"],
   },
+  {
+    input: "thisIsATest",
+    expectedOutput: ["this", "Is", "A", "Test"],
+  },
 ];

packages/cursorless-engine/src/util/allocateHats/HatMetrics.ts

@@ -15,6 +15,12 @@ export type HatMetric = (hat: HatCandidate) => number;
  */
 export const negativePenalty: HatMetric = ({ penalty }) => -penalty;
 
+/**
+ * @returns A metric that penalizes graphemes that are the first letter of a word within a token
+ */
+export const avoidFirstLetter: HatMetric = ({ isFirstLetter }) =>
+  isFirstLetter ? -1 : 0;
+
 /**
  * @param hatOldTokenRanks A map from a hat candidate (grapheme+style combination) to the score of the
  * token that used the given hat in the previous hat allocation.

packages/cursorless-engine/src/util/allocateHats/allocateHats.ts

@@ -7,6 +7,7 @@ import type {
   TokenHat,
 } from "@cursorless/common";
 import { CompositeKeyMap, DefaultMap, Range } from "@cursorless/common";
+import { WordTokenizer } from "../../processTargets/modifiers/scopeHandlers/WordScopeHandler/WordTokenizer";
 import type {
   Grapheme,
   TokenGraphemeSplitter,
@@ -19,6 +20,7 @@ export interface HatCandidate {
   grapheme: Grapheme;
   style: HatStyleName;
   penalty: number;
+  isFirstLetter: boolean;
 }
 
 interface AllocateHatsOptions {
@@ -169,12 +171,21 @@ function getTokenRemainingHatCandidates(
   graphemeRemainingHatCandidates: DefaultMap<string, HatStyleName[]>,
   enabledHatStyles: HatStyleMap,
 ): HatCandidate[] {
+  const candidates: HatCandidate[] = [];
+  const graphemes = tokenGraphemeSplitter.getTokenGraphemes(token.text);
+  const firstLetterOffsets = new Set(
+    new WordTokenizer(token.editor.document.languageId)
+      .splitIdentifier(token.text)
+      .map((word) => word.index),
+  );
+
   // Use iteration here instead of functional constructs,
   // because this is a hot path and we want to avoid allocating arrays
   // and calling tiny functions lots of times.
-  const candidates: HatCandidate[] = [];
-  const graphemes = tokenGraphemeSplitter.getTokenGraphemes(token.text);
+
   for (const grapheme of graphemes) {
+    const isFirstLetter = firstLetterOffsets.has(grapheme.tokenStartOffset);
+
     for (const style of graphemeRemainingHatCandidates.get(grapheme.text)) {
       // Allocating and pushing all of these objects is
       // the single most expensive thing in hat allocation.
@@ -183,9 +194,11 @@ function getTokenRemainingHatCandidates(
         grapheme,
         style,
         penalty: enabledHatStyles[style].penalty,
+        isFirstLetter,
       });
     }
   }
+
   return candidates;
 }
 

packages/cursorless-engine/src/util/allocateHats/chooseTokenHat.ts

@@ -2,6 +2,7 @@ import type { HatStability, TokenHat } from "@cursorless/common";
 import type { HatCandidate } from "./allocateHats";
 import type { RankingContext } from "./getHatRankingContext";
 import {
+  avoidFirstLetter,
   hatOldTokenRank,
   isOldTokenHat,
   minimumTokenRankContainingGrapheme,
@@ -75,6 +76,9 @@ export function chooseTokenHat(
     // Narrow to the hats with the lowest penalty
     negativePenalty,
 
+    // Avoid the first grapheme of the token if possible
+    avoidFirstLetter,
+
     // Prefer hats that sit on a grapheme that doesn't appear in any highly
     // ranked token
     minimumTokenRankContainingGrapheme(tokenRank, graphemeTokenRanks),

packages/cursorless-vscode-e2e/src/suite/breakpoints.vscode.test.ts

@@ -38,7 +38,7 @@ async function breakpointHarpAdd() {
         mark: {
           type: "decoratedSymbol",
           symbolColor: "default",
-          character: "h",
+          character: "e",
         },
       },
     ],
@@ -66,7 +66,7 @@ async function breakpointTokenHarpAdd() {
         mark: {
           type: "decoratedSymbol",
           symbolColor: "default",
-          character: "h",
+          character: "e",
         },
       },
     ],
@@ -101,7 +101,7 @@ async function breakpointHarpRemove() {
         mark: {
           type: "decoratedSymbol",
           symbolColor: "default",
-          character: "h",
+          character: "e",
         },
       },
     ],
@@ -136,7 +136,7 @@ async function breakpointTokenHarpRemove() {
         mark: {
           type: "decoratedSymbol",
           symbolColor: "default",
-          character: "h",
+          character: "e",
         },
       },
     ],

packages/cursorless-vscode-e2e/src/suite/commandHistory.vscode.test.ts

@@ -45,7 +45,7 @@ suite("commandHistory", function () {
 async function testActive(tmpdir: string) {
   await injectFakeIsActive(true);
   await initalizeEditor();
-  const command = takeCommand("h");
+  const command = takeCommand("e");
   await runCursorlessCommand(command);
 
   const content = await getLogEntry(tmpdir);
@@ -71,7 +71,7 @@ async function testSanitization(tmpdir: string) {
 async function testInactive(tmpdir: string) {
   await injectFakeIsActive(false);
   await initalizeEditor();
-  await runCursorlessCommand(takeCommand("h"));
+  await runCursorlessCommand(takeCommand("e"));
 
   assert.notOk(existsSync(tmpdir));
 }

packages/cursorless-vscode-e2e/src/suite/crossCellsSetSelection.vscode.test.ts

@@ -34,7 +34,7 @@ async function runTest() {
         mark: {
           type: "decoratedSymbol",
           symbolColor: "default",
-          character: "w",
+          character: "o",
         },
       },
     ],

packages/cursorless-vscode-e2e/src/suite/intraCellSetSelection.vscode.test.ts

@@ -34,7 +34,7 @@ async function runTest() {
         mark: {
           type: "decoratedSymbol",
           symbolColor: "default",
-          character: "w",
+          character: "r",
         },
       },
     ],

packages/cursorless-vscode-e2e/src/suite/keyboard/basic.vscode.test.ts

@@ -211,8 +211,8 @@ async function basic() {
 
   await vscode.commands.executeCommand("cursorless.keyboard.modal.modeOn");
 
-  // Target default f
-  await typeText("df");
+  // Target default o
+  await typeText("do");
 
   // Target containing function
   await typeText("sf");

packages/cursorless-vscode-e2e/src/suite/pourAcrossSplit.vscode.test.ts

@@ -26,7 +26,7 @@ async function runTest() {
         mark: {
           type: "decoratedSymbol",
           symbolColor: "default",
-          character: "h",
+          character: "e",
         },
       },
     ],

packages/cursorless-vscode-e2e/src/suite/testCaseRecorder.vscode.test.ts

@@ -93,7 +93,7 @@ async function testCaseRecorderGracefulError() {
     }
 
     await initalizeEditor(hatTokenMap);
-    await takeHarp();
+    await takeEach();
     await stopRecording();
     await checkRecordedTest(tmpdir);
   } finally {
@@ -108,7 +108,7 @@ async function runAndCheckTestCaseRecorder(
 ) {
   await initalizeEditor(hatTokenMap);
   await startRecording(...extraArgs);
-  await takeHarp();
+  await takeEach();
   await stopRecording();
   await checkRecordedTest(tmpdir);
 }
@@ -132,10 +132,10 @@ async function stopRecording() {
   await vscode.commands.executeCommand("cursorless.recordTestCase");
 }
 
-async function takeHarp() {
+async function takeEach() {
   await runCursorlessCommand({
     version: LATEST_VERSION,
-    spokenForm: "take harp",
+    spokenForm: "take each",
     usePrePhraseSnapshot: false,
     action: {
       name: "setSelection",
@@ -144,7 +144,7 @@ async function takeHarp() {
         mark: {
           type: "decoratedSymbol",
           symbolColor: "default",
-          character: "h",
+          character: "e",
         },
       },
     },
@@ -156,11 +156,11 @@ async function checkRecordedTest(tmpdir: string) {
   assert.lengthOf(paths, 1);
 
   const actualRecordedTestPath = paths[0];
-  assert.equal(path.basename(actualRecordedTestPath), "takeHarp.yml");
+  assert.equal(path.basename(actualRecordedTestPath), "takeEach.yml");
 
   const expected = (
     await readFile(
-      getFixturePath("recorded/testCaseRecorder/takeHarp.yml"),
+      getFixturePath("recorded/testCaseRecorder/takeEach.yml"),
       "utf8",
     )
   )

packages/cursorless-vscode-e2e/src/suite/wrapWithSnippetAcrossSplit.vscode.test.ts

@@ -38,7 +38,7 @@ async function runTest() {
         mark: {
           type: "decoratedSymbol",
           symbolColor: "default",
-          character: "h",
+          character: "e",
         },
       },
     },