Migrate collection item and argument scopes (#2081)

Note that we are here just migrating markdown and yaml collection. For languages like typescript I propose that the insertion delimiter is either `, ` for an inline collection or `,\n` for an vertical collection. When the edit is constructed by our destination `\n` gets replaced by `{indent}\n` or `\n{indent}` depending if it's before or after. In short I don't think we actually should use leading or trailing for the insertion delimiters and the destination can take care of that on its own. To get the correct insertion delimiter we could have a conditional insertion delimiter predicate that actually uses the collection and not the item itself to determine if it's a vertical collection/insertion delimiter. `insertionDelimiter = @list.range.isSingleLine ? ", " : ",\n"` ``` ( (array (_) @collectionItem ) @list (#conditional-insertion-delimiter! @collectionItem @list ", " ",\n") ) ``` Edit: Just went ahead and migrated typescript argument using the above predicate. Personally I think it turned out quite nicely. Will fix the second task #585 ## 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: Pokey Rule <755842+pokey@users.noreply.github.com>
cursorless-dev · Dec 8, 2023 · 40a6fee · 40a6fee
1 parent 811447f
commit 40a6fee
Show file tree

Hide file tree

Showing 38 changed files with 696 additions and 131 deletions.
diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/queryPredicateOperators.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/queryPredicateOperators.ts
@@ -214,6 +214,37 @@ class InsertionDelimiter extends QueryPredicateOperator<InsertionDelimiter> {
   }
 }
 
+/**
+ * A predicate operator that sets the insertion delimiter of {@link nodeInfo} to
+ * either {@link insertionDelimiterConsequence} or
+ * {@link insertionDelimiterAlternative} depending on whether
+ * {@link conditionNodeInfo} is single or multiline, respectively. For example,
+ *
+ * ```scm
+ * (#single-or-multi-line-delimiter! @foo @bar ", " ",\n")
+ * ```
+ *
+ * will set the insertion delimiter of the `@foo` capture to `", "` if the
+ * `@bar` capture is a single line and `",\n"` otherwise.
+ */
+class SingleOrMultilineDelimiter extends QueryPredicateOperator<SingleOrMultilineDelimiter> {
+  name = "single-or-multi-line-delimiter!" as const;
+  schema = z.tuple([q.node, q.node, q.string, q.string]);
+
+  run(
+    nodeInfo: MutableQueryCapture,
+    conditionNodeInfo: MutableQueryCapture,
+    insertionDelimiterConsequence: string,
+    insertionDelimiterAlternative: string,
+  ) {
+    nodeInfo.insertionDelimiter = conditionNodeInfo.range.isSingleLine
+      ? insertionDelimiterConsequence
+      : insertionDelimiterAlternative;
+
+    return true;
+  }
+}
+
 export const queryPredicateOperators = [
   new Log(),
   new NotType(),
@@ -224,5 +255,6 @@ export const queryPredicateOperators = [
   new ShrinkToMatch(),
   new AllowMultiple(),
   new InsertionDelimiter(),
+  new SingleOrMultilineDelimiter(),
   new HasMultipleChildrenOfType(),
 ];
diff --git a/packages/cursorless-engine/src/languages/getNodeMatcher.ts b/packages/cursorless-engine/src/languages/getNodeMatcher.ts
@@ -22,7 +22,6 @@ import { patternMatchers as ruby } from "./ruby";
 import rust from "./rust";
 import scala from "./scala";
 import { patternMatchers as scss } from "./scss";
-import { patternMatchers as typescript } from "./typescript";
 
 export function getNodeMatcher(
   languageId: string,
@@ -59,8 +58,6 @@ export const languageMatchers: Record<
   clojure,
   go,
   java,
-  javascript: typescript,
-  javascriptreact: typescript,
   latex,
   markdown,
   php,
@@ -69,8 +66,6 @@ export const languageMatchers: Record<
   scala,
   scss,
   rust,
-  typescript,
-  typescriptreact: typescript,
 };
 
 function matcherIncludeSiblings(matcher: NodeMatcher): NodeMatcher {

diff --git a/packages/cursorless-engine/src/languages/markdown.ts b/packages/cursorless-engine/src/languages/markdown.ts
@@ -1,17 +1,9 @@
-import { Range, SimpleScopeTypeType, TextEditor } from "@cursorless/common";
+import { SimpleScopeTypeType, TextEditor } from "@cursorless/common";
 import type { SyntaxNode } from "web-tree-sitter";
-import {
-  NodeFinder,
-  NodeMatcherAlternative,
-  SelectionWithContext,
-} from "../typings/Types";
-import { getMatchesInRange } from "../util/getMatchesInRange";
+import { NodeFinder, NodeMatcherAlternative } from "../typings/Types";
 import { leadingSiblingNodeFinder, patternFinder } from "../util/nodeFinders";
 import { createPatternMatchers, matcher } from "../util/nodeMatchers";
-import {
-  extendUntilNextMatchingSiblingOrLast,
-  selectWithLeadingDelimiter,
-} from "../util/nodeSelectors";
+import { extendUntilNextMatchingSiblingOrLast } from "../util/nodeSelectors";
 import { shrinkRangeToFitContent } from "../util/selectionUtils";
 
 const HEADING_MARKER_TYPES = [
@@ -69,48 +61,9 @@ function sectionMatcher(...patterns: string[]) {
   return matcher(leadingSiblingNodeFinder(finder), sectionExtractor);
 }
 
-const itemLeadingDelimiterExtractor = selectWithLeadingDelimiter(
-  "list_marker_parenthesis",
-  "list_marker_dot",
-  "list_marker_star",
-  "list_marker_minus",
-  "list_marker_plus",
-);
-
-function excludeTrailingNewline(editor: TextEditor, range: Range) {
-  const matches = getMatchesInRange(/\r?\n\s*$/g, editor, range);
-
-  if (matches.length > 0) {
-    return new Range(range.start, matches[0].start);
-  }
-
-  return range;
-}
-
-function itemExtractor(
-  editor: TextEditor,
-  node: SyntaxNode,
-): SelectionWithContext {
-  const { selection } = itemLeadingDelimiterExtractor(editor, node);
-  const line = editor.document.lineAt(selection.start);
-  const leadingRange = new Range(line.range.start, selection.start);
-  const indent = editor.document.getText(leadingRange);
-
-  return {
-    context: {
-      containingListDelimiter: `\n${indent}`,
-      leadingDelimiterRange: leadingRange,
-    },
-    selection: excludeTrailingNewline(editor, selection).toSelection(
-      selection.isReversed,
-    ),
-  };
-}
-
 const nodeMatchers: Partial<
   Record<SimpleScopeTypeType, NodeMatcherAlternative>
 > = {
-  collectionItem: matcher(patternFinder("list_item.paragraph!"), itemExtractor),
   section: sectionMatcher("atx_heading"),
   sectionLevelOne: sectionMatcher("atx_heading.atx_h1_marker"),
   sectionLevelTwo: sectionMatcher("atx_heading.atx_h2_marker"),

diff --git a/packages/cursorless-engine/src/languages/typescript.ts b/packages/cursorless-engine/src/languages/typescript.ts
diff --git a/packages/cursorless-engine/src/processTargets/modifiers/EveryScopeStage.ts b/packages/cursorless-engine/src/processTargets/modifiers/EveryScopeStage.ts
@@ -73,13 +73,20 @@ export class EveryScopeStage implements ModifierStage {
     if (scopes == null) {
       // If target had no explicit range, or was contained by a single target
       // instance, expand to iteration scope before overlapping
-      scopes = this.getDefaultIterationRange(
-        scopeHandler,
-        this.scopeHandlerFactory,
-        target,
-      ).flatMap((iterationRange) =>
-        getScopesOverlappingRange(scopeHandler, editor, iterationRange),
-      );
+      try {
+        scopes = this.getDefaultIterationRange(
+          scopeHandler,
+          this.scopeHandlerFactory,
+          target,
+        ).flatMap((iterationRange) =>
+          getScopesOverlappingRange(scopeHandler, editor, iterationRange),
+        );
+      } catch (error) {
+        if (!(error instanceof NoContainingScopeError)) {
+          throw error;
+        }
+        scopes = [];
+      }
     }
 
     if (scopes.length === 0) {

diff --git a/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/ItemStage.ts b/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/ItemStage.ts
@@ -8,7 +8,6 @@ import {
 } from "@cursorless/common";
 import { LanguageDefinitions } from "../../../languages/LanguageDefinitions";
 import { Target } from "../../../typings/target.types";
-import { getInsertionDelimiter } from "../../../util/nodeSelectors";
 import { getRangeLength } from "../../../util/rangeUtils";
 import { ModifierStage } from "../../PipelineStages.types";
 import { ScopeTypeTarget } from "../../targets";
@@ -109,25 +108,34 @@ export class ItemStage implements ModifierStage {
     itemInfo: ItemInfo,
     removalRange?: Range,
   ) {
-    const delimiter = getInsertionDelimiter(
-      target.editor,
+    const insertionDelimiter = getInsertionDelimiter(
       itemInfo.leadingDelimiterRange,
       itemInfo.trailingDelimiterRange,
-      ", ",
     );
     return new ScopeTypeTarget({
       scopeTypeType: this.modifier.scopeType.type as SimpleScopeTypeType,
       editor: target.editor,
       isReversed: target.isReversed,
       contentRange: itemInfo.contentRange,
-      delimiter,
+      insertionDelimiter,
       leadingDelimiterRange: itemInfo.leadingDelimiterRange,
       trailingDelimiterRange: itemInfo.trailingDelimiterRange,
       removalRange,
     });
   }
 }
 
+function getInsertionDelimiter(
+  leadingDelimiterRange?: Range,
+  trailingDelimiterRange?: Range,
+): string {
+  return (leadingDelimiterRange != null &&
+    !leadingDelimiterRange.isSingleLine) ||
+    (trailingDelimiterRange != null && !trailingDelimiterRange.isSingleLine)
+    ? ",\n"
+    : ", ";
+}
+
 /** Filter item infos by content range and domain intersection */
 function filterItemInfos(target: Target, itemInfos: ItemInfo[]): ItemInfo[] {
   return itemInfos.filter(

diff --git a/...c/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterScopeHandler.ts b/...c/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterScopeHandler.ts
@@ -70,6 +70,17 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler {
       true,
     );
 
+    const rawPrefixRange = getRelatedRange(
+      match,
+      scopeTypeType,
+      "prefix",
+      true,
+    );
+    const prefixRange =
+      rawPrefixRange != null
+        ? new Range(rawPrefixRange.start, contentRange.start)
+        : undefined;
+
     return {
       editor,
       domain,
@@ -80,11 +91,12 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler {
           editor,
           isReversed,
           contentRange,
+          prefixRange,
           removalRange,
           leadingDelimiterRange,
           trailingDelimiterRange,
           interiorRange,
-          delimiter: insertionDelimiter,
+          insertionDelimiter,
         }),
       ],
     };

diff --git a/...s-engine/src/processTargets/modifiers/scopeTypeStages/LegacyContainingSyntaxScopeStage.ts b/...s-engine/src/processTargets/modifiers/scopeTypeStages/LegacyContainingSyntaxScopeStage.ts
@@ -82,7 +82,7 @@ export class LegacyContainingSyntaxScopeStage implements ModifierStage {
         contentRange: contentSelection,
         removalRange: removalRange,
         interiorRange: interiorRange,
-        delimiter: containingListDelimiter,
+        insertionDelimiter: containingListDelimiter,
         leadingDelimiterRange,
         trailingDelimiterRange,
       });

diff --git a/packages/cursorless-engine/src/processTargets/targets/DestinationImpl.ts b/packages/cursorless-engine/src/processTargets/targets/DestinationImpl.ts
@@ -10,12 +10,14 @@ import {
   EditNewActionType,
   Target,
 } from "../../typings/target.types";
+import { union } from "../../util/rangeUtils";
 
 export class DestinationImpl implements Destination {
   public readonly contentRange: Range;
   private readonly isLineDelimiter: boolean;
   private readonly isBefore: boolean;
   private readonly indentationString: string;
+  private readonly insertionPrefix?: string;
 
   constructor(
     public readonly target: Target,
@@ -24,12 +26,15 @@ export class DestinationImpl implements Destination {
   ) {
     this.contentRange = getContentRange(target.contentRange, insertionMode);
     this.isBefore = insertionMode === "before";
-    // It's only considered a line if the delimiter is only new line symbols
-    this.isLineDelimiter = /^(\n)+$/.test(target.insertionDelimiter);
+    this.isLineDelimiter = target.insertionDelimiter.includes("\n");
     this.indentationString =
       indentationString ?? this.isLineDelimiter
         ? getIndentationString(target.editor, target.contentRange)
         : "";
+    this.insertionPrefix =
+      target.prefixRange != null
+        ? target.editor.document.getText(target.prefixRange)
+        : undefined;
   }
 
   get contentSelection(): Selection {
@@ -65,9 +70,10 @@ export class DestinationImpl implements Destination {
 
   getEditNewActionType(): EditNewActionType {
     if (
-      this.insertionDelimiter === "\n" &&
       this.insertionMode === "after" &&
-      this.target.contentRange.isSingleLine
+      this.target.contentRange.isSingleLine &&
+      this.insertionDelimiter === "\n" &&
+      this.insertionPrefix == null
     ) {
       // If the target that we're wrapping is not a single line, then we
       // want to compute indentation based on the entire target.  Otherwise,
@@ -110,43 +116,46 @@ export class DestinationImpl implements Destination {
 
   private getEditRange() {
     const position = (() => {
-      const contentPosition = this.isBefore
-        ? this.contentRange.start
-        : this.contentRange.end;
+      const insertionPosition = this.isBefore
+        ? union(this.target.contentRange, this.target.prefixRange).start
+        : this.target.contentRange.end;
 
       if (this.isLineDelimiter) {
-        const line = this.editor.document.lineAt(contentPosition);
+        const line = this.editor.document.lineAt(insertionPosition);
         const nonWhitespaceCharacterIndex = this.isBefore
           ? line.firstNonWhitespaceCharacterIndex
           : line.lastNonWhitespaceCharacterIndex;
 
-        // Use the full line to include indentation
-        if (contentPosition.character === nonWhitespaceCharacterIndex) {
+        // Use the full line with included indentation and trailing whitespaces
+        if (insertionPosition.character === nonWhitespaceCharacterIndex) {
           return this.isBefore ? line.range.start : line.range.end;
         }
       }
 
-      return contentPosition;
+      return insertionPosition;
     })();
 
     return new Range(position, position);
   }
 
   private getEditText(text: string) {
-    const insertionText = this.indentationString + text;
+    const insertionText =
+      this.indentationString + (this.insertionPrefix ?? "") + text;
 
     return this.isBefore
       ? insertionText + this.insertionDelimiter
       : this.insertionDelimiter + insertionText;
   }
 
   private updateRange(range: Range, text: string) {
-    const baseStartOffset = this.editor.document.offsetAt(range.start);
+    const baseStartOffset =
+      this.editor.document.offsetAt(range.start) +
+      this.indentationString.length +
+      (this.insertionPrefix?.length ?? 0);
+
     const startIndex = this.isBefore
-      ? baseStartOffset + this.indentationString.length
-      : baseStartOffset +
-        this.getLengthOfInsertionDelimiter() +
-        this.indentationString.length;
+      ? baseStartOffset
+      : baseStartOffset + this.getLengthOfInsertionDelimiter();
 
     const endIndex = startIndex + text.length;
 
@@ -160,11 +169,10 @@ export class DestinationImpl implements Destination {
     // Went inserting a new line with eol `CRLF` each `\n` will be converted to
     // `\r\n` and therefore the length is doubled.
     if (this.editor.document.eol === "CRLF") {
-      // This function is only called when inserting after a range. Therefore we
-      // only care about leading new lines in the insertion delimiter.
-      const match = this.insertionDelimiter.match(/^\n+/);
-      const nlCount = match?.[0].length ?? 0;
-      return this.insertionDelimiter.length + nlCount;
+      const matches = this.insertionDelimiter.match(/\n/g);
+      if (matches != null) {
+        return this.insertionDelimiter.length + matches.length;
+      }
     }
     return this.insertionDelimiter.length;
   }