Initial work for smarter that marks (#957)

* Initial work for smarter `that` marks

* No target serialization default attribute removal

* test transformer [to remove]

* Run transformation script on that and source marks

Author: pokey

src/actions/BringMoveSwap.ts

@@ -261,7 +261,7 @@ class BringMoveSwap implements Action {
 
     await this.decorateThatMark(thatMark);
 
-    return { thatMark, sourceMark };
+    return { thatSelections: thatMark, sourceSelections: sourceMark };
   }
 }
 

src/actions/Call.ts

@@ -22,12 +22,13 @@ export default class Call implements Action {
     );
 
     // NB: We unwrap and then rewrap the return value here so that we don't include the source mark
-    const { thatMark } = await this.graph.actions.wrapWithPairedDelimiter.run(
-      [destinations],
-      texts[0] + "(",
-      ")"
-    );
+    const { thatSelections: thatMark } =
+      await this.graph.actions.wrapWithPairedDelimiter.run(
+        [destinations],
+        texts[0] + "(",
+        ")"
+      );
 
-    return { thatMark };
+    return { thatSelections: thatMark };
   }
 }

src/actions/Clear.ts

@@ -23,7 +23,9 @@ export default class Clear implements Action {
         })
     );
 
-    const { thatMark } = await this.graph.actions.remove.run([plainTargets]);
+    const { thatSelections: thatMark } = await this.graph.actions.remove.run([
+      plainTargets,
+    ]);
 
     if (thatMark != null) {
       await setSelectionsAndFocusEditor(
@@ -32,6 +34,6 @@ export default class Clear implements Action {
       );
     }
 
-    return { thatMark };
+    return { thatSelections: thatMark };
   }
 }

src/actions/CommandAction.ts

@@ -129,6 +129,6 @@ export default class CommandAction implements Action {
       await focusEditor(originalEditor);
     }
 
-    return { thatMark };
+    return { thatSelections: thatMark };
   }
 }

src/actions/CutCopy.ts

@@ -42,12 +42,12 @@ export class Cut implements Action {
 
     await this.graph.actions.copyToClipboard.run([targets], options);
 
-    const { thatMark } = await this.graph.actions.remove.run(
+    const { thatSelections: thatMark } = await this.graph.actions.remove.run(
       [targets],
       options
     );
 
-    return { thatMark };
+    return { thatSelections: thatMark };
   }
 }
 

src/actions/Deselect.ts

@@ -30,7 +30,7 @@ export default class Deselect implements Action {
     });
 
     return {
-      thatMark: createThatMark(targets),
+      thatSelections: createThatMark(targets),
     };
   }
 }

src/actions/EditNew/EditNew.ts

@@ -50,7 +50,7 @@ export class EditNew implements Action {
     await setSelectionsAndFocusEditor(editor, newSelections);
 
     return {
-      thatMark: createThatMark(state.targets, state.thatRanges),
+      thatSelections: createThatMark(state.targets, state.thatRanges),
     };
   }
 }

src/actions/EditNew/runNotebookCellTargets.ts

@@ -25,5 +25,5 @@ export async function runNotebookCellTargets(
     );
   }
 
-  return { thatMark };
+  return { thatSelections: thatMark };
 }

src/actions/Find.ts

@@ -14,13 +14,13 @@ export class FindInFiles implements Action {
 
     const {
       returnValue: [query],
-      thatMark,
+      thatSelections: thatMark,
     } = await this.graph.actions.getText.run([targets]);
 
     await commands.executeCommand("workbench.action.findInFiles", {
       query,
     });
 
-    return { thatMark };
+    return { thatSelections: thatMark };
   }
 }

src/actions/Fold.ts

@@ -45,7 +45,7 @@ class FoldAction implements Action {
     }
 
     return {
-      thatMark: createThatMark(targets),
+      thatSelections: createThatMark(targets),
     };
   }
 }

src/actions/FollowLink.ts

@@ -30,7 +30,7 @@ export default class FollowLink implements Action {
     }
 
     return {
-      thatMark: createThatMark(targets),
+      thatSelections: createThatMark(targets),
     };
   }
 

src/actions/GenerateSnippet/GenerateSnippet.ts

@@ -229,7 +229,7 @@ export default class GenerateSnippet implements Action {
     });
 
     return {
-      thatMark: targets.map(({ editor, contentSelection }) => ({
+      thatSelections: targets.map(({ editor, contentSelection }) => ({
         editor,
         selection: contentSelection,
       })),

src/actions/GetText.ts

@@ -28,7 +28,7 @@ export default class GetText implements Action {
 
     return {
       returnValue: targets.map((target) => target.contentText),
-      thatMark: createThatMark(targets),
+      thatSelections: createThatMark(targets),
     };
   }
 }

src/actions/Highlight.ts

@@ -19,7 +19,7 @@ export default class Highlight implements Action {
     await this.graph.editStyles.setDecorations(targets, style);
 
     return {
-      thatMark: createThatMark(targets),
+      thatSelections: createThatMark(targets),
     };
   }
 }

src/actions/InsertCopy.ts

@@ -33,8 +33,8 @@ class InsertCopy implements Action {
     );
 
     return {
-      sourceMark: results.flatMap(({ sourceMark }) => sourceMark),
-      thatMark: results.flatMap(({ thatMark }) => thatMark),
+      sourceSelections: results.flatMap(({ sourceMark }) => sourceMark),
+      thatSelections: results.flatMap(({ thatMark }) => thatMark),
     };
   }
 

src/actions/InsertEmptyLines.ts

@@ -87,7 +87,7 @@ class InsertEmptyLines implements Action {
 
     const thatMark = results.flatMap((result) => result.thatMark);
 
-    return { thatMark };
+    return { thatSelections: thatMark };
   }
 }
 

src/actions/InsertSnippet.ts

@@ -96,7 +96,7 @@ export default class InsertSnippet implements Action {
     );
 
     return {
-      thatMark: updatedTargetSelections.map((selection) => ({
+      thatSelections: updatedTargetSelections.map((selection) => ({
         editor,
         selection,
       })),

src/actions/Paste.ts

@@ -72,7 +72,7 @@ export class Paste {
     );
 
     return {
-      thatMark: updatedTargetSelections.map((selection) => ({
+      thatSelections: updatedTargetSelections.map((selection) => ({
         editor: targetEditor,
         selection,
       })),

src/actions/Remove.ts

@@ -42,6 +42,6 @@ export default class Delete implements Action {
       })
     );
 
-    return { thatMark };
+    return { thatSelections: thatMark };
   }
 }

src/actions/Replace.ts

@@ -70,6 +70,6 @@ export default class Replace implements Action {
       )
     );
 
-    return { thatMark };
+    return { thatSelections: thatMark };
   }
 }

src/actions/Rewrap.ts

@@ -60,8 +60,8 @@ export default class Rewrap implements Action {
     );
 
     return {
-      sourceMark: results.flatMap(({ sourceMark }) => sourceMark),
-      thatMark: results.flatMap(({ thatMark }) => thatMark),
+      sourceSelections: results.flatMap(({ sourceMark }) => sourceMark),
+      thatSelections: results.flatMap(({ thatMark }) => thatMark),
     };
   }
 }

src/actions/Scroll.ts

@@ -56,7 +56,7 @@ class Scroll implements Action {
     );
 
     return {
-      thatMark: createThatMark(targets),
+      thatSelections: createThatMark(targets),
     };
   }
 }

src/actions/SetSelection.ts

@@ -21,7 +21,7 @@ export class SetSelection implements Action {
     await setSelectionsAndFocusEditor(editor, selections);
 
     return {
-      thatMark: createThatMark(targets),
+      thatSelections: createThatMark(targets),
     };
   }
 }

src/actions/ToggleBreakpoint.ts

@@ -61,7 +61,7 @@ export default class ToggleBreakpoint implements Action {
     debug.removeBreakpoints(toRemove);
 
     return {
-      thatMark: createThatMark(thatTargets),
+      thatSelections: createThatMark(thatTargets),
     };
   }
 }

src/actions/Wrap.ts

@@ -127,8 +127,8 @@ export default class Wrap implements Action {
     );
 
     return {
-      sourceMark: results.flatMap(({ sourceMark }) => sourceMark),
-      thatMark: results.flatMap(({ thatMark }) => thatMark),
+      sourceSelections: results.flatMap(({ sourceMark }) => sourceMark),
+      thatSelections: results.flatMap(({ thatMark }) => thatMark),
     };
   }
 }

src/actions/WrapWithSnippet.ts

@@ -88,7 +88,7 @@ export default class WrapWithSnippet implements Action {
     );
 
     return {
-      thatMark: updatedTargetSelections.map((selection) => ({
+      thatSelections: updatedTargetSelections.map((selection) => ({
         editor,
         selection,
       })),

src/actions/actions.types.ts

@@ -56,10 +56,41 @@ export type ActionType =
   | "wrapWithPairedDelimiter"
   | "wrapWithSnippet";
 
+/**
+ * To be returned by {@link Action.run}
+ */
 export interface ActionReturnValue {
+  /**
+   * The value that should be returned to the caller of the command
+   */
   returnValue?: any;
-  thatMark?: SelectionWithEditor[];
-  sourceMark?: SelectionWithEditor[];
+
+  /**
+   * A list of selections that will become the `that` mark for the next command.
+   * The given selections will be wrapped in {@link UntypedTarget}s. This
+   * attribute is provided for convenience. Mutually exclusive with
+   * {@link thatTargets}
+   */
+  thatSelections?: SelectionWithEditor[];
+
+  /**
+   * A list of targets that will become the `that` mark for the next command.
+   * Mutually exclusive with {@link thatSelections}
+   */
+  thatTargets?: Target[];
+
+  /**
+   * A list of selections that will become the `source` mark for the next command.
+   * The given selections will be wrapped in {@link UntypedTarget}s. This
+   * attribute is provided for convenience. Mutually exclusive with {@link sourceTargets}
+   */
+  sourceSelections?: SelectionWithEditor[];
+
+  /**
+   * A list of targets that will become the `source` mark for the next command.
+   * Mutually exclusive with {@link sourceSelections}
+   */
+  sourceTargets?: Target[];
 }
 
 export interface Action {

src/core/ThatMark.ts

@@ -1,10 +1,10 @@
-import { SelectionWithEditor } from "../typings/Types";
+import { Target } from "../typings/target.types";
 
 export class ThatMark {
-  private mark?: SelectionWithEditor[];
+  private mark?: Target[];
 
-  set(value?: SelectionWithEditor[]) {
-    this.mark = value;
+  set(targets: Target[] | undefined) {
+    this.mark = targets;
   }
 
   get() {

src/core/commandRunner/CommandRunner.ts

@@ -3,13 +3,19 @@ import { ActionType } from "../../actions/actions.types";
 import { OutdatedExtensionError } from "../../errors";
 import processTargets from "../../processTargets";
 import isTesting from "../../testUtil/isTesting";
-import { Graph, ProcessedTargetsContext } from "../../typings/Types";
+import { Target } from "../../typings/target.types";
+import {
+  Graph,
+  ProcessedTargetsContext,
+  SelectionWithEditor,
+} from "../../typings/Types";
 import { isString } from "../../util/type";
 import { canonicalizeAndValidateCommand } from "../commandVersionUpgrades/canonicalizeAndValidateCommand";
 import { PartialTargetV0V1 } from "../commandVersionUpgrades/upgradeV1ToV2/commandV1.types";
 import inferFullTargets from "../inferFullTargets";
 import { ThatMark } from "../ThatMark";
 import { Command } from "./command.types";
+import { selectionToThatTarget } from "./selectionToThatTarget";
 
 // TODO: Do this using the graph once we migrate its dependencies onto the graph
 export default class CommandRunner {
@@ -136,12 +142,16 @@ export default class CommandRunner {
 
       const {
         returnValue,
-        thatMark: newThatMark,
-        sourceMark: newSourceMark,
+        thatSelections: newThatSelections,
+        thatTargets: newThatTargets,
+        sourceSelections: newSourceSelections,
+        sourceTargets: newSourceTargets,
       } = await action.run(targets, ...actionArgs);
 
-      this.thatMark.set(newThatMark);
-      this.sourceMark.set(newSourceMark);
+      this.thatMark.set(constructThatTarget(newThatTargets, newThatSelections));
+      this.sourceMark.set(
+        constructThatTarget(newSourceTargets, newSourceSelections)
+      );
 
       if (this.graph.testCaseRecorder.isActive()) {
         await this.graph.testCaseRecorder.postCommandHook(returnValue);
@@ -210,3 +220,20 @@ export default class CommandRunner {
     this.disposables.forEach(({ dispose }) => dispose());
   }
 }
+
+function constructThatTarget(
+  targets: Target[] | undefined,
+  selections: SelectionWithEditor[] | undefined
+) {
+  if (targets != null && selections != null) {
+    throw Error(
+      "Actions may only return full targets or selections for that mark"
+    );
+  }
+
+  if (selections != null) {
+    return selections.map(selectionToThatTarget);
+  } else {
+    return targets;
+  }
+}