Working version

src/extension.ts

@@ -23,6 +23,7 @@ import canonicalizeActionName from "./util/canonicalizeActionName";
 import { mkdir, stat } from "fs/promises";
 import { join } from "path";
 import { getPrimitiveTargets } from "./util/targetUtils";
+import { doTargetsUseSnapshot } from "./util/doTargetsRequireSnapshot";
 
 export async function activate(context: vscode.ExtensionContext) {
   const fontMeasurements = new FontMeasurements(context);
@@ -124,13 +125,8 @@ export async function activate(context: vscode.ExtensionContext) {
             inputExtraArgs
           );
 
-        if (
-          getPrimitiveTargets(partialTargets).some(
-            (partialTarget) =>
-              partialTarget.mark?.type === "decoratedSymbol" &&
-              partialTarget.mark.useSnapshot
-          )
-        ) {
+        const useSnapshot = doTargetsUseSnapshot(partialTargets);
+        if (useSnapshot) {
           await graph.navigationMap.maybeTakeSnapshot();
         }
 
@@ -175,6 +171,7 @@ export async function activate(context: vscode.ExtensionContext) {
             sourceMark,
             navigationMap: graph.navigationMap!,
             spokenForm,
+            useSnapshot,
           };
           await testCaseRecorder.preCommandHook(command, context);
         }

src/test/suite/fixtures/recorded/navigationMap/takeHarp.yml

@@ -0,0 +1,26 @@
+spokenForm: take harp
+languageId: plaintext
+command:
+  actionName: setSelection
+  partialTargets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: h, useSnapshot: true}
+  extraArgs: []
+initialState:
+  documentContents: hello world
+  selections:
+    - anchor: {line: 0, character: 11}
+      active: {line: 0, character: 11}
+  marks:
+    default.h:
+      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}
+  thatMark:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 5}
+fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: h, useSnapshot: true}, selectionType: token, position: contents, insideOutsideType: inside, modifier: {type: identity}}]

src/test/suite/recorded.test.ts

@@ -24,6 +24,7 @@ import {
 } from "../../util/getExtensionApi";
 import { enableDebugLog } from "../../util/debug";
 import { extractTargetedMarks } from "../../testUtil/extractTargetedMarks";
+import { doTargetsUseSnapshot } from "../../util/doTargetsRequireSnapshot";
 
 function createPosition(position: PositionPlainObject) {
   return new vscode.Position(position.line, position.character);
@@ -108,6 +109,9 @@ async function runTest(file: string) {
   // Assert that recorded decorations are present
   checkMarks(fixture.initialState.marks, cursorlessApi.navigationMap);
 
+  const useSnapshot = doTargetsUseSnapshot(fixture.command.partialTargets);
+  // TODO: Issue the barrier if we need to
+
   const returnValue = await vscode.commands.executeCommand(
     "cursorless.command",
     fixture.spokenForm,
@@ -122,7 +126,8 @@ async function runTest(file: string) {
       : marksToPlainObject(
           extractTargetedMarks(
             Object.keys(fixture.finalState.marks) as string[],
-            cursorlessApi.navigationMap
+            cursorlessApi.navigationMap,
+            useSnapshot
           )
         );
 

src/testUtil/TestCase.ts

@@ -23,6 +23,7 @@ export type TestCaseContext = {
   sourceMark: ThatMark;
   targets: Target[];
   navigationMap: NavigationMap;
+  useSnapshot: boolean;
 };
 
 export type TestCaseFixture = {
@@ -73,15 +74,15 @@ export class TestCase {
   private getMarks() {
     let marks: Record<string, Token>;
 
-    const { navigationMap } = this.context;
+    const { navigationMap, useSnapshot } = this.context;
 
     if (this.isNavigationMapTest) {
       // If we're doing a navigation map test, then we grab the entire
       // navigation map because we'll filter it later based on the marks
       // referenced in the expected follow up command
       marks = Object.fromEntries(navigationMap.getEntries());
     } else {
-      marks = extractTargetedMarks(this.targetKeys, navigationMap);
+      marks = extractTargetedMarks(this.targetKeys, navigationMap, useSnapshot);
     }
 
     return marksToPlainObject(marks);

src/testUtil/extractTargetedMarks.ts

@@ -31,13 +31,18 @@ export function extractTargetKeys(target: Target): string[] {
 
 export function extractTargetedMarks(
   targetKeys: string[],
-  navigationMap: NavigationMap
+  navigationMap: NavigationMap,
+  useSnapshot: boolean
 ) {
   const targetedMarks: { [decoratedCharacter: string]: Token } = {};
 
   targetKeys.forEach((key) => {
     const { hatStyle, character } = NavigationMap.splitKey(key);
-    targetedMarks[key] = navigationMap.getToken(hatStyle, character);
+    targetedMarks[key] = navigationMap.getToken(
+      hatStyle,
+      character,
+      useSnapshot
+    );
   });
 
   return targetedMarks;

src/util/canonicalizeAndValidateCommand.ts

@@ -1,7 +1,7 @@
 import canonicalizeActionName from "./canonicalizeActionName";
 import canonicalizeTargets from "./canonicalizeTargets";
 import { ActionType, PartialTarget, SelectionType } from "../typings/Types";
-import { getPrimitiveTargets } from "./targetUtils";
+import { getPartialPrimitiveTargets } from "./targetUtils";
 
 export function canonicalizeAndValidateCommand(
   inputActionName: string,
@@ -40,7 +40,7 @@ function usesSelectionType(
   selectionType: SelectionType,
   partialTargets: PartialTarget[]
 ) {
-  return getPrimitiveTargets(partialTargets).some(
+  return getPartialPrimitiveTargets(partialTargets).some(
     (partialTarget) => partialTarget.selectionType === selectionType
   );
 }

src/util/canonicalizeTargets.ts

@@ -4,7 +4,7 @@ import {
   ScopeType,
 } from "../typings/Types";
 import update from "immutability-helper";
-import { transformPrimitiveTargets } from "./targetUtils";
+import { transformPartialPrimitiveTargets } from "./targetUtils";
 import { HatStyleName } from "../core/constants";
 import { flow } from "lodash";
 
@@ -43,7 +43,7 @@ const canonicalizeColors = (
     : target;
 
 export default function canonicalizeTargets(partialTargets: PartialTarget[]) {
-  return transformPrimitiveTargets(
+  return transformPartialPrimitiveTargets(
     partialTargets,
     flow(canonicalizeScopeTypes, canonicalizeColors)
   );

src/util/doTargetsRequireSnapshot.ts

@@ -0,0 +1,19 @@
+import { PartialTarget } from "../typings/Types";
+import { uniq } from "lodash";
+import { getPartialPrimitiveTargets } from "./targetUtils";
+
+export function doTargetsUseSnapshot(targets: PartialTarget[]): boolean {
+  const snapshotIds = getPartialPrimitiveTargets(targets).map((target) =>
+    target.mark?.type === "decoratedSymbol"
+      ? target.mark.useSnapshot
+      : undefined
+  );
+
+  const uniqueSnapshotIds = uniq(snapshotIds);
+
+  if (uniqueSnapshotIds.length !== 1) {
+    throw new Error("Must use the same snapshot id for all targets");
+  }
+
+  return uniqueSnapshotIds[0] ?? false;
+}

src/util/targetUtils.ts

@@ -4,6 +4,8 @@ import {
   PartialPrimitiveTarget,
   PartialRangeTarget,
   PartialTarget,
+  PrimitiveTarget,
+  Target,
   TypedSelection,
 } from "../typings/Types";
 
@@ -99,22 +101,43 @@ function createTypeSelection(
  * @param targets The targets to extract from
  * @returns A list of primitive targets
  */
-export function getPrimitiveTargets(targets: PartialTarget[]) {
-  return targets.flatMap(getPrimitiveTargetsHelper);
+export function getPartialPrimitiveTargets(targets: PartialTarget[]) {
+  return targets.flatMap(getPartialPrimitiveTargetsHelper);
 }
 
-function getPrimitiveTargetsHelper(
+function getPartialPrimitiveTargetsHelper(
   target: PartialTarget
 ): PartialPrimitiveTarget[] {
   switch (target.type) {
     case "primitive":
       return [target];
     case "list":
-      return target.elements.flatMap(getPrimitiveTargetsHelper);
+      return target.elements.flatMap(getPartialPrimitiveTargetsHelper);
     case "range":
       return [target.start, target.end];
   }
 }
+/**
+ * Given a list of targets, recursively descends all targets and returns every
+ * contained primitive target.
+ *
+ * @param targets The targets to extract from
+ * @returns A list of primitive targets
+ */
+export function getPrimitiveTargets(targets: Target[]) {
+  return targets.flatMap(getPrimitiveTargetsHelper);
+}
+
+function getPrimitiveTargetsHelper(target: Target): PrimitiveTarget[] {
+  switch (target.type) {
+    case "primitive":
+      return [target];
+    case "list":
+      return target.elements.flatMap(getPrimitiveTargetsHelper);
+    case "range":
+      return [target.anchor, target.active];
+  }
+}
 
 /**
  * Given a list of targets, recursively descends all targets and applies `func`
@@ -123,14 +146,16 @@ function getPrimitiveTargetsHelper(
  * @param targets The targets to extract from
  * @returns A list of primitive targets
  */
-export function transformPrimitiveTargets(
+export function transformPartialPrimitiveTargets(
   targets: PartialTarget[],
   func: (target: PartialPrimitiveTarget) => PartialPrimitiveTarget
 ) {
-  return targets.map((target) => transformPrimitiveTargetsHelper(target, func));
+  return targets.map((target) =>
+    transformPartialPrimitiveTargetsHelper(target, func)
+  );
 }
 
-function transformPrimitiveTargetsHelper(
+function transformPartialPrimitiveTargetsHelper(
   target: PartialTarget,
   func: (target: PartialPrimitiveTarget) => PartialPrimitiveTarget
 ): PartialTarget {
@@ -142,7 +167,7 @@ function transformPrimitiveTargetsHelper(
         ...target,
         elements: target.elements.map(
           (element) =>
-            transformPrimitiveTargetsHelper(element, func) as
+            transformPartialPrimitiveTargetsHelper(element, func) as
               | PartialPrimitiveTarget
               | PartialRangeTarget
         ),