Don't discard everyScope selections for multiple targets (#2893)

Unfortunately became quite a lot of changes to pass the modifier
options, but I think this is the only way. It also opens up for more
context aware modifiers in the future.

Fixes #2889

Author: AndreasArvidsson

data/fixtures/recorded/everyScope/takeEveryToken.yml

@@ -0,0 +1,33 @@
+languageId: plaintext
+command:
+  version: 7
+  spokenForm: take every token
+  action:
+    name: setSelection
+    target:
+      type: primitive
+      modifiers:
+        - type: everyScope
+          scopeType: {type: token}
+  usePrePhraseSnapshot: false
+initialState:
+  documentContents: |-
+    aaa bbb ccc
+    ddd eee
+  selections:
+    - anchor: {line: 0, character: 4}
+      active: {line: 0, character: 11}
+    - anchor: {line: 1, character: 4}
+      active: {line: 1, character: 7}
+  marks: {}
+finalState:
+  documentContents: |-
+    aaa bbb ccc
+    ddd eee
+  selections:
+    - anchor: {line: 0, character: 4}
+      active: {line: 0, character: 7}
+    - anchor: {line: 0, character: 8}
+      active: {line: 0, character: 11}
+    - anchor: {line: 1, character: 4}
+      active: {line: 1, character: 7}

packages/cursorless-engine/src/actions/snippetsLegacy/snippet.ts

@@ -2,14 +2,15 @@ import type {
   SimpleScopeTypeType,
   SnippetDefinition,
 } from "@cursorless/common";
+import type { ModifierStageFactory } from "../../processTargets/ModifierStageFactory";
+import type { ModifierStateOptions } from "../../processTargets/PipelineStages.types";
 import {
   Placeholder,
   Text,
   Variable,
   type TextmateSnippet,
 } from "../../snippets/vendor/vscodeSnippet/snippetParser";
 import { KnownSnippetVariableNames } from "../../snippets/vendor/vscodeSnippet/snippetVariables";
-import type { ModifierStageFactory } from "../../processTargets/ModifierStageFactory";
 import type { Target } from "../../typings/target.types";
 
 /**
@@ -138,6 +139,10 @@ function findMatchingSnippetDefinitionForSingleTarget(
 ): number {
   const languageId = target.editor.document.languageId;
 
+  const options: ModifierStateOptions = {
+    multipleTargets: false,
+  };
+
   // We want to find the first definition that matches the given context.
   // Note that we just use the first match we find because the definitions are
   // guaranteed to come sorted in precedence order.
@@ -165,7 +170,7 @@ function findMatchingSnippetDefinitionForSingleTarget(
               type: "containingScope",
               scopeType: { type: scopeTypeType },
             })
-            .run(target)[0];
+            .run(target, options)[0];
 
           if (target.contentRange.isRangeEqual(containingTarget.contentRange)) {
             // Skip this scope if the target is exactly the same as the
@@ -177,7 +182,7 @@ function findMatchingSnippetDefinitionForSingleTarget(
                 scopeType: { type: scopeTypeType },
                 ancestorIndex: 1,
               })
-              .run(target)[0];
+              .run(target, options)[0];
           }
 
           if (

packages/cursorless-engine/src/processTargets/PipelineStages.types.ts

@@ -4,6 +4,10 @@ export interface MarkStage {
   run(): Target[];
 }
 
+export interface ModifierStateOptions {
+  multipleTargets: boolean;
+}
+
 export interface ModifierStage {
-  run(target: Target): Target[];
+  run(target: Target, options: ModifierStateOptions): Target[];
 }

packages/cursorless-engine/src/processTargets/TargetPipelineRunner.ts

@@ -14,7 +14,11 @@ import type {
 import type { Target } from "../typings/target.types";
 import type { MarkStageFactory } from "./MarkStageFactory";
 import type { ModifierStageFactory } from "./ModifierStageFactory";
-import type { MarkStage, ModifierStage } from "./PipelineStages.types";
+import type {
+  MarkStage,
+  ModifierStage,
+  ModifierStateOptions,
+} from "./PipelineStages.types";
 import { createContinuousRangeTarget } from "./createContinuousRangeTarget";
 import { ImplicitStage } from "./marks/ImplicitStage";
 import { ContainingTokenIfUntypedEmptyStage } from "./modifiers/ConditionalModifierStages";
@@ -296,7 +300,10 @@ export function processModifierStages(
   // one-by-one and concatenating the results before passing them on to the
   // next stage.
   modifierStages.forEach((stage) => {
-    targets = targets.flatMap((target) => stage.run(target));
+    const options: ModifierStateOptions = {
+      multipleTargets: targets.length > 1,
+    };
+    targets = targets.flatMap((target) => stage.run(target, options));
   });
 
   // Then return the output from the final stage
@@ -309,6 +316,9 @@ function getExcludedScope(
   scopeType: ScopeType,
   direction: Direction,
 ): Target {
+  const options: ModifierStateOptions = {
+    multipleTargets: false,
+  };
   return (
     modifierStageFactory
       .create({
@@ -321,7 +331,7 @@ function getExcludedScope(
       // NB: The following line assumes that content range is always
       // contained by domain, so that "every" will properly reconstruct
       // the target from the content range.
-      .run(target)[0]
+      .run(target, options)[0]
   );
 }
 

packages/cursorless-engine/src/processTargets/modifiers/CascadingStage.ts

@@ -1,7 +1,10 @@
 import type { CascadingModifier } from "@cursorless/common";
 import type { Target } from "../../typings/target.types";
 import type { ModifierStageFactory } from "../ModifierStageFactory";
-import type { ModifierStage } from "../PipelineStages.types";
+import type {
+  ModifierStage,
+  ModifierStateOptions,
+} from "../PipelineStages.types";
 
 /**
  * Tries each of the given modifiers in turn until one of them doesn't throw an
@@ -25,10 +28,10 @@ export class CascadingStage implements ModifierStage {
     return this.nestedStages_;
   }
 
-  run(target: Target): Target[] {
+  run(target: Target, options: ModifierStateOptions): Target[] {
     for (const nestedStage of this.nestedStages) {
       try {
-        return nestedStage.run(target);
+        return nestedStage.run(target, options);
       } catch (_error) {
         continue;
       }

packages/cursorless-engine/src/processTargets/modifiers/ConditionalModifierStages.ts

@@ -1,7 +1,10 @@
 import type { Modifier, ModifyIfUntypedModifier } from "@cursorless/common";
 import type { Target } from "../../typings/target.types";
 import type { ModifierStageFactory } from "../ModifierStageFactory";
-import type { ModifierStage } from "../PipelineStages.types";
+import type {
+  ModifierStage,
+  ModifierStateOptions,
+} from "../PipelineStages.types";
 
 abstract class ConditionalModifierBaseStage implements ModifierStage {
   private nestedStage_?: ModifierStage;
@@ -12,12 +15,12 @@ abstract class ConditionalModifierBaseStage implements ModifierStage {
     private nestedModifier: Modifier,
   ) {}
 
-  run(target: Target): Target[] {
+  run(target: Target, options: ModifierStateOptions): Target[] {
     if (this.shouldModify(target)) {
       // Modify this target
       try {
         return this.nestedStage
-          .run(target)
+          .run(target, options)
           .map((newTarget) => newTarget.withThatTarget(target));
       } catch (ex) {
         // suppressErrors === true => Allow this target to be returned unmodified

packages/cursorless-engine/src/processTargets/modifiers/ContainingScopeStage.ts

@@ -2,9 +2,12 @@ import type { ContainingScopeModifier } from "@cursorless/common";
 import { NoContainingScopeError } from "@cursorless/common";
 import type { Target } from "../../typings/target.types";
 import type { ModifierStageFactory } from "../ModifierStageFactory";
-import type { ModifierStage } from "../PipelineStages.types";
-import type { ScopeHandlerFactory } from "./scopeHandlers/ScopeHandlerFactory";
+import type {
+  ModifierStage,
+  ModifierStateOptions,
+} from "../PipelineStages.types";
 import { getContainingScopeTarget } from "./getContainingScopeTarget";
+import type { ScopeHandlerFactory } from "./scopeHandlers/ScopeHandlerFactory";
 
 /**
  * This modifier stage expands from the input target to the smallest containing
@@ -31,7 +34,7 @@ export class ContainingScopeStage implements ModifierStage {
     private modifier: ContainingScopeModifier,
   ) {}
 
-  run(target: Target): Target[] {
+  run(target: Target, options: ModifierStateOptions): Target[] {
     const { scopeType, ancestorIndex = 0 } = this.modifier;
 
     const scopeHandler = this.scopeHandlerFactory.maybeCreate(
@@ -42,7 +45,7 @@ export class ContainingScopeStage implements ModifierStage {
     if (scopeHandler == null) {
       return this.modifierStageFactory
         .getLegacyScopeStage(this.modifier)
-        .run(target);
+        .run(target, options);
     }
 
     const containingScopes = getContainingScopeTarget(

packages/cursorless-engine/src/processTargets/modifiers/EveryScopeStage.ts

@@ -1,8 +1,11 @@
-import type { EveryScopeModifier, TextEditor, Range } from "@cursorless/common";
+import type { EveryScopeModifier, Range, TextEditor } from "@cursorless/common";
 import { NoContainingScopeError } from "@cursorless/common";
 import type { Target } from "../../typings/target.types";
 import type { ModifierStageFactory } from "../ModifierStageFactory";
-import type { ModifierStage } from "../PipelineStages.types";
+import type {
+  ModifierStage,
+  ModifierStateOptions,
+} from "../PipelineStages.types";
 import { getContainingScopeTarget } from "./getContainingScopeTarget";
 import type { ScopeHandlerFactory } from "./scopeHandlers/ScopeHandlerFactory";
 import type { TargetScope } from "./scopeHandlers/scope.types";
@@ -35,7 +38,7 @@ export class EveryScopeStage implements ModifierStage {
     private modifier: EveryScopeModifier,
   ) {}
 
-  run(target: Target): Target[] {
+  run(target: Target, options: ModifierStateOptions): Target[] {
     const { scopeType } = this.modifier;
     const { editor, isReversed } = target;
 
@@ -47,7 +50,7 @@ export class EveryScopeStage implements ModifierStage {
     if (scopeHandler == null) {
       return this.modifierStageFactory
         .getLegacyScopeStage(this.modifier)
-        .run(target);
+        .run(target, options);
     }
 
     let scopes: TargetScope[] | undefined;
@@ -62,7 +65,8 @@ export class EveryScopeStage implements ModifierStage {
       if (
         scopes.length === 1 &&
         scopes[0].domain.contains(target.contentRange) &&
-        !target.hasExplicitScopeType
+        !target.hasExplicitScopeType &&
+        !options.multipleTargets
       ) {
         // If the only scope that came back completely contains the input target
         // range, we treat the input as if it had no explicit range, expanding

packages/cursorless-engine/src/processTargets/modifiers/HeadTailStage.ts

@@ -6,7 +6,10 @@ import {
 } from "@cursorless/common";
 import type { Target } from "../../typings/target.types";
 import type { ModifierStageFactory } from "../ModifierStageFactory";
-import type { ModifierStage } from "../PipelineStages.types";
+import type {
+  ModifierStage,
+  ModifierStateOptions,
+} from "../PipelineStages.types";
 import {
   getModifierStagesFromTargetModifiers,
   processModifierStages,
@@ -53,9 +56,9 @@ class BoundedLineStage implements ModifierStage {
     private modifier: HeadModifier | TailModifier,
   ) {}
 
-  run(target: Target): Target[] {
-    const line = this.getContainingLine(target);
-    const pairInterior = this.getContainingPairInterior(target);
+  run(target: Target, options: ModifierStateOptions): Target[] {
+    const line = this.getContainingLine(target, options);
+    const pairInterior = this.getContainingPairInterior(target, options);
 
     const intersection =
       pairInterior != null
@@ -75,9 +78,12 @@ class BoundedLineStage implements ModifierStage {
     ];
   }
 
-  private getContainingPairInterior(target: Target): Target | undefined {
+  private getContainingPairInterior(
+    target: Target,
+    options: ModifierStateOptions,
+  ): Target | undefined {
     try {
-      return this.getContaining(target, {
+      return this.getContaining(target, options, {
         type: "surroundingPairInterior",
         delimiter: "any",
       })[0];
@@ -89,15 +95,22 @@ class BoundedLineStage implements ModifierStage {
     }
   }
 
-  private getContainingLine(target: Target): Target {
-    return this.getContaining(target, {
+  private getContainingLine(
+    target: Target,
+    options: ModifierStateOptions,
+  ): Target {
+    return this.getContaining(target, options, {
       type: "line",
     })[0];
   }
 
-  private getContaining(target: Target, scopeType: ScopeType): Target[] {
+  private getContaining(
+    target: Target,
+    options: ModifierStateOptions,
+    scopeType: ScopeType,
+  ): Target[] {
     return this.modifierStageFactory
       .create({ type: "containingScope", scopeType })
-      .run(target);
+      .run(target, options);
   }
 }