Added the ability to remove whitespaces with join token (#2651)

AndreasArvidsson · phillco · web-flow · commit 9cf3a1bb366a · 2024-08-11T21:50:08.000Z
## 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: Phil Cohen <phillip@phillip.io>
diff --git a/changelog/2024-08-joinTokens.md b/changelog/2024-08-joinTokens.md
@@ -0,0 +1,6 @@
+---
+tags: [enhancement]
+pullRequest: 2651
+---
+
+`join token` now behaves as expected. `"join two tokens air"` will now remove the whitespace between the tokens; previously, it simply joined together their lines, which was usually not expected.
diff --git a/data/fixtures/recorded/actions/joinBatPastEach.yml b/data/fixtures/recorded/actions/joinBatPastEach.yml
@@ -0,0 +1,43 @@
+languageId: plaintext
+command:
+  version: 7
+  spokenForm: join bat past each
+  action:
+    name: joinLines
+    target:
+      type: range
+      anchor:
+        type: primitive
+        mark: {type: decoratedSymbol, symbolColor: default, character: b}
+      active:
+        type: primitive
+        mark: {type: decoratedSymbol, symbolColor: default, character: e}
+      excludeAnchor: false
+      excludeActive: false
+  usePrePhraseSnapshot: true
+initialState:
+  documentContents: |-
+    aa bb cc
+    dd ee ff
+  selections:
+    - anchor: {line: 0, character: 4}
+      active: {line: 0, character: 4}
+  marks:
+    default.b:
+      start: {line: 0, character: 3}
+      end: {line: 0, character: 5}
+    default.e:
+      start: {line: 1, character: 3}
+      end: {line: 1, character: 5}
+finalState:
+  documentContents: aa bb cc dd ee ff
+  selections:
+    - anchor: {line: 0, character: 4}
+      active: {line: 0, character: 4}
+  thatMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 0}
+        end: {line: 0, character: 17}
+      isReversed: false
+      hasExplicitRange: true
diff --git a/data/fixtures/recorded/actions/joinThreeTokens.yml b/data/fixtures/recorded/actions/joinThreeTokens.yml
@@ -0,0 +1,37 @@
+languageId: plaintext
+command:
+  version: 7
+  spokenForm: join three tokens
+  action:
+    name: joinLines
+    target:
+      type: primitive
+      modifiers:
+        - type: relativeScope
+          scopeType: {type: token}
+          offset: 0
+          length: 3
+          direction: forward
+  usePrePhraseSnapshot: true
+initialState:
+  documentContents: |-
+    aa bb cc
+    dd ee ff
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+  marks: {}
+finalState:
+  documentContents: |-
+    aabbcc
+    dd ee ff
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+  thatMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 0}
+        end: {line: 0, character: 6}
+      isReversed: false
+      hasExplicitRange: true
diff --git a/data/fixtures/recorded/actions/joinTokenBattPastEach.yml b/data/fixtures/recorded/actions/joinTokenBattPastEach.yml
@@ -0,0 +1,47 @@
+languageId: plaintext
+command:
+  version: 7
+  spokenForm: join token batt past each
+  action:
+    name: joinLines
+    target:
+      type: range
+      anchor:
+        type: primitive
+        mark: {type: decoratedSymbol, symbolColor: default, character: b}
+        modifiers:
+          - type: preferredScope
+            scopeType: {type: token}
+      active:
+        type: primitive
+        mark: {type: decoratedSymbol, symbolColor: default, character: e}
+      excludeAnchor: false
+      excludeActive: false
+  usePrePhraseSnapshot: true
+spokenFormError: Modifier 'preferredScope'
+initialState:
+  documentContents: |-
+    aa bb cc
+    dd ee ff
+  selections:
+    - anchor: {line: 0, character: 4}
+      active: {line: 0, character: 4}
+  marks:
+    default.b:
+      start: {line: 0, character: 3}
+      end: {line: 0, character: 5}
+    default.e:
+      start: {line: 1, character: 3}
+      end: {line: 1, character: 5}
+finalState:
+  documentContents: aa bbccddee ff
+  selections:
+    - anchor: {line: 0, character: 4}
+      active: {line: 0, character: 4}
+  thatMark:
+    - type: UntypedTarget
+      contentRange:
+        start: {line: 0, character: 3}
+        end: {line: 0, character: 11}
+      isReversed: false
+      hasExplicitRange: true
diff --git a/packages/cursorless-engine/src/actions/Actions.ts b/packages/cursorless-engine/src/actions/Actions.ts
@@ -117,7 +117,7 @@ export class Actions implements ActionRecord {
     this,
     this.modifierStageFactory,
   );
-  joinLines = new JoinLines(this.rangeUpdater);
+  joinLines = new JoinLines(this.rangeUpdater, this.modifierStageFactory);
   breakLine = new BreakLine(this.rangeUpdater);
   moveToTarget = new Move(this.rangeUpdater);
   outdentLine = new OutdentLine(this.rangeUpdater);
diff --git a/packages/cursorless-engine/src/actions/JoinLines.ts b/packages/cursorless-engine/src/actions/JoinLines.ts
@@ -1,39 +1,53 @@
 import type { Edit, TextEditor } from "@cursorless/common";
-import { FlashStyle, Range } from "@cursorless/common";
+import { FlashStyle, Range, zipStrict } from "@cursorless/common";
 import { range as iterRange, map, pairwise } from "itertools";
-import { flatten, zip } from "lodash-es";
+import { flatten } from "lodash-es";
 import type { RangeUpdater } from "../core/updateSelections/RangeUpdater";
 import { performEditsAndUpdateSelections } from "../core/updateSelections/updateSelections";
+import { containingLineIfUntypedModifier } from "../processTargets/modifiers/commonContainingScopeIfUntypedModifiers";
+import type { ModifierStageFactory } from "../processTargets/ModifierStageFactory";
+import type { ModifierStage } from "../processTargets/PipelineStages.types";
 import { ide } from "../singletons/ide.singleton";
+import { getMatcher } from "../tokenizer";
 import type { Target } from "../typings/target.types";
+import { generateMatchesInRange } from "../util/getMatchesInRange";
 import { flashTargets, runOnTargetsForEachEditor } from "../util/targetUtils";
 import type { ActionReturnValue } from "./actions.types";
 
 export default class JoinLines {
-  constructor(private rangeUpdater: RangeUpdater) {
+  getFinalStages(): ModifierStage[] {
+    return [this.modifierStageFactory.create(containingLineIfUntypedModifier)];
+  }
+
+  constructor(
+    private rangeUpdater: RangeUpdater,
+    private modifierStageFactory: ModifierStageFactory,
+  ) {
     this.run = this.run.bind(this);
   }
 
   async run(targets: Target[]): Promise<ActionReturnValue> {
-    await flashTargets(ide(), targets, FlashStyle.pendingModification0);
+    await flashTargets(
+      ide(),
+      targets.map(({ thatTarget }) => thatTarget),
+      FlashStyle.pendingModification0,
+    );
 
     const thatSelections = flatten(
       await runOnTargetsForEachEditor(targets, async (editor, targets) => {
-        const contentRanges = targets.map(({ contentRange }) => contentRange);
-
-        const { contentRanges: updatedRanges } =
+        const { thatRanges: updatedThatRanges } =
           await performEditsAndUpdateSelections({
             rangeUpdater: this.rangeUpdater,
             editor: ide().getEditableTextEditor(editor),
-            edits: getEdits(editor, contentRanges),
+            edits: getEdits(editor, targets),
             selections: {
-              contentRanges,
+              thatRanges: targets.map(({ contentRange }) => contentRange),
             },
           });
 
-        return zip(targets, updatedRanges).map(([target, range]) => ({
-          editor: target!.editor,
-          selection: range!.toSelection(target!.isReversed),
+        return zipStrict(targets, updatedThatRanges).map(([target, range]) => ({
+          editor,
+          selection: range.toSelection(target.isReversed),
         }));
       }),
     );
@@ -42,28 +56,60 @@ export default class JoinLines {
   }
 }
 
-function getEdits(editor: TextEditor, contentRanges: Range[]): Edit[] {
-  const { document } = editor;
+function getEdits(editor: TextEditor, targets: Target[]): Edit[] {
   const edits: Edit[] = [];
 
-  for (const range of contentRanges) {
-    const startLine = range.start.line;
-    const endLine = range.isSingleLine ? startLine + 1 : range.end.line;
+  for (const target of targets) {
+    const targetsEdits =
+      target.joinAs === "line"
+        ? getLineTargetEdits(target)
+        : getTokenTargetEdits(target);
 
-    const lineIter = map(iterRange(startLine, endLine + 1), (i) =>
-      document.lineAt(i),
-    );
-    for (const [line1, line2] of pairwise(lineIter)) {
-      edits.push({
-        range: new Range(
-          line1.rangeTrimmed?.end ?? line1.range.end,
-          line2.rangeTrimmed?.start ?? line2.range.start,
-        ),
-        text: line2.isEmptyOrWhitespace ? "" : " ",
-        isReplace: true,
-      });
-    }
+    edits.push(...targetsEdits);
   }
 
   return edits;
 }
+
+function getTokenTargetEdits(target: Target): Edit[] {
+  const { editor, contentRange } = target;
+  const regex = getMatcher(editor.document.languageId).tokenMatcher;
+  const matches = generateMatchesInRange(
+    regex,
+    editor,
+    contentRange,
+    "forward",
+  );
+
+  return Array.from(pairwise(matches)).map(
+    ([range1, range2]): Edit => ({
+      range: new Range(range1.end, range2.start),
+      text: "",
+      isReplace: true,
+    }),
+  );
+}
+
+function getLineTargetEdits(target: Target): Edit[] {
+  const { document } = target.editor;
+  const range = target.contentRange;
+  const startLine = range.start.line;
+  const endLine = range.isSingleLine
+    ? Math.min(startLine + 1, document.lineCount - 1)
+    : range.end.line;
+
+  const lines = map(iterRange(startLine, endLine + 1), (i) =>
+    document.lineAt(i),
+  );
+
+  return Array.from(pairwise(lines)).map(
+    ([line1, line2]): Edit => ({
+      range: new Range(
+        line1.rangeTrimmed?.end ?? line1.range.end,
+        line2.rangeTrimmed?.start ?? line2.range.start,
+      ),
+      text: line2.isEmptyOrWhitespace ? "" : " ",
+      isReplace: true,
+    }),
+  );
+}
diff --git a/packages/cursorless-engine/src/processTargets/targets/BaseTarget.ts b/packages/cursorless-engine/src/processTargets/targets/BaseTarget.ts
@@ -1,15 +1,19 @@
 import type {
   EnforceUndefined,
   InsertionMode,
-  TargetPlainObject,
   Range,
   Selection,
+  TargetPlainObject,
   TextEditor,
 } from "@cursorless/common";
 import { rangeToPlainObject } from "@cursorless/common";
 import { isEqual } from "lodash-es";
 import type { EditWithRangeUpdater } from "../../typings/Types";
-import type { Destination, Target } from "../../typings/target.types";
+import type {
+  Destination,
+  JoinAsType,
+  Target,
+} from "../../typings/target.types";
 import { DestinationImpl } from "./DestinationImpl";
 import { createContinuousRange } from "./util/createContinuousRange";
 
@@ -49,6 +53,7 @@ export abstract class BaseTarget<
   isImplicit = false;
   isNotebookCell = false;
   isWord = false;
+  joinAs: JoinAsType = "line";
 
   constructor(parameters: TParameters & CommonTargetParameters) {
     this.state = {
diff --git a/packages/cursorless-engine/src/processTargets/targets/TokenTarget.ts b/packages/cursorless-engine/src/processTargets/targets/TokenTarget.ts
@@ -1,7 +1,7 @@
 import type { Range } from "@cursorless/common";
+import type { JoinAsType, Target } from "../../typings/target.types";
 import type { CommonTargetParameters } from "./BaseTarget";
 import { BaseTarget } from "./BaseTarget";
-import type { Target } from "../../typings/target.types";
 import {
   getTokenLeadingDelimiterTarget,
   getTokenRemovalRange,
@@ -11,6 +11,7 @@ import {
 export class TokenTarget extends BaseTarget<CommonTargetParameters> {
   type = "TokenTarget";
   insertionDelimiter = " ";
+  joinAs: JoinAsType = "token";
 
   getLeadingDelimiterTarget(): Target | undefined {
     return getTokenLeadingDelimiterTarget(this);
diff --git a/packages/cursorless-engine/src/typings/target.types.ts b/packages/cursorless-engine/src/typings/target.types.ts
@@ -28,6 +28,7 @@ import type {
 import type { EditWithRangeUpdater } from "./Types";
 
 export type EditNewActionType = "edit" | "insertLineAfter";
+export type JoinAsType = "line" | "token";
 
 export interface Target {
   /** The text editor used for all ranges */
@@ -54,6 +55,9 @@ export interface Target {
   /** If true this target should be treated as a word */
   readonly isWord: boolean;
 
+  /** Specifies how a target should be joined */
+  readonly joinAs: JoinAsType;
+
   /**
    * If `true`, then this target has an explicit scope type, and so should never
    * be automatically expanded to a containing scope.
