Added "its" modifier to use with inference (#1024)

* Added its modifier to use with inference

* Fix test failures

* Remove unnecessary spy

Co-authored-by: Pokey Rule <[email protected]>

Author: AndreasArvidsson

cursorless-talon/src/modifiers/modifiers.py

@@ -19,6 +19,7 @@
     "trailing": "trailing",
     "content": "keepContentFilter",
     "empty": "keepEmptyFilter",
+    "its": "inferPreviousMark",
 }
 
 mod.list(

docs/user/README.md

@@ -333,6 +333,14 @@ For example:
 
 If your cursor / mark is between two delimiters (not adjacent to one), then saying either "left" or "right" will cause cursorless to just expand to the nearest delimiters on either side, without trying to determine whether they are opening or closing delimiters.
 
+#### `"its"`
+
+The the modifier `"its"` is intended to be used as part of a compound target, and will tell Cursorless to use the previously mentioned mark in the compound target.
+
+For example, `"take air past end of its line"` selects the range from the token containing letter `a` to the end of the line containing the same token. This is in contrast from `"take air past end of line"` which selects the range from the token containing letter `a` to the end of the line containing the current selection.
+
+Another example is `"bring air to its value"`, which would cause the token with a hat over `a` to replace the return value containing it.
+
 ### Compound targets
 
 Individual targets can be combined into compound targets to make bigger targets or refer to multiple selections at the same time.

src/core/commandRunner/CommandRunner.ts

@@ -10,7 +10,10 @@ import {
   SelectionWithEditor,
 } from "../../typings/Types";
 import { isString } from "../../util/type";
-import { canonicalizeAndValidateCommand } from "../commandVersionUpgrades/canonicalizeAndValidateCommand";
+import {
+  canonicalizeAndValidateCommand,
+  checkForOldInference,
+} from "../commandVersionUpgrades/canonicalizeAndValidateCommand";
 import { PartialTargetV0V1 } from "../commandVersionUpgrades/upgradeV1ToV2/commandV1.types";
 import inferFullTargets from "../inferFullTargets";
 import { ThatMark } from "../ThatMark";
@@ -135,6 +138,10 @@ export default class CommandRunner {
         );
       }
 
+      // NB: We do this once test recording has started so that we can capture
+      // warning.
+      checkForOldInference(this.graph, partialTargetDescriptors);
+
       const targets = processTargets(
         processedTargetsContext,
         targetDescriptors

src/core/commandVersionUpgrades/canonicalizeAndValidateCommand.ts

@@ -1,11 +1,14 @@
+import { window } from "vscode";
 import { ActionType } from "../../actions/actions.types";
 import { OutdatedExtensionError } from "../../errors";
 import {
   Modifier,
   PartialTargetDescriptor,
   SimpleScopeTypeType,
 } from "../../typings/targetDescriptor.types";
+import { Graph } from "../../typings/Types";
 import { getPartialPrimitiveTargets } from "../../util/getPrimitiveTargets";
+import { globalStateKeys } from "../../util/globalStateKeys";
 import {
   Command,
   CommandComplete,
@@ -84,7 +87,7 @@ function upgradeCommand(command: Command): CommandLatest {
   return command;
 }
 
-export function validateCommand(
+function validateCommand(
   actionName: ActionType,
   partialTargets: PartialTargetDescriptor[]
 ) {
@@ -110,3 +113,40 @@ function usesScopeType(
     )
   );
 }
+
+export async function checkForOldInference(
+  graph: Graph,
+  partialTargets: PartialTargetDescriptor[]
+) {
+  const hasOldInference = partialTargets.some((target) => {
+    return (
+      target.type === "range" &&
+      target.active.mark == null &&
+      target.active.modifiers?.some((m) => m.type === "position") &&
+      !target.active.modifiers?.some((m) => m.type === "inferPreviousMark")
+    );
+  });
+
+  if (hasOldInference) {
+    const hideInferenceWarning =
+      graph.extensionContext.globalState.get<boolean>(
+        globalStateKeys.hideInferenceWarning,
+        false
+      );
+
+    if (!hideInferenceWarning) {
+      const pressed = await graph.ide.messages.showWarning(
+        "deprecatedPositionInference",
+        'The "past start of" / "past end of" form has changed behavior.  For the old behavior, you can now say "past start of its" / "past end of its". For example, "take air past end of its line".  You may also consider using "head" / "tail" instead; see https://www.cursorless.org/docs/#head-and-tail',
+        "Don't show again"
+      );
+
+      if (pressed) {
+        graph.extensionContext.globalState.update(
+          globalStateKeys.hideInferenceWarning,
+          true
+        );
+      }
+    }
+  }
+}

src/core/inferFullTargets.ts

@@ -98,19 +98,17 @@ function inferPrimitiveTarget(
   }
 
   const ownPositionModifier = getPositionModifier(target);
-  const ownNonPositionModifiers = getNonPositionModifiers(target);
+  const ownModifiers = getPreservedModifiers(target);
 
-  // Position without a mark can be something like "take air past end of line"
-  // We will remove this case when we implement #736
   const mark = target.mark ??
-    (ownPositionModifier == null ? null : getPreviousMark(previousTargets)) ?? {
+    (shouldInferPreviousMark(target)
+      ? getPreviousMark(previousTargets)
+      : null) ?? {
       type: "cursor",
     };
 
   const modifiers =
-    ownNonPositionModifiers ??
-    getPreviousNonPositionModifiers(previousTargets) ??
-    [];
+    ownModifiers ?? getPreviousPreservedModifiers(previousTargets) ?? [];
 
   const positionModifier =
     ownPositionModifier ?? getPreviousPositionModifier(previousTargets);
@@ -143,22 +141,33 @@ function getPositionModifier(
     : (target.modifiers[positionModifierIndex] as PositionModifier);
 }
 
+function shouldInferPreviousMark(
+  target: PartialPrimitiveTargetDescriptor
+): boolean {
+  return target.modifiers?.some((m) => m.type === "inferPreviousMark") ?? false;
+}
+
 /**
- * Return a list of non-positional modifiers on the given target. We return
- * undefined if there are none. Note that we will never return an empty list; we
- * will always return `undefined` if there are no non-positional modifiers.
- * @param target The target from which to get the non-positional modifiers
- * @returns A list of non-positional modifiers or `undefined` if there are none
+ * Return a list of modifiers that should not be removed during inference.
+ * Today, we remove positional modifiers, because they have their own field on
+ * the full targets.  We also remove modifiers that only impact inference, such
+ * as `inferPreviousMark`.
+ *
+ * We return `undefined` if there are no preserved modifiers. Note that we will
+ * never return an empty list; we will always return `undefined` if there are no
+ * preserved modifiers.
+ * @param target The target from which to get the modifiers
+ * @returns A list of preserved modifiers or `undefined` if there are none
  */
-function getNonPositionModifiers(
+function getPreservedModifiers(
   target: PartialPrimitiveTargetDescriptor
 ): Modifier[] | undefined {
-  const nonPositionModifiers = target.modifiers?.filter(
-    (modifier) => modifier.type !== "position"
+  const preservedModifiers = target.modifiers?.filter(
+    (modifier) => !["position", "inferPreviousMark"].includes(modifier.type)
   );
-  return nonPositionModifiers == null || nonPositionModifiers.length === 0
+  return preservedModifiers == null || preservedModifiers.length === 0
     ? undefined
-    : nonPositionModifiers;
+    : preservedModifiers;
 }
 
 function getPreviousMark(
@@ -170,10 +179,10 @@ function getPreviousMark(
   );
 }
 
-function getPreviousNonPositionModifiers(
+function getPreviousPreservedModifiers(
   previousTargets: PartialTargetDescriptor[]
 ): Modifier[] | undefined {
-  return getPreviousTargetAttribute(previousTargets, getNonPositionModifiers);
+  return getPreviousTargetAttribute(previousTargets, getPreservedModifiers);
 }
 
 function getPreviousPositionModifier(

src/extension.ts

@@ -4,6 +4,7 @@ import { ThatMark } from "./core/ThatMark";
 import isTesting from "./testUtil/isTesting";
 import { Graph } from "./typings/Types";
 import { getCommandServerApi, getParseTreeApi } from "./util/getExtensionApi";
+import { globalStateKeys } from "./util/globalStateKeys";
 import graphFactories from "./util/graphFactories";
 import makeGraph, { FactoryMap } from "./util/makeGraph";
 
@@ -36,6 +37,11 @@ export async function activate(context: vscode.ExtensionContext) {
   graph.cheatsheet.init();
   graph.statusBarItem.init();
 
+  // Mark these keys for synchronization
+  graph.extensionContext.globalState.setKeysForSync([
+    globalStateKeys.hideInferenceWarning,
+  ]);
+
   const thatMark = new ThatMark();
   const sourceMark = new ThatMark();
 

src/ide/spies/SpyConfiguration.ts

@@ -1,20 +1,18 @@
-import { values } from "lodash";
+import { pickBy, values } from "lodash";
 import { Graph } from "../../typings/Types";
-import { Disposable, IDE } from "../ide.types";
-import SpyConfiguration from "./SpyConfiguration";
+import { Configuration, Disposable, IDE } from "../ide.types";
 import SpyMessages, { Message } from "./SpyMessages";
 
 export interface SpyIDERecordedValues {
-  configuration: undefined;
-  messages: Message[] | undefined;
+  messages?: Message[];
 }
 
 export default class SpyIDE implements IDE {
-  configuration: SpyConfiguration;
+  configuration: Configuration;
   messages: SpyMessages;
 
   constructor(private original: IDE) {
-    this.configuration = new SpyConfiguration(original.configuration);
+    this.configuration = original.configuration;
     this.messages = new SpyMessages(original.messages);
   }
 
@@ -24,11 +22,12 @@ export default class SpyIDE implements IDE {
 
   getSpyValues(): SpyIDERecordedValues | undefined {
     const ret = {
-      configuration: this.configuration.getSpyValues(),
       messages: this.messages.getSpyValues(),
     };
 
-    return values(ret).every((value) => value == null) ? undefined : ret;
+    return values(ret).every((value) => value == null)
+      ? undefined
+      : pickBy(ret, (value) => value != null);
   }
 }
 

src/ide/spies/SpyIDE.ts

@@ -79,6 +79,10 @@ export default (modifier: Modifier): ModifierStage => {
       return new ModifyIfUntypedStage(modifier);
     case "range":
       return new RangeModifierStage(modifier);
+    case "inferPreviousMark":
+      throw Error(
+        `Unexpected modifier '${modifier.type}'; it should have been removed during inference`
+      );
   }
 };
 

src/processTargets/getModifierStage.ts

@@ -21,3 +21,6 @@ finalState:
     - anchor: {line: 0, character: 8}
       active: {line: 0, character: 11}
 fullTargets: [{type: range, excludeStart: false, excludeEnd: false, start: {type: primitive, mark: {type: cursor}, selectionType: token, position: contents, modifier: {type: identity}, insideOutsideType: inside}, end: {type: primitive, mark: {type: cursorToken}, selectionType: token, position: after, modifier: {type: identity}, insideOutsideType: inside}}]
+ide:
+  messages:
+    - {type: warning, id: deprecatedPositionInference}

src/test/suite/fixtures/recorded/compoundTargets/takePastEndOfToken.yml

@@ -21,3 +21,6 @@ finalState:
     - anchor: {line: 0, character: 8}
       active: {line: 0, character: 6}
 fullTargets: [{type: range, excludeStart: false, excludeEnd: false, start: {type: primitive, mark: {type: cursor}, selectionType: token, position: contents, modifier: {type: identity}, insideOutsideType: inside}, end: {type: primitive, mark: {type: cursorToken}, selectionType: token, position: before, modifier: {type: identity}, insideOutsideType: inside}}]
+ide:
+  messages:
+    - {type: warning, id: deprecatedPositionInference}

src/test/suite/fixtures/recorded/compoundTargets/takePastStartOfToken.yml

@@ -0,0 +1,45 @@
+languageId: typescript
+command:
+  spokenForm: bring batt before funk
+  version: 3
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: b}
+    - type: primitive
+      modifiers:
+        - {type: position, position: before}
+        - type: containingScope
+          scopeType: {type: namedFunction}
+  usePrePhraseSnapshot: true
+  action: {name: replaceWithTarget}
+initialState:
+  documentContents: |-
+    function foo() {
+        return "";
+    }
+
+    function bar() {
+        return "";
+    }
+  selections:
+    - anchor: {line: 1, character: 12}
+      active: {line: 1, character: 12}
+  marks:
+    default.b:
+      start: {line: 4, character: 9}
+      end: {line: 4, character: 12}
+finalState:
+  documentContents: |-
+    bar
+
+    function foo() {
+        return "";
+    }
+
+    function bar() {
+        return "";
+    }
+  selections:
+    - anchor: {line: 3, character: 12}
+      active: {line: 3, character: 12}
+fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: b}, modifiers: []}, {type: primitive, mark: {type: cursor}, modifiers: [{type: containingScope, scopeType: {type: namedFunction}}], positionModifier: {type: position, position: before}}]

src/test/suite/fixtures/recorded/inference/bringBattBeforeFunk.yml

@@ -0,0 +1,46 @@
+languageId: typescript
+command:
+  spokenForm: bring batt before its funk
+  version: 3
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: b}
+    - type: primitive
+      modifiers:
+        - {type: position, position: before}
+        - {type: inferPreviousMark}
+        - type: containingScope
+          scopeType: {type: namedFunction}
+  usePrePhraseSnapshot: true
+  action: {name: replaceWithTarget}
+initialState:
+  documentContents: |-
+    function foo() {
+        return "";
+    }
+
+    function bar() {
+        return "";
+    }
+  selections:
+    - anchor: {line: 1, character: 12}
+      active: {line: 1, character: 12}
+  marks:
+    default.b:
+      start: {line: 4, character: 9}
+      end: {line: 4, character: 12}
+finalState:
+  documentContents: |-
+    function foo() {
+        return "";
+    }
+
+    bar
+
+    function bar() {
+        return "";
+    }
+  selections:
+    - anchor: {line: 1, character: 12}
+      active: {line: 1, character: 12}
+fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: b}, modifiers: []}, {type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: b}, modifiers: [{type: containingScope, scopeType: {type: namedFunction}}], positionModifier: {type: position, position: before}}]