Cleanup

pokey · pokey · commit 5e5562b21f30 · 2023-04-18T20:14:16.000+01:00
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/ContainingScopeStage.ts b/packages/cursorless-engine/src/processTargets/modifiers/ContainingScopeStage.ts
@@ -1,4 +1,7 @@
-import { ContainingScopeModifier } from "@cursorless/common";
+import {
+  ContainingScopeModifier,
+  NoContainingScopeError,
+} from "@cursorless/common";
 import type { ProcessedTargetsContext } from "../../typings/Types";
 import type { Target } from "../../typings/target.types";
 import { ModifierStageFactory } from "../ModifierStageFactory";
@@ -44,13 +47,17 @@ export class ContainingScopeStage implements ModifierStage {
         .getLegacyScopeStage(this.modifier)
         .run(context, target);
     }
-    const errorName = this.modifier.scopeType.type;
 
-    return getContainingScopeTarget(
+    const containingScope = getContainingScopeTarget(
       target,
       scopeHandler,
-      errorName,
       ancestorIndex,
     );
+
+    if (containingScope == null) {
+      throw new NoContainingScopeError(this.modifier.scopeType.type);
+    }
+
+    return [containingScope];
   }
 }
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/EveryScopeStage.ts b/packages/cursorless-engine/src/processTargets/modifiers/EveryScopeStage.ts
@@ -107,10 +107,17 @@ export class EveryScopeStage implements ModifierStage {
       throw Error("Could not find iteration scope handler");
     }
 
-    return getContainingScopeTarget(
+    const iterationScopeTarget = getContainingScopeTarget(
       target,
       iterationScopeHandler,
-      `iteration scope for ${scopeHandler.scopeType?.type ?? "unknown"}`,
-    )[0].contentRange;
+    );
+
+    if (iterationScopeTarget == null) {
+      throw new NoContainingScopeError(
+        `iteration scope for ${scopeHandler.iterationScopeType.type}`,
+      );
+    }
+
+    return iterationScopeTarget.contentRange;
   }
 }
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/getContainingScopeTarget.ts b/packages/cursorless-engine/src/processTargets/modifiers/getContainingScopeTarget.ts
@@ -1,21 +1,23 @@
-import {
-  Direction,
-  NoContainingScopeError,
-  Position,
-  TextEditor,
-} from "@cursorless/common";
+import { Direction, Position, TextEditor } from "@cursorless/common";
 import type { Target } from "../../typings/target.types";
 import { constructScopeRangeTarget } from "./constructScopeRangeTarget";
 import { getContainingScope } from "./getContainingScope";
 import { TargetScope } from "./scopeHandlers/scope.types";
 import { ScopeHandler } from "./scopeHandlers/scopeHandler.types";
 
+/**
+ * Finds the containing scope of the target for the given scope handler
+ *
+ * @param target The target to find the containing scope of
+ * @param scopeHandler The scope handler for the scope type to find containing scope of
+ * @param ancestorIndex How many ancestors to go up. 0 means the immediate containing scope
+ * @returns A target representing the containing scope, or undefined if no containing scope found
+ */
 export function getContainingScopeTarget(
   target: Target,
   scopeHandler: ScopeHandler,
-  errorName: string,
   ancestorIndex: number = 0,
-) {
+): Target | undefined {
   const {
     isReversed,
     editor,
@@ -27,7 +29,7 @@ export function getContainingScopeTarget(
     let scope = getPreferredScopeTouchingPosition(scopeHandler, editor, start);
 
     if (scope == null) {
-      throw new NoContainingScopeError(errorName);
+      return undefined;
     }
 
     if (ancestorIndex > 0) {
@@ -41,10 +43,10 @@ export function getContainingScopeTarget(
     }
 
     if (scope == null) {
-      throw new NoContainingScopeError(errorName);
+      return undefined;
     }
 
-    return [scope.getTarget(isReversed)];
+    return scope.getTarget(isReversed);
   }
 
   const startScope = expandFromPosition(
@@ -56,11 +58,11 @@ export function getContainingScopeTarget(
   );
 
   if (startScope == null) {
-    throw new NoContainingScopeError(errorName);
+    return undefined;
   }
 
   if (startScope.domain.contains(end)) {
-    return [startScope.getTarget(isReversed)];
+    return startScope.getTarget(isReversed);
   }
 
   const endScope = expandFromPosition(
@@ -72,11 +74,12 @@ export function getContainingScopeTarget(
   );
 
   if (endScope == null) {
-    throw new NoContainingScopeError(errorName);
+    return undefined;
   }
 
-  return [constructScopeRangeTarget(isReversed, startScope, endScope)];
+  return constructScopeRangeTarget(isReversed, startScope, endScope);
 }
+
 function expandFromPosition(
   scopeHandler: ScopeHandler,
   editor: TextEditor,
@@ -100,6 +103,7 @@ function expandFromPosition(
 
   return undefined;
 }
+
 function getPreferredScopeTouchingPosition(
   scopeHandler: ScopeHandler,
   editor: TextEditor,
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/BaseTreeSitterScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/BaseTreeSitterScopeHandler.ts
@@ -1,13 +1,18 @@
-import { Direction, Position, TextEditor } from "@cursorless/common";
+import {
+  Direction,
+  Position,
+  TextDocument,
+  TextEditor,
+} from "@cursorless/common";
 import { Query, QueryMatch } from "web-tree-sitter";
 import { TreeSitter } from "../../../..";
+import BaseScopeHandler from "../BaseScopeHandler";
 import { compareTargetScopes } from "../compareTargetScopes";
 import { TargetScope } from "../scope.types";
 import { ScopeIteratorRequirements } from "../scopeHandler.types";
-import { getQueryRange } from "./getQueryRange";
-import BaseScopeHandler from "../BaseScopeHandler";
-import { positionToPoint } from "./getCaptureRangeByName";
+import { positionToPoint } from "./captureUtils";
 
+/** Base scope handler to use for both tree-sitter scopes and their iteration scopes */
 export abstract class BaseTreeSitterScopeHandler extends BaseScopeHandler {
   constructor(protected treeSitter: TreeSitter, protected query: Query) {
     super();
@@ -22,7 +27,12 @@ export abstract class BaseTreeSitterScopeHandler extends BaseScopeHandler {
     const { document } = editor;
 
     /** Narrow the range within which tree-sitter searches, for performance */
-    const { start, end } = getQueryRange(document, position, direction, hints);
+    const { start, end } = getQuerySearchRange(
+      document,
+      position,
+      direction,
+      hints,
+    );
 
     yield* this.query
       .matches(
@@ -35,8 +45,70 @@ export abstract class BaseTreeSitterScopeHandler extends BaseScopeHandler {
       .sort((a, b) => compareTargetScopes(direction, position, a, b));
   }
 
+  /**
+   * Convert a tree-sitter match to a scope, or undefined if the match is not
+   * relevant to this scope handler
+   * @param editor The editor in which the match was found
+   * @param match The match to convert to a scope
+   * @returns The scope, or undefined if the match is not relevant to this scope
+   * handler
+   */
   protected abstract matchToScope(
     editor: TextEditor,
     match: QueryMatch,
   ): TargetScope | undefined;
 }
+
+/**
+ * Constructs a range to pass to {@link Query.matches} to find scopes. Note
+ * that {@link Query.matches} will only return scopes that have non-empty
+ * intersection with this range.  Also note that the base
+ * {@link BaseScopeHandler.generateScopes} will filter out any extra scopes
+ * that we yield, so we don't need to be totally precise.
+ *
+ * @returns Range to pass to {@link Query.matches}
+ */
+function getQuerySearchRange(
+  document: TextDocument,
+  position: Position,
+  direction: Direction,
+  { containment, distalPosition }: ScopeIteratorRequirements,
+) {
+  const offset = document.offsetAt(position);
+  const distalOffset =
+    distalPosition == null ? null : document.offsetAt(distalPosition);
+
+  if (containment === "required") {
+    // If containment is required, we smear the position left and right by one
+    // character so that we have a non-empty intersection with any scope that
+    // touches position
+    return {
+      start: document.positionAt(offset - 1),
+      end: document.positionAt(offset + 1),
+    };
+  }
+
+  // If containment is disallowed, we can shift the position forward by a character to avoid
+  // matching scopes that touch position.  Otherwise, we shift the position backward by a
+  // character to ensure we get scopes that touch position.
+  const proximalShift = containment === "disallowed" ? 1 : -1;
+
+  // FIXME: Don't go all the way to end of document when there is no distalPosition?
+  // Seems wasteful to query all the way to end of document for something like "next funk"
+  // Might be better to start smaller and exponentially grow
+  return direction === "forward"
+    ? {
+        start: document.positionAt(offset + proximalShift),
+        end:
+          distalOffset == null
+            ? document.range.end
+            : document.positionAt(distalOffset + 1),
+      }
+    : {
+        start:
+          distalOffset == null
+            ? document.range.start
+            : document.positionAt(distalOffset - 1),
+        end: document.positionAt(offset - proximalShift),
+      };
+}
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterIterationScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterIterationScopeHandler.ts
@@ -1,57 +1,62 @@
-import {
-  ScopeType,
-  SimpleScopeType, TextEditor
-} from "@cursorless/common";
+import { ScopeType, SimpleScopeType, TextEditor } from "@cursorless/common";
 import { Query, QueryMatch } from "web-tree-sitter";
 import { TreeSitter } from "../../../..";
 import { PlainTarget } from "../../../targets";
 import { TargetScope } from "../scope.types";
 import { BaseTreeSitterScopeHandler } from "./BaseTreeSitterScopeHandler";
-import { getRelatedRange, getCaptureRangeByName } from "./getCaptureRangeByName";
-
+import { getRelatedRange, getCaptureRangeByName } from "./captureUtils";
 
+/** Scope handler to be used for iteration scopes of tree-sitter scope types */
 export class TreeSitterIterationScopeHandler extends BaseTreeSitterScopeHandler {
   protected isHierarchical = true;
+
+  // Doesn't correspond to any scope type
   public scopeType = undefined;
 
+  // Doesn't have any iteration scope type itself; that would correspond to
+  // something like "every every"
   public get iterationScopeType(): ScopeType {
     throw Error("Not implemented");
   }
 
   constructor(
     treeSitter: TreeSitter,
     query: Query,
-    private iterateeScopeType: SimpleScopeType
+    /** The scope type for which we are the iteration scope */
+    private iterateeScopeType: SimpleScopeType,
   ) {
     super(treeSitter, query);
   }
 
   protected matchToScope(
     editor: TextEditor,
-    match: QueryMatch
+    match: QueryMatch,
   ): TargetScope | undefined {
     const scopeTypeType = this.iterateeScopeType.type;
 
     const contentRange = getRelatedRange(match, scopeTypeType, "iteration")!;
 
     if (contentRange == null) {
+      // This capture was for some unrelated scope type
       return undefined;
     }
 
-    const domain = getCaptureRangeByName(
-      match,
-      `${scopeTypeType}.iteration.domain`,
-      `_.iteration.domain`
-    ) ?? contentRange;
+    const domain =
+      getCaptureRangeByName(
+        match,
+        `${scopeTypeType}.iteration.domain`,
+        `_.iteration.domain`,
+      ) ?? contentRange;
 
     return {
       editor,
       domain,
-      getTarget: (isReversed) => new PlainTarget({
-        editor,
-        isReversed,
-        contentRange,
-      }),
+      getTarget: (isReversed) =>
+        new PlainTarget({
+          editor,
+          isReversed,
+          contentRange,
+        }),
     };
   }
 }
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterScopeHandler.ts
@@ -7,10 +7,7 @@ import { TargetScope } from "../scope.types";
 import { CustomScopeType } from "../scopeHandler.types";
 import { BaseTreeSitterScopeHandler } from "./BaseTreeSitterScopeHandler";
 import { TreeSitterIterationScopeHandler } from "./TreeSitterIterationScopeHandler";
-import {
-  getCaptureRangeByName,
-  getRelatedRange,
-} from "./getCaptureRangeByName";
+import { getCaptureRangeByName, getRelatedRange } from "./captureUtils";
 
 /**
  * Handles scopes that are implemented using tree-sitter.
@@ -26,6 +23,8 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler {
     super(treeSitter, query);
   }
 
+  // We just create a custom scope handler that doesn't necessarily correspond
+  // to any well-defined scope type
   public get iterationScopeType(): CustomScopeType {
     return {
       type: "custom",
@@ -46,6 +45,7 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler {
     const contentRange = getCaptureRangeByName(match, scopeTypeType);
 
     if (contentRange == null) {
+      // This capture was for some unrelated scope type
       return undefined;
     }
 
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/captureUtils.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/captureUtils.ts
@@ -15,17 +15,26 @@ import { getNodeRange } from "../../../../util/nodeSelectors";
 export function getRelatedRange(
   match: QueryMatch,
   scopeTypeType: string,
-  relationship: string
+  relationship: string,
 ) {
   return getCaptureRangeByName(
     match,
     `${scopeTypeType}.${relationship}`,
-    `_.${relationship}`
+    `_.${relationship}`,
   );
 }
 
+/**
+ * Looks in the captures of a match for a capture with one of the given names, and
+ * returns the range of that capture, or undefined if no matching capture was found
+ *
+ * @param match The match to get the range from
+ * @param names The possible names of the capture to get the range for
+ * @returns A range or undefined if no matching capture was found
+ */
 export function getCaptureRangeByName(match: QueryMatch, ...names: string[]) {
-  const relatedNode = match.captures.find((capture) => names.some((name) => capture.name === name)
+  const relatedNode = match.captures.find((capture) =>
+    names.some((name) => capture.name === name),
   )?.node;
 
   return relatedNode == null ? undefined : getNodeRange(relatedNode);
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/getQueryRange.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/getQueryRange.ts
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/scopeHandler.types.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/scopeHandler.types.ts
