Changed expansion behavior for bring source targets to closedClosed (#1576)

Fixes #476

## Checklist

- [x] 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: Pokey Rule <[email protected]>

Author: AndreasArvidsson

packages/cursorless-engine/src/actions/BringMoveSwap.ts

@@ -171,7 +171,29 @@ class BringMoveSwap implements Action {
               ? edits
               : edits.filter(({ isSource }) => !isSource);
 
-          const editSelectionInfos = edits.map(
+          const sourceEdits =
+            this.type === "swap"
+              ? []
+              : edits.filter(({ isSource }) => isSource);
+          const destinationEdits =
+            this.type === "swap"
+              ? edits
+              : edits.filter(({ isSource }) => !isSource);
+
+          // Sources should be closedClosed, because they should be logically
+          // the same as the original source.
+          const sourceEditSelectionInfos = sourceEdits.map(
+            ({ edit: { range }, originalTarget }) =>
+              getSelectionInfo(
+                editor.document,
+                range.toSelection(originalTarget.isReversed),
+                RangeExpansionBehavior.closedClosed,
+              ),
+          );
+
+          // Destinations should be openOpen, because they should grow to contain
+          // the new text.
+          const destinationEditSelectionInfos = destinationEdits.map(
             ({ edit: { range }, originalTarget }) =>
               getSelectionInfo(
                 editor.document,
@@ -190,35 +212,64 @@ class BringMoveSwap implements Action {
 
           const editableEditor = ide().getEditableTextEditor(editor);
 
-          const [updatedEditSelections, cursorSelections]: Selection[][] =
-            await performEditsAndUpdateFullSelectionInfos(
-              this.rangeUpdater,
-              editableEditor,
-              filteredEdits.map(({ edit }) => edit),
-              [editSelectionInfos, cursorSelectionInfos],
-            );
+          const [
+            updatedSourceEditSelections,
+            updatedDestinationEditSelections,
+            cursorSelections,
+          ]: Selection[][] = await performEditsAndUpdateFullSelectionInfos(
+            this.rangeUpdater,
+            editableEditor,
+            filteredEdits.map(({ edit }) => edit),
+            [
+              sourceEditSelectionInfos,
+              destinationEditSelectionInfos,
+              cursorSelectionInfos,
+            ],
+          );
 
           // NB: We set the selections here because we don't trust vscode to
           // properly move the cursor on a bring. Sometimes it will smear an
           // empty selection
           setSelectionsWithoutFocusingEditor(editableEditor, cursorSelections);
 
-          return edits.map((edit, index): MarkEntry => {
-            const selection = updatedEditSelections[index];
-            const range = edit.edit.updateRange(selection);
-            const target = edit.originalTarget;
-            return {
-              editor,
-              selection: range.toSelection(target.isReversed),
-              isSource: edit.isSource,
-              target,
-            };
-          });
+          const marks = [
+            ...this.getMarks(sourceEdits, updatedSourceEditSelections),
+            ...this.getMarks(
+              destinationEdits,
+              updatedDestinationEditSelections,
+            ),
+          ];
+
+          // Restore original order before split into source and destination
+          marks.sort(
+            (a, b) =>
+              edits.findIndex((e) => e.originalTarget === a.target) -
+              edits.findIndex((e) => e.originalTarget === b.target),
+          );
+
+          return marks;
         },
       ),
     );
   }
 
+  private getMarks(
+    edits: ExtendedEdit[],
+    selections: Selection[],
+  ): MarkEntry[] {
+    return edits.map((edit, index): MarkEntry => {
+      const selection = selections[index];
+      const range = edit.edit.updateRange(selection);
+      const target = edit.originalTarget;
+      return {
+        editor: edit.editor,
+        selection: range.toSelection(target.isReversed),
+        isSource: edit.isSource,
+        target,
+      };
+    });
+  }
+
   private async decorateThatMark(thatMark: MarkEntry[]) {
     const decorationContext = this.getDecorationContext();
     const getRange = (target: Target) =>

packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/actions/bringAirAfterAir.yml

@@ -0,0 +1,41 @@
+languageId: plaintext
+command:
+  version: 5
+  spokenForm: bring air after air
+  action: {name: replaceWithTarget}
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: a}
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: a}
+      modifiers:
+        - {type: position, position: after}
+  usePrePhraseSnapshot: true
+initialState:
+  documentContents: a
+  selections:
+    - anchor: {line: 0, character: 1}
+      active: {line: 0, character: 1}
+  marks:
+    default.a:
+      start: {line: 0, character: 0}
+      end: {line: 0, character: 1}
+finalState:
+  documentContents: a a
+  selections:
+    - anchor: {line: 0, character: 1}
+      active: {line: 0, character: 1}
+  thatMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 2}
+        end: {line: 0, character: 3}
+      isReversed: false
+      hasExplicitRange: true
+  sourceMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 0}
+        end: {line: 0, character: 1}
+      isReversed: false
+      hasExplicitRange: true

packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/actions/bringAirBeforeAir.yml

@@ -0,0 +1,41 @@
+languageId: plaintext
+command:
+  version: 5
+  spokenForm: bring air before air
+  action: {name: replaceWithTarget}
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: a}
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: a}
+      modifiers:
+        - {type: position, position: before}
+  usePrePhraseSnapshot: true
+initialState:
+  documentContents: a
+  selections:
+    - anchor: {line: 0, character: 1}
+      active: {line: 0, character: 1}
+  marks:
+    default.a:
+      start: {line: 0, character: 0}
+      end: {line: 0, character: 1}
+finalState:
+  documentContents: a a
+  selections:
+    - anchor: {line: 0, character: 3}
+      active: {line: 0, character: 3}
+  thatMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 0}
+        end: {line: 0, character: 1}
+      isReversed: false
+      hasExplicitRange: true
+  sourceMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 2}
+        end: {line: 0, character: 3}
+      isReversed: false
+      hasExplicitRange: true

packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/actions/bringAirToEndOfAir.yml

@@ -0,0 +1,41 @@
+languageId: plaintext
+command:
+  version: 5
+  spokenForm: bring air to end of air
+  action: {name: replaceWithTarget}
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: a}
+    - type: primitive
+      modifiers:
+        - {type: position, position: end}
+      mark: {type: decoratedSymbol, symbolColor: default, character: a}
+  usePrePhraseSnapshot: true
+initialState:
+  documentContents: a
+  selections:
+    - anchor: {line: 0, character: 1}
+      active: {line: 0, character: 1}
+  marks:
+    default.a:
+      start: {line: 0, character: 0}
+      end: {line: 0, character: 1}
+finalState:
+  documentContents: aa
+  selections:
+    - anchor: {line: 0, character: 2}
+      active: {line: 0, character: 2}
+  thatMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 1}
+        end: {line: 0, character: 2}
+      isReversed: false
+      hasExplicitRange: true
+  sourceMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 0}
+        end: {line: 0, character: 1}
+      isReversed: false
+      hasExplicitRange: true

packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/actions/bringAirToStartOfAir.yml

@@ -0,0 +1,41 @@
+languageId: plaintext
+command:
+  version: 5
+  spokenForm: bring air to start of air
+  action: {name: replaceWithTarget}
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: a}
+    - type: primitive
+      modifiers:
+        - {type: position, position: start}
+      mark: {type: decoratedSymbol, symbolColor: default, character: a}
+  usePrePhraseSnapshot: true
+initialState:
+  documentContents: a
+  selections:
+    - anchor: {line: 0, character: 1}
+      active: {line: 0, character: 1}
+  marks:
+    default.a:
+      start: {line: 0, character: 0}
+      end: {line: 0, character: 1}
+finalState:
+  documentContents: aa
+  selections:
+    - anchor: {line: 0, character: 2}
+      active: {line: 0, character: 2}
+  thatMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 0}
+        end: {line: 0, character: 1}
+      isReversed: false
+      hasExplicitRange: true
+  sourceMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 1}
+        end: {line: 0, character: 2}
+      isReversed: false
+      hasExplicitRange: true