Update token removal range to keep leading indentation

src/processTargets/targetUtil/insertionRemovalBehaviors/DelimitedSequenceInsertionRemovalBehavior.ts

@@ -5,15 +5,10 @@ import { Target } from "../../../typings/target.types";
  * Constructs a removal range for the given target that includes either the
  * trailing or leading delimiter
  * @param target The target to get the removal range for
- * @param contentRange Can be used to override the content range instead of
- * using the one on the target
  * @returns The removal range for the given target
  */
-export function getDelimitedSequenceRemovalRange(
-  target: Target,
-  contentRange?: Range
-): Range {
-  contentRange = contentRange ?? target.contentRange;
+export function getDelimitedSequenceRemovalRange(target: Target): Range {
+  const { contentRange } = target;
 
   const delimiterTarget =
     target.getTrailingDelimiterTarget() ?? target.getLeadingDelimiterTarget();

src/processTargets/targetUtil/insertionRemovalBehaviors/TokenInsertionRemovalBehavior.ts

@@ -1,9 +1,8 @@
-import { Range, TextDocument } from "vscode";
+import { Range, TextDocument, TextEditor } from "vscode";
 import { tokenize } from "../../../core/tokenizer";
 import type { Target } from "../../../typings/target.types";
-import { expandToFullLine } from "../../../util/rangeUtils";
+import { expandToFullLine, makeEmptyRange } from "../../../util/rangeUtils";
 import { PlainTarget } from "../../targets";
-import { getDelimitedSequenceRemovalRange } from "./DelimitedSequenceInsertionRemovalBehavior";
 
 export function getTokenLeadingDelimiterTarget(
   target: Target
@@ -59,45 +58,81 @@ export function getTokenTrailingDelimiterTarget(
  * removal range is designed to be used with things that should clean themselves
  * up as if they're a range of tokens.
  * @param target The target to get the token removal range for
- * @param contentRange Can be used to override the content range instead of
- * using the one on the target
  * @returns The removal range for the given target
  */
-export function getTokenRemovalRange(
-  target: Target,
-  contentRange?: Range
-): Range {
-  const { document } = target.editor;
-  const actualContentRange = contentRange ?? target.contentRange;
-  const removalRange = getDelimitedSequenceRemovalRange(target, contentRange);
-
-  if (!actualContentRange.isEqual(removalRange)) {
-    const fullRange = expandToFullLine(target.editor, actualContentRange);
-    const fullText = document.getText(fullRange);
-    const fullTextOffset = document.offsetAt(fullRange.start);
-
-    const numTokensContentRangeRemoved = calculateNumberOfTokensAfterRemoval(
-      document,
-      fullText,
-      fullTextOffset,
-      actualContentRange
-    );
-
-    const numTokensRemovalRangeRemoved = calculateNumberOfTokensAfterRemoval(
-      document,
-      fullText,
-      fullTextOffset,
-      removalRange
-    );
-
-    // Using removal range has not merged any tokens. Removal range is ok to use.
-    if (numTokensContentRangeRemoved === numTokensRemovalRangeRemoved) {
-      return removalRange;
+export function getTokenRemovalRange(target: Target): Range {
+  const { editor, contentRange } = target;
+  const { start, end } = contentRange;
+
+  const leadingWhitespaceRange =
+    target.getLeadingDelimiterTarget()?.contentRange ?? makeEmptyRange(start);
+
+  const trailingWhitespaceRange =
+    target.getTrailingDelimiterTarget()?.contentRange ?? makeEmptyRange(end);
+
+  const fullLineRange = expandToFullLine(editor, contentRange);
+
+  if (
+    leadingWhitespaceRange.union(trailingWhitespaceRange).isEqual(fullLineRange)
+  ) {
+    // If we would just be leaving a line with whitespace on it, we delete the
+    // whitespace
+    return fullLineRange;
+  }
+
+  if (!trailingWhitespaceRange.isEmpty) {
+    const candidateRemovalRange = contentRange.union(trailingWhitespaceRange);
+
+    if (!mergesTokens(editor, contentRange, candidateRemovalRange)) {
+      // If there is trailing whitespace and it doesn't result in tokens getting
+      // merged, then we remove it
+      return candidateRemovalRange;
     }
   }
 
-  // No removal range available or it would merge tokens.
-  return actualContentRange;
+  if (
+    !leadingWhitespaceRange.isEmpty &&
+    leadingWhitespaceRange.start.character !== 0
+  ) {
+    const candidateRemovalRange = leadingWhitespaceRange.union(contentRange);
+
+    if (!mergesTokens(editor, contentRange, candidateRemovalRange)) {
+      // If there is leading whitespace that is not indentation and it doesn't
+      // result in tokens getting merged, then we remove it
+      return candidateRemovalRange;
+    }
+  }
+
+  // Otherwise just return the content range
+  return contentRange;
+}
+
+/** Returns true if removal range causes tokens to merge */
+function mergesTokens(
+  editor: TextEditor,
+  contentRange: Range,
+  removalRange: Range
+) {
+  const { document } = editor;
+  const fullRange = expandToFullLine(editor, contentRange);
+  const fullText = document.getText(fullRange);
+  const fullTextOffset = document.offsetAt(fullRange.start);
+
+  const numTokensContentRangeRemoved = calculateNumberOfTokensAfterRemoval(
+    document,
+    fullText,
+    fullTextOffset,
+    contentRange
+  );
+
+  const numTokensRemovalRangeRemoved = calculateNumberOfTokensAfterRemoval(
+    document,
+    fullText,
+    fullTextOffset,
+    removalRange
+  );
+
+  return numTokensContentRangeRemoved !== numTokensRemovalRangeRemoved;
 }
 
 function calculateNumberOfTokensAfterRemoval(

src/processTargets/targets/ScopeTypeTarget.ts

@@ -90,7 +90,7 @@ export default class ScopeTypeTarget extends BaseTarget {
 
   getRemovalRange(): Range {
     return this.removalRange_ != null
-      ? getTokenRemovalRange(this, this.removalRange_)
+      ? this.removalRange_
       : this.hasDelimiterRange_
       ? getDelimitedSequenceRemovalRange(this)
       : getTokenRemovalRange(this);

src/test/suite/fixtures/recorded/scopes/token/ditchFine.yml

@@ -0,0 +1,26 @@
+languageId: plaintext
+command:
+  spokenForm: ditch fine
+  version: 3
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: f}
+  usePrePhraseSnapshot: true
+  action: {name: remove}
+initialState:
+  documentContents: |2
+        foo   
+  selections:
+    - anchor: {line: 1, character: 0}
+      active: {line: 1, character: 0}
+  marks:
+    default.f:
+      start: {line: 0, character: 4}
+      end: {line: 0, character: 7}
+finalState:
+  documentContents: |+
+
+  selections:
+    - anchor: {line: 1, character: 0}
+      active: {line: 1, character: 0}
+fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: f}, modifiers: []}]

src/test/suite/fixtures/recorded/scopes/token/ditchFinePastBat.yml

@@ -0,0 +1,37 @@
+languageId: plaintext
+command:
+  spokenForm: ditch fine past bat
+  version: 3
+  targets:
+    - type: range
+      anchor:
+        type: primitive
+        mark: {type: decoratedSymbol, symbolColor: default, character: f}
+      active:
+        type: primitive
+        mark: {type: decoratedSymbol, symbolColor: default, character: b}
+      excludeAnchor: false
+      excludeActive: false
+  usePrePhraseSnapshot: true
+  action: {name: remove}
+initialState:
+  documentContents: |2
+        foo
+        bar.
+  selections:
+    - anchor: {line: 2, character: 0}
+      active: {line: 2, character: 0}
+  marks:
+    default.f:
+      start: {line: 0, character: 4}
+      end: {line: 0, character: 7}
+    default.b:
+      start: {line: 1, character: 4}
+      end: {line: 1, character: 7}
+finalState:
+  documentContents: |2
+        .
+  selections:
+    - anchor: {line: 1, character: 0}
+      active: {line: 1, character: 0}
+fullTargets: [{type: range, excludeAnchor: false, excludeActive: false, rangeType: continuous, anchor: {type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: f}, modifiers: []}, active: {type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: b}, modifiers: []}}]

src/test/suite/fixtures/recorded/scopes/token/ditchFinePastPoint.yml

@@ -0,0 +1,37 @@
+languageId: plaintext
+command:
+  spokenForm: ditch fine past point
+  version: 3
+  targets:
+    - type: range
+      anchor:
+        type: primitive
+        mark: {type: decoratedSymbol, symbolColor: default, character: f}
+      active:
+        type: primitive
+        mark: {type: decoratedSymbol, symbolColor: default, character: .}
+      excludeAnchor: false
+      excludeActive: false
+  usePrePhraseSnapshot: true
+  action: {name: remove}
+initialState:
+  documentContents: |2
+       foo
+       bar.
+  selections:
+    - anchor: {line: 2, character: 0}
+      active: {line: 2, character: 0}
+  marks:
+    default.f:
+      start: {line: 0, character: 3}
+      end: {line: 0, character: 6}
+    default..:
+      start: {line: 1, character: 6}
+      end: {line: 1, character: 7}
+finalState:
+  documentContents: |+
+
+  selections:
+    - anchor: {line: 1, character: 0}
+      active: {line: 1, character: 0}
+fullTargets: [{type: range, excludeAnchor: false, excludeActive: false, rangeType: continuous, anchor: {type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: f}, modifiers: []}, active: {type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: .}, modifiers: []}}]

src/test/suite/fixtures/recorded/scopes/token/ditchFinePastPoint2.yml

@@ -0,0 +1,37 @@
+languageId: plaintext
+command:
+  spokenForm: ditch fine past point
+  version: 3
+  targets:
+    - type: range
+      anchor:
+        type: primitive
+        mark: {type: decoratedSymbol, symbolColor: default, character: f}
+      active:
+        type: primitive
+        mark: {type: decoratedSymbol, symbolColor: default, character: .}
+      excludeAnchor: false
+      excludeActive: false
+  usePrePhraseSnapshot: true
+  action: {name: remove}
+initialState:
+  documentContents: |2
+       foo
+       bar.   
+  selections:
+    - anchor: {line: 2, character: 0}
+      active: {line: 2, character: 0}
+  marks:
+    default.f:
+      start: {line: 0, character: 3}
+      end: {line: 0, character: 6}
+    default..:
+      start: {line: 1, character: 6}
+      end: {line: 1, character: 7}
+finalState:
+  documentContents: |+
+
+  selections:
+    - anchor: {line: 1, character: 0}
+      active: {line: 1, character: 0}
+fullTargets: [{type: range, excludeAnchor: false, excludeActive: false, rangeType: continuous, anchor: {type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: f}, modifiers: []}, active: {type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: .}, modifiers: []}}]

src/test/suite/fixtures/recorded/selectionTypes/chuckDot.yml

@@ -0,0 +1,24 @@
+languageId: plaintext
+command:
+  spokenForm: chuck dot
+  version: 3
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: .}
+  usePrePhraseSnapshot: true
+  action: {name: remove}
+initialState:
+  documentContents: "  .a"
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+  marks:
+    default..:
+      start: {line: 0, character: 2}
+      end: {line: 0, character: 3}
+finalState:
+  documentContents: "  a"
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: .}, modifiers: []}]

src/test/suite/fixtures/recorded/selectionTypes/chuckDot2.yml

@@ -0,0 +1,24 @@
+languageId: plaintext
+command:
+  spokenForm: chuck dot
+  version: 3
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: .}
+  usePrePhraseSnapshot: true
+  action: {name: remove}
+initialState:
+  documentContents: "  ."
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+  marks:
+    default..:
+      start: {line: 0, character: 2}
+      end: {line: 0, character: 3}
+finalState:
+  documentContents: ""
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: .}, modifiers: []}]

src/test/suite/fixtures/recorded/selectionTypes/chuckDot3.yml

@@ -0,0 +1,24 @@
+languageId: plaintext
+command:
+  spokenForm: chuck dot
+  version: 3
+  targets:
+    - type: primitive
+      mark: {type: decoratedSymbol, symbolColor: default, character: .}
+  usePrePhraseSnapshot: true
+  action: {name: remove}
+initialState:
+  documentContents: "  . a"
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+  marks:
+    default..:
+      start: {line: 0, character: 2}
+      end: {line: 0, character: 3}
+finalState:
+  documentContents: "  a"
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: .}, modifiers: []}]

src/util/nodeMatchers.ts

@@ -74,7 +74,7 @@ export function chainedMatcher(
  * them in reverse, walking up the ancestor chain from `node`.
  * Returns `null` if any finder in the chain returns null.  For example:
  *
- * ancestorChainNodeFinder(0, patternFinder("foo", "bar"), patternFinder("bongo"))
+ * ancestorChainNodeFinder([patternFinder("foo", "bar"), patternFinder("bongo")], 0)
  *
  * is equivalent to:
  *