Initial work for smarter `that` marks #957

pokey · 2022-09-13T18:24:14Z

This PR is the first step towards adding smarter that marks (#466). It adds the infrastructure so that actions can return smarter that marks, and allows us to test this behaviour, but for readability, it doesn't actually modify any targets to return smarter that marks. Thus, this PR doesn't actually change Cursorless functionally at all.

See also #965, which builds on this PR to add rich targets to a few easy actions

This PR builds on #967

Checklist

Stop removing fields equal to their defaults
Add new PR which removes that mark from all tests except for tests in action/ directory and makes that mark optional in recorded tests
Add new PR which adds support for excluding / including that marks in tests (defaults to excluding), adds ability to include recording test config in a directory, adding config to include that marks in action directory. Make it recursive with parents
I have added tests

pokey · 2022-09-18T14:43:41Z

src/actions/BringMoveSwap.ts

@@ -261,7 +261,7 @@ class BringMoveSwap implements Action {

    await this.decorateThatMark(thatMark);

-    return { thatMark, sourceMark };
+    return { thatSelections: thatMark, sourceSelections: sourceMark };


We renamed thatMark to thatSelections. All changes to actions are just performing this rename

pokey · 2022-09-18T15:13:18Z

src/actions/actions.types.ts

@@ -56,10 +56,41 @@ export type ActionType =
  | "wrapWithPairedDelimiter"


This file is important

pokey · 2022-09-18T15:14:03Z

src/core/ThatMark.ts

@@ -1,10 +1,10 @@
-import { SelectionWithEditor } from "../typings/Types";
+import { Target } from "../typings/target.types";


Note that now the ThatMark class doesn't know anything about selections; felt specific to the CommandRunner

pokey · 2022-09-18T15:15:00Z

src/core/commandRunner/CommandRunner.ts

@@ -210,3 +220,20 @@ export default class CommandRunner {
    this.disposables.forEach(({ dispose }) => dispose());
  }
 }
+
+function constructThatTarget(


This is now where the logic lives for wrapping selections in a UntypedTarget. Seemed better than spreading it out

pokey · 2022-09-18T15:23:08Z

src/processTargets/marks/CursorStage.ts

@@ -1,9 +1,9 @@
-import { Target } from "../../typings/target.types";
+import type { Target } from "../../typings/target.types";


I went through and added type in a bunch of places

pokey · 2022-09-18T15:27:08Z

src/scripts/transformRecordedTests/index.ts


 const AVAILABLE_TRANSFORMATIONS: Record<string, FixtureTransformation> = {
  upgrade,
  autoFormat: identity,
-  // custom: MY_CUSTOM_TRANSFORMER,
+  custom: upgradeThatMarks,


we'll revert this file before merging; it just imports the transformation I used on test cases

pokey · 2022-09-18T15:28:04Z

src/scripts/transformRecordedTests/upgradeThatMarks.ts

@@ -0,0 +1,48 @@
+import { TestCaseFixture } from "../../testUtil/TestCase";


This file will be reverted. It transforms all that marks to be the UntypedTarget that they would have become via ThatMarkStage before.

pokey · 2022-09-18T15:29:10Z

src/test/suite/fixtures/recorded/actions/bringArgueOxAndZipToAfterJustLeper.yml

@@ -50,11 +50,14 @@ finalState:
    - anchor: {line: 3, character: 0}
      active: {line: 3, character: 0}
  thatMark:
-    - anchor: {line: 4, character: 15}
-      active: {line: 4, character: 39}
+    - contentRange:


Tons of test case changes; worth a quick look at a few to get a flavour

pokey · 2022-09-18T15:39:30Z

src/test/suite/recorded.test.ts

+    const initialThatTargets = fixture.initialState.thatMark.map((mark) =>
+      plainObjectToTarget(editor, mark)
+    );
+    cursorlessApi.thatMark.set(initialThatTargets);


Note how we now rehydrate the that mark to create a proper target

pokey · 2022-09-18T15:53:02Z

src/typings/Types.ts

+  thatMark: Target[];
+  sourceMark: Target[];


Note that these are rich targets now

phillco

lgtm ship it

pokey marked this pull request as draft September 14, 2022 09:49

pokey force-pushed the pokey/partial-smarter-that-marks branch from 2c5c603 to cee3583 Compare September 18, 2022 14:32

pokey changed the title ~~Smarter that mark for setSelection and highlight~~ Initial work for smarter that marks Sep 18, 2022

pokey force-pushed the pokey/partial-smarter-that-marks branch 6 times, most recently from 067accf to 38872e7 Compare September 18, 2022 15:53

pokey commented Sep 18, 2022

View reviewed changes

pokey marked this pull request as ready for review September 18, 2022 15:54

phillco previously approved these changes Sep 18, 2022

View reviewed changes

pokey dismissed phillco’s stale review via 70a0526 September 19, 2022 18:13

pokey force-pushed the pokey/partial-smarter-that-marks branch from 38872e7 to 70a0526 Compare September 19, 2022 18:13

pokey changed the base branch from main to pokey/remove-that-mark-from-non-action-tests September 19, 2022 18:14

pokey marked this pull request as draft September 20, 2022 09:44

pokey force-pushed the pokey/partial-smarter-that-marks branch from 70a0526 to bc1b679 Compare September 21, 2022 11:53

Base automatically changed from pokey/remove-that-mark-from-non-action-tests to main September 21, 2022 12:33

pokey force-pushed the pokey/partial-smarter-that-marks branch from bc1b679 to 7ac6044 Compare September 21, 2022 13:04

Initial work for smarter that marks

cdac276

pokey force-pushed the pokey/partial-smarter-that-marks branch from 7ac6044 to a1bcd50 Compare September 21, 2022 13:21

pokey added 3 commits September 21, 2022 14:27

No target serialization default attribute removal

6a4c42a

test transformer [to remove]

aa21ba5

Run transformation script on that and source marks

fd2b692

pokey force-pushed the pokey/partial-smarter-that-marks branch from a1bcd50 to fd2b692 Compare September 21, 2022 13:37

pokey requested a review from AndreasArvidsson September 21, 2022 13:40

pokey marked this pull request as ready for review September 21, 2022 13:40

AndreasArvidsson approved these changes Sep 21, 2022

View reviewed changes

AndreasArvidsson merged commit da757c2 into main Sep 21, 2022

AndreasArvidsson deleted the pokey/partial-smarter-that-marks branch September 21, 2022 13:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial work for smarter `that` marks #957

Initial work for smarter `that` marks #957

pokey commented Sep 13, 2022 •

edited

Loading

pokey Sep 18, 2022

pokey Sep 18, 2022

pokey Sep 18, 2022

pokey Sep 18, 2022

pokey Sep 18, 2022

pokey Sep 18, 2022

pokey Sep 18, 2022

pokey Sep 18, 2022

pokey Sep 18, 2022

pokey Sep 18, 2022

phillco left a comment •

edited

Loading

		@@ -56,10 +56,41 @@ export type ActionType =
		\| "wrapWithPairedDelimiter"

		@@ -1,10 +1,10 @@
		import { SelectionWithEditor } from "../typings/Types";
		import { Target } from "../typings/target.types";

		@@ -1,9 +1,9 @@
		import { Target } from "../../typings/target.types";
		import type { Target } from "../../typings/target.types";

		@@ -0,0 +1,48 @@
		import { TestCaseFixture } from "../../testUtil/TestCase";

Initial work for smarter that marks #957

Initial work for smarter that marks #957

Conversation

pokey commented Sep 13, 2022 • edited Loading

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phillco left a comment • edited Loading

Choose a reason for hiding this comment

Initial work for smarter `that` marks #957

Initial work for smarter `that` marks #957

pokey commented Sep 13, 2022 •

edited

Loading

phillco left a comment •

edited

Loading