Changed getRemovalHighlightRange to return a GeneralizedRange (#2784)

Fixes #1657

## Checklist

- [/] 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

Author: AndreasArvidsson

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

@@ -1,4 +1,9 @@
-import type { Range, Selection, TextEditor } from "@cursorless/common";
+import type {
+  GeneralizedRange,
+  Range,
+  Selection,
+  TextEditor,
+} from "@cursorless/common";
 import { FlashStyle, RangeExpansionBehavior } from "@cursorless/common";
 import { flatten } from "lodash-es";
 import type { RangeUpdater } from "../core/updateSelections/RangeUpdater";
@@ -8,8 +13,8 @@ import type { EditWithRangeUpdater } from "../typings/Types";
 import type { Destination, Target } from "../typings/target.types";
 import {
   flashTargets,
-  getContentRange,
   runForEachEditor,
+  toGeneralizedRange,
 } from "../util/targetUtils";
 import { unifyRemovalTargets } from "../util/unifyRanges";
 import type { ActionReturnValue } from "./actions.types";
@@ -34,7 +39,7 @@ abstract class BringMoveSwap {
   protected abstract decoration: {
     sourceStyle: FlashStyle;
     destinationStyle: FlashStyle;
-    getSourceRangeCallback: (target: Target) => Range;
+    getSourceRangeCallback?: (target: Target) => GeneralizedRange;
   };
 
   constructor(
@@ -209,8 +214,12 @@ abstract class BringMoveSwap {
   }
 
   protected async decorateThatMark(thatMark: MarkEntry[]) {
-    const getRange = (target: Target) =>
-      thatMark.find((t) => t.target === target)!.selection;
+    const getRange = (target: Target) => {
+      return toGeneralizedRange(
+        target,
+        thatMark.find((t) => t.target === target)!.selection,
+      );
+    };
     return Promise.all([
       flashTargets(
         ide(),
@@ -250,7 +259,6 @@ export class Bring extends BringMoveSwap {
   decoration = {
     sourceStyle: FlashStyle.referenced,
     destinationStyle: FlashStyle.pendingModification0,
-    getSourceRangeCallback: getContentRange,
   };
 
   constructor(rangeUpdater: RangeUpdater) {
@@ -320,7 +328,6 @@ export class Swap extends BringMoveSwap {
   decoration = {
     sourceStyle: FlashStyle.pendingModification1,
     destinationStyle: FlashStyle.pendingModification0,
-    getSourceRangeCallback: getContentRange,
   };
 
   constructor(rangeUpdater: RangeUpdater) {

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

@@ -29,12 +29,7 @@ export class CopyToClipboard implements SimpleAction {
     }
 
     if (options.showDecorations) {
-      await flashTargets(
-        ide(),
-        targets,
-        FlashStyle.referenced,
-        (target) => target.contentRange,
-      );
+      await flashTargets(ide(), targets, FlashStyle.referenced);
     }
 
     // FIXME: We should really keep track of the number of targets from the

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

@@ -1,57 +1,17 @@
-import type { FlashDescriptor } from "@cursorless/common";
-import {
-  FlashStyle,
-  Range,
-  toCharacterRange,
-  toLineRange,
-} from "@cursorless/common";
+import type { CharacterRange, FlashDescriptor } from "@cursorless/common";
+import { FlashStyle, Range, toCharacterRange } from "@cursorless/common";
 import { ide } from "../singletons/ide.singleton";
 import type { Target } from "../typings/target.types";
 import type { Actions } from "./Actions";
-import type { SimpleAction, ActionReturnValue } from "./actions.types";
+import type { ActionReturnValue, SimpleAction } from "./actions.types";
 
 export class CutToClipboard implements SimpleAction {
   constructor(private actions: Actions) {
     this.run = this.run.bind(this);
   }
 
   async run(targets: Target[]): Promise<ActionReturnValue> {
-    await ide().flashRanges(
-      targets.flatMap((target) => {
-        const { editor, contentRange } = target;
-        const removalHighlightRange = target.getRemovalHighlightRange();
-
-        if (target.isLine) {
-          return [
-            {
-              editor,
-              range: toCharacterRange(contentRange),
-              style: FlashStyle.referenced,
-            },
-            {
-              editor,
-              range: toLineRange(removalHighlightRange),
-              style: FlashStyle.pendingDelete,
-            },
-          ];
-        }
-
-        return [
-          {
-            editor,
-            range: toCharacterRange(contentRange),
-            style: FlashStyle.referenced,
-          },
-          ...getOutsideOverflow(contentRange, removalHighlightRange).map(
-            (overflow): FlashDescriptor => ({
-              editor,
-              range: toCharacterRange(overflow),
-              style: FlashStyle.pendingDelete,
-            }),
-          ),
-        ];
-      }),
-    );
+    await ide().flashRanges(targets.flatMap(getFlashDescriptors));
 
     const options = { showDecorations: false };
 
@@ -63,8 +23,44 @@ export class CutToClipboard implements SimpleAction {
   }
 }
 
+function getFlashDescriptors(target: Target): FlashDescriptor[] {
+  const { editor, contentRange } = target;
+  const removalHighlightRange = target.getRemovalHighlightRange();
+
+  const flashDescriptors: FlashDescriptor[] = [
+    {
+      editor,
+      range: toCharacterRange(contentRange),
+      style: FlashStyle.referenced,
+    },
+  ];
+
+  if (removalHighlightRange.type === "line") {
+    flashDescriptors.push({
+      editor,
+      range: removalHighlightRange,
+      style: FlashStyle.pendingDelete,
+    });
+  } else {
+    flashDescriptors.push(
+      ...getOutsideOverflow(contentRange, removalHighlightRange).map(
+        (overflow): FlashDescriptor => ({
+          editor,
+          range: toCharacterRange(overflow),
+          style: FlashStyle.pendingDelete,
+        }),
+      ),
+    );
+  }
+
+  return flashDescriptors;
+}
+
 /** Get the possible leading and trailing overflow ranges of the outside range compared to the inside range */
-function getOutsideOverflow(insideRange: Range, outsideRange: Range): Range[] {
+function getOutsideOverflow(
+  insideRange: Range,
+  outsideRange: CharacterRange,
+): Range[] {
   const { start: insideStart, end: insideEnd } = insideRange;
   const { start: outsideStart, end: outsideEnd } = outsideRange;
   const result = [];

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

@@ -33,7 +33,9 @@ export default class Highlight {
         ide().setHighlightRanges(
           highlightId,
           editor,
-          targets.map(toGeneralizedRange),
+          targets.map((target) =>
+            toGeneralizedRange(target, target.contentRange),
+          ),
         ),
       );
     }

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

@@ -25,7 +25,9 @@ export default class ToggleBreakpoint implements SimpleAction {
     await flashTargets(ide(), thatTargets, FlashStyle.referenced);
 
     await runOnTargetsForEachEditor(targets, async (editor, targets) => {
-      const generalizedRanges = targets.map(toGeneralizedRange);
+      const generalizedRanges = targets.map((target) =>
+        toGeneralizedRange(target, target.contentRange),
+      );
 
       await ide()
         .getEditableTextEditor(editor)

packages/cursorless-engine/src/processTargets/targets/BaseTarget.ts

@@ -1,5 +1,6 @@
 import type {
   EnforceUndefined,
+  GeneralizedRange,
   InsertionMode,
   Range,
   Selection,
@@ -14,6 +15,7 @@ import type {
   JoinAsType,
   Target,
 } from "../../typings/target.types";
+import { toGeneralizedRange } from "../../util/targetUtils";
 import { DestinationImpl } from "./DestinationImpl";
 import { createContinuousRange } from "./util/createContinuousRange";
 
@@ -97,8 +99,8 @@ export abstract class BaseTarget<
     };
   }
 
-  getRemovalHighlightRange(): Range {
-    return this.getRemovalRange();
+  getRemovalHighlightRange(): GeneralizedRange {
+    return toGeneralizedRange(this, this.getRemovalRange());
   }
 
   withThatTarget(thatTarget: Target): Target {

packages/cursorless-engine/src/processTargets/targets/BoundedParagraphTarget.ts

@@ -1,9 +1,9 @@
-import type { Range } from "@cursorless/common";
+import { toLineRange, type Range } from "@cursorless/common";
+import type { InteriorTarget, ParagraphTarget } from ".";
+import { expandToFullLine } from "../../util/rangeUtils";
 import type { MinimumTargetParameters } from "./BaseTarget";
 import { BaseTarget } from "./BaseTarget";
 import { LineTarget } from "./LineTarget";
-import { expandToFullLine } from "../../util/rangeUtils";
-import type { InteriorTarget, ParagraphTarget } from ".";
 
 interface BoundedParagraphTargetParameters extends MinimumTargetParameters {
   readonly paragraphTarget: ParagraphTarget;
@@ -86,16 +86,20 @@ export class BoundedParagraphTarget extends BaseTarget<BoundedParagraphTargetPar
   }
 
   getRemovalHighlightRange() {
-    if (this.startLineGap < 1 || this.endLineGap < 1) {
-      return this.getRemovalRange();
-    }
+    const range = (() => {
+      if (this.startLineGap < 1 || this.endLineGap < 1) {
+        return this.getRemovalRange();
+      }
 
-    const delimiterTarget =
-      this.getTrailingDelimiterTarget() ?? this.getLeadingDelimiterTarget();
+      const delimiterTarget =
+        this.getTrailingDelimiterTarget() ?? this.getLeadingDelimiterTarget();
+
+      return delimiterTarget != null
+        ? this.fullLineContentRange.union(delimiterTarget.contentRange)
+        : this.fullLineContentRange;
+    })();
 
-    return delimiterTarget != null
-      ? this.fullLineContentRange.union(delimiterTarget.contentRange)
-      : this.fullLineContentRange;
+    return toLineRange(range);
   }
 
   maybeCreateRichRangeTarget(

packages/cursorless-engine/src/processTargets/targets/LineTarget.ts

@@ -1,11 +1,11 @@
 import type { TextEditor } from "@cursorless/common";
-import { Position, Range } from "@cursorless/common";
+import { Position, Range, toLineRange } from "@cursorless/common";
+import { expandToFullLine } from "../../util/rangeUtils";
+import { tryConstructTarget } from "../../util/tryConstructTarget";
 import type { CommonTargetParameters } from "./BaseTarget";
 import { BaseTarget } from "./BaseTarget";
-import { expandToFullLine } from "../../util/rangeUtils";
 import { tryConstructPlainTarget } from "./PlainTarget";
 import { createContinuousLineRange } from "./util/createContinuousRange";
-import { tryConstructTarget } from "../../util/tryConstructTarget";
 
 export class LineTarget extends BaseTarget<CommonTargetParameters> {
   type = "LineTarget";
@@ -42,7 +42,9 @@ export class LineTarget extends BaseTarget<CommonTargetParameters> {
       : contentRemovalRange.union(delimiterTarget.contentRange);
   }
 
-  getRemovalHighlightRange = () => this.fullLineContentRange;
+  getRemovalHighlightRange = () => {
+    return toLineRange(this.fullLineContentRange);
+  };
 
   maybeCreateRichRangeTarget(
     isReversed: boolean,

packages/cursorless-engine/src/processTargets/targets/ParagraphTarget.ts

@@ -1,10 +1,14 @@
-import type { TextDocument, TextEditor, TextLine } from "@cursorless/common";
-import { Position, Range } from "@cursorless/common";
+import type {
+  GeneralizedRange,
+  TextDocument,
+  TextEditor,
+  TextLine,
+} from "@cursorless/common";
+import { Position, Range, toLineRange } from "@cursorless/common";
+import { expandToFullLine } from "../../util/rangeUtils";
 import type { CommonTargetParameters } from "./BaseTarget";
 import { BaseTarget } from "./BaseTarget";
-import { LineTarget } from "./LineTarget";
-import { expandToFullLine } from "../../util/rangeUtils";
-import { constructLineTarget } from "./LineTarget";
+import { constructLineTarget, LineTarget } from "./LineTarget";
 import { createContinuousLineRange } from "./util/createContinuousRange";
 
 export class ParagraphTarget extends BaseTarget<CommonTargetParameters> {
@@ -59,13 +63,16 @@ export class ParagraphTarget extends BaseTarget<CommonTargetParameters> {
     return expandToFullLine(this.editor, this.contentRange);
   }
 
-  getRemovalHighlightRange() {
+  getRemovalHighlightRange(): GeneralizedRange {
     const delimiterTarget =
       this.getTrailingDelimiterTarget() ?? this.getLeadingDelimiterTarget();
 
-    return delimiterTarget != null
-      ? this.fullLineContentRange.union(delimiterTarget.contentRange)
-      : this.fullLineContentRange;
+    const range =
+      delimiterTarget != null
+        ? this.fullLineContentRange.union(delimiterTarget.contentRange)
+        : this.fullLineContentRange;
+
+    return toLineRange(range);
   }
 
   maybeCreateRichRangeTarget(

packages/cursorless-engine/src/scopeProviders/getTargetRanges.ts

@@ -1,14 +1,11 @@
 import type { TargetRanges } from "@cursorless/common";
-import { toCharacterRange, toLineRange } from "@cursorless/common";
 import type { Target } from "../typings/target.types";
 
 export function getTargetRanges(target: Target): TargetRanges {
   return {
     contentRange: target.contentRange,
     removalRange: target.getRemovalRange(),
-    removalHighlightRange: target.isLine
-      ? toLineRange(target.getRemovalHighlightRange())
-      : toCharacterRange(target.getRemovalHighlightRange()),
+    removalHighlightRange: target.getRemovalHighlightRange(),
     leadingDelimiter: getOptionalTarget(target.getLeadingDelimiterTarget()),
     trailingDelimiter: getOptionalTarget(target.getTrailingDelimiterTarget()),
     interior: target.getInterior()?.map(getTargetRanges),