Refactoring

Author: AndreasArvidsson

data/fixtures/recorded/implicitExpansion/clearCoreToken.yml

@@ -17,8 +17,4 @@ initialState:
     - anchor: {line: 0, character: 1}
       active: {line: 0, character: 1}
   marks: {}
-finalState:
-  documentContents: ()
-  selections:
-    - anchor: {line: 0, character: 1}
-      active: {line: 0, character: 1}
+thrownError: {name: NoContainingScopeError}

data/fixtures/recorded/modifiers/interior/changeInsideToken.yml

@@ -1,6 +1,7 @@
 import type {
   RawTreeSitterQueryProvider,
   ScopeType,
+  SimpleScopeType,
   SimpleScopeTypeType,
   TreeSitter,
 } from "@cursorless/common";
@@ -77,7 +78,7 @@ export class LanguageDefinition {
       return undefined;
     }
 
-    return new TreeSitterScopeHandler(this.query, scopeType);
+    return new TreeSitterScopeHandler(this.query, scopeType as SimpleScopeType);
   }
 
   /**

packages/cursorless-engine/src/languages/LanguageDefinition.ts

@@ -18,32 +18,39 @@ export class FallbackScopeHandler extends BaseScopeHandler {
   public scopeType = undefined;
   protected isHierarchical = true;
 
-  get iterationScopeType(): ScopeType {
-    throw new NoContainingScopeError(
-      "Iteration scope for FallbackScopeHandler",
+  static create(
+    scopeHandlerFactory: ScopeHandlerFactory,
+    scopeType: FallbackScopeType,
+    languageId: string,
+  ): ScopeHandler {
+    const scopeHandlers: ScopeHandler[] = scopeType.scopeTypes.map(
+      (scopeType) => scopeHandlerFactory.create(scopeType, languageId),
     );
+
+    return this.createFromScopeHandlers(scopeHandlers);
   }
 
-  constructor(
-    public scopeHandlerFactory: ScopeHandlerFactory,
-    private fallbackScopeType: FallbackScopeType,
-    private languageId: string,
-  ) {
+  static createFromScopeHandlers(scopeHandlers: ScopeHandler[]): ScopeHandler {
+    return new FallbackScopeHandler(scopeHandlers);
+  }
+
+  private constructor(private scopeHandlers: ScopeHandler[]) {
     super();
   }
 
+  get iterationScopeType(): ScopeType {
+    throw new NoContainingScopeError(
+      "Iteration scope for FallbackScopeHandler",
+    );
+  }
+
   *generateScopeCandidates(
     editor: TextEditor,
     position: Position,
     direction: Direction,
     hints: ScopeIteratorRequirements,
   ): Iterable<TargetScope> {
-    const scopeHandlers: ScopeHandler[] = this.fallbackScopeType.scopeTypes.map(
-      (scopeType) =>
-        this.scopeHandlerFactory.create(scopeType, this.languageId),
-    );
-
-    for (const scopeHandler of scopeHandlers) {
+    for (const scopeHandler of this.scopeHandlers) {
       yield* scopeHandler.generateScopes(editor, position, direction, hints);
     }
   }

packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/FallbackScopeHandler.ts

@@ -132,7 +132,7 @@ export class ScopeHandlerFactoryImpl implements ScopeHandlerFactory {
       case "oneOf":
         return OneOfScopeHandler.create(this, scopeType, languageId);
       case "fallback":
-        return new FallbackScopeHandler(this, scopeType, languageId);
+        return FallbackScopeHandler.create(this, scopeType, languageId);
       case "conditional":
         return new ConditionalScopeHandler(this, scopeType, languageId);
       case "instance":

packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactoryImpl.ts

@@ -8,7 +8,10 @@ import {
   type TextEditor,
 } from "@cursorless/common";
 import type { LanguageDefinitions } from "../../../../languages/LanguageDefinitions";
+import type { Target } from "../../../../typings/target.types";
+import { InteriorTarget } from "../../../targets";
 import { BaseScopeHandler } from "../BaseScopeHandler";
+import { FallbackScopeHandler } from "../FallbackScopeHandler";
 import { OneOfScopeHandler } from "../OneOfScopeHandler";
 import type { TargetScope } from "../scope.types";
 import type {
@@ -42,6 +45,7 @@ export class InteriorScopeHandler extends BaseScopeHandler {
     direction: Direction,
     hints: ScopeIteratorRequirements,
   ): Iterable<TargetScope> {
+    const targetDomain = new Range(position, hints.distalPosition);
     const scopeHandler = this.getScopeHandler();
 
     if (scopeHandler == null) {
@@ -55,22 +59,9 @@ export class InteriorScopeHandler extends BaseScopeHandler {
       hints,
     );
 
-    // No explicit scope type. Just yield all scopes.
-    if (!this.scopeType.explicitScopeType) {
-      yield* scopes;
-      return;
-    }
-
-    const targetDomain = new Range(position, hints.distalPosition);
-
-    // For an explicit scope type we only yield scopes that are contained within
-    // the target domain. E.g the user said "inside token", then we don't want
-    // to yield scopes that are larger than the token. The definition of an
-    // interior is that it's inside the scope.
-
     for (const scope of scopes) {
-      if (targetDomain.contains(scope.domain)) {
-        yield scope;
+      if (this.shouldYield(targetDomain, scope)) {
+        yield createInteriorScope(scope);
       }
     }
   }
@@ -80,29 +71,31 @@ export class InteriorScopeHandler extends BaseScopeHandler {
       .get(this.languageId)
       ?.getScopeHandler(this.scopeType);
 
+    const pairScopeHandler = this.scopeHandlerFactory.create(
+      {
+        type: "surroundingPair",
+        delimiter: "any",
+      },
+      this.languageId,
+    );
+
     // If the scope type is explicit (ie, the user has specified a scope
-    // type), then we don't want to include matching pairs. The user might
+    // type), then we want to prioritize language scopes. The user might
     // have said something like "inside element" and then we don't want to
-    // yield the interior of the `<div>` pair.
+    // yield the interior of the `<div>` pair first.
 
     if (this.scopeType.explicitScopeType) {
       if (languageScopeHandler == null) {
-        return undefined;
+        return pairScopeHandler;
       }
-      return languageScopeHandler;
+      return FallbackScopeHandler.createFromScopeHandlers([
+        languageScopeHandler,
+        pairScopeHandler,
+      ]);
     }
 
-    const pairInteriorScopeHandler = this.scopeHandlerFactory.create(
-      {
-        type: "surroundingPairInterior",
-        delimiter: "any",
-        allowWeakContainment: true,
-      },
-      this.languageId,
-    );
-
     if (languageScopeHandler == null) {
-      return pairInteriorScopeHandler;
+      return pairScopeHandler;
     }
 
     return OneOfScopeHandler.createFromScopeHandlers(
@@ -111,11 +104,46 @@ export class InteriorScopeHandler extends BaseScopeHandler {
         type: "oneOf",
         scopeTypes: [
           languageScopeHandler.scopeType,
-          pairInteriorScopeHandler.scopeType!,
+          pairScopeHandler.scopeType!,
         ],
       },
-      [languageScopeHandler, pairInteriorScopeHandler],
+      [languageScopeHandler, pairScopeHandler],
       this.languageId,
     );
   }
+
+  private shouldYield(targetDomain: Range, scope: TargetScope): boolean {
+    // For an explicit scope type we only yield scopes that are contained within
+    // the target domain. E.g the user said "inside token", then we don't want
+    // to yield scopes that are larger than the token. The definition of an
+    // interior is that it's inside the scope.
+
+    return (
+      !this.scopeType.explicitScopeType || targetDomain.contains(scope.domain)
+    );
+  }
+}
+
+function createInteriorScope(scope: TargetScope): TargetScope {
+  return {
+    editor: scope.editor,
+    domain: scope.domain,
+    getTargets(isReversed) {
+      return scope.getTargets(isReversed).flatMap(createInteriorTarget);
+    },
+  };
+}
+
+function createInteriorTarget(target: Target): Target[] {
+  const interior = target.getInterior();
+  if (interior != null) {
+    return interior;
+  }
+  return [
+    new InteriorTarget({
+      editor: target.editor,
+      isReversed: target.isReversed,
+      fullInteriorRange: target.contentRange,
+    }),
+  ];
 }

packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/InteriorScopeHandler.ts

@@ -21,7 +21,7 @@ export class SurroundingPairInteriorScopeHandler extends BaseScopeHandler {
       {
         type: "surroundingPair",
         delimiter: this.scopeType.delimiter,
-        requireStrongContainment: !this.scopeType.allowWeakContainment,
+        requireStrongContainment: true,
       },
       this.languageId,
     );

packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/SurroundingPairInteriorScopeHandler.ts

@@ -1,4 +1,8 @@
-import type { ScopeType, TextEditor } from "@cursorless/common";
+import type {
+  ScopeType,
+  SimpleScopeType,
+  TextEditor,
+} from "@cursorless/common";
 import type { TreeSitterQuery } from "../../../../languages/TreeSitterQuery";
 import type { QueryMatch } from "../../../../languages/TreeSitterQuery/QueryCapture";
 import { PlainTarget } from "../../../targets";
@@ -22,7 +26,7 @@ export class TreeSitterIterationScopeHandler extends BaseTreeSitterScopeHandler
   constructor(
     query: TreeSitterQuery,
     /** The scope type for which we are the iteration scope */
-    private iterateeScopeType: ScopeType,
+    private iterateeScopeType: SimpleScopeType,
   ) {
     super(query);
   }

packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterIterationScopeHandler.ts

@@ -1,7 +1,6 @@
-import type { ScopeType, TextEditor } from "@cursorless/common";
+import type { SimpleScopeType, TextEditor } from "@cursorless/common";
 import type { TreeSitterQuery } from "../../../../languages/TreeSitterQuery";
 import type { QueryMatch } from "../../../../languages/TreeSitterQuery/QueryCapture";
-import { InteriorTarget } from "../../../targets";
 import { ScopeTypeTarget } from "../../../targets/ScopeTypeTarget";
 import type { CustomScopeType } from "../scopeHandler.types";
 import { getCollectionItemRemovalRange } from "../util/getCollectionItemRemovalRange";
@@ -18,7 +17,7 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler {
 
   constructor(
     query: TreeSitterQuery,
-    public scopeType: ScopeType,
+    public scopeType: SimpleScopeType,
   ) {
     super(query);
   }
@@ -54,21 +53,6 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler {
     const domain =
       getRelatedRange(match, scopeTypeType, "domain", true) ?? contentRange;
 
-    if (scopeTypeType === "interior") {
-      return {
-        editor,
-        domain,
-        allowMultiple,
-        getTargets: (isReversed) => [
-          new InteriorTarget({
-            editor,
-            isReversed,
-            fullInteriorRange: contentRange,
-          }),
-        ],
-      };
-    }
-
     const prefixRange = getRelatedRange(
       match,
       scopeTypeType,

packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterScopeHandler.ts

@@ -1,4 +1,4 @@
-import type { Range, ScopeTypeType } from "@cursorless/common";
+import type { Range, SimpleScopeTypeType } from "@cursorless/common";
 import type { Target } from "../../typings/target.types";
 import type { CommonTargetParameters } from "./BaseTarget";
 import { BaseTarget } from "./BaseTarget";
@@ -11,7 +11,7 @@ import {
 } from "./util/insertionRemovalBehaviors/TokenInsertionRemovalBehavior";
 
 export interface ScopeTypeTargetParameters extends CommonTargetParameters {
-  readonly scopeTypeType: ScopeTypeType;
+  readonly scopeTypeType: SimpleScopeTypeType;
   readonly insertionDelimiter?: string;
   readonly prefixRange?: Range;
   readonly removalRange?: Range;
@@ -21,7 +21,7 @@ export interface ScopeTypeTargetParameters extends CommonTargetParameters {
 
 export class ScopeTypeTarget extends BaseTarget<ScopeTypeTargetParameters> {
   type = "ScopeTypeTarget";
-  private scopeTypeType_: ScopeTypeType;
+  private scopeTypeType_: SimpleScopeTypeType;
   private removalRange_?: Range;
   private leadingDelimiterRange_?: Range;
   private trailingDelimiterRange_?: Range;
@@ -118,7 +118,7 @@ export class ScopeTypeTarget extends BaseTarget<ScopeTypeTargetParameters> {
   }
 }
 
-function getInsertionDelimiter(scopeType: ScopeTypeType): string {
+function getInsertionDelimiter(scopeType: SimpleScopeTypeType): string {
   switch (scopeType) {
     case "class":
     case "namedFunction":