diff --git a/data/fixtures/scopes/python/argument.formal2.scope b/data/fixtures/scopes/python/argument.formal2.scope new file mode 100644 index 0000000000..fd92a8b35c --- /dev/null +++ b/data/fixtures/scopes/python/argument.formal2.scope @@ -0,0 +1,49 @@ +def foo( + aaa, + bbb, +): + pass +--- + +[#1 Content] = +[#1 Domain] = 1:4-1:7 + >---< +1| aaa, + +[#1 Removal] = 1:4-2:4 + >---- +1| aaa, +2| bbb, + ----< + +[#1 Trailing delimiter] = 1:7-2:4 + >- +1| aaa, +2| bbb, + ----< + +[#1 Insertion delimiter] = ",\n" + + +[#2 Content] = +[#2 Domain] = 2:4-2:7 + >---< +2| bbb, + +[#2 Removal] = 1:7-2:7 + >- +1| aaa, +2| bbb, + -------< + +[#2 Leading delimiter] = 1:7-2:4 + >- +1| aaa, +2| bbb, + ----< + +[#2 Trailing delimiter] = 2:7-2:8 + >-< +2| bbb, + +[#2 Insertion delimiter] = ",\n" diff --git a/packages/cursorless-engine/src/languages/TreeSitterQuery/validateQueryCaptures.ts b/packages/cursorless-engine/src/languages/TreeSitterQuery/validateQueryCaptures.ts index c32737f8f5..4f2b8f4db9 100644 --- a/packages/cursorless-engine/src/languages/TreeSitterQuery/validateQueryCaptures.ts +++ b/packages/cursorless-engine/src/languages/TreeSitterQuery/validateQueryCaptures.ts @@ -5,7 +5,14 @@ const wildcard = "_"; const captureNames = [wildcard, ...simpleScopeTypeTypes]; const positionRelationships = ["prefix", "leading", "trailing"]; -const positionSuffixes = ["startOf", "endOf"]; +const positionSuffixes = [ + "startOf", + "endOf", + "start.startOf", + "start.endOf", + "end.startOf", + "end.endOf", +]; const rangeRelationships = [ "domain", diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/BoundedScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/BoundedScopeHandler.ts index 19d11caa7e..6f0eff3f9b 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/BoundedScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/BoundedScopeHandler.ts @@ -4,6 +4,7 @@ import type { ScopeType, TextEditor, } from "@cursorless/common"; +import type { Target } from "../../../typings/target.types"; import type { InteriorTarget } from "../../targets"; import { BoundedParagraphTarget, @@ -18,7 +19,7 @@ import type { ScopeIteratorRequirements, } from "./scopeHandler.types"; import type { ScopeHandlerFactory } from "./ScopeHandlerFactory"; -import type { Target } from "../../../typings/target.types"; +import { isEveryScopeModifier } from "./util/isHintsEveryScope"; abstract class BoundedBaseScopeHandler extends BaseScopeHandler { protected readonly isHierarchical = true; @@ -91,11 +92,10 @@ abstract class BoundedBaseScopeHandler extends BaseScopeHandler { direction, { ...hints, - // For every (skipAncestorScopes=true) we don't want to go outside of the surrounding pair - containment: - hints.containment == null && hints.skipAncestorScopes - ? "required" - : hints.containment, + // For the every scope, we don't want to go outside of the surrounding pair + containment: isEveryScopeModifier(hints) + ? "required" + : hints.containment, }, ), ); diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts index b290edb0d2..57afc37d04 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts @@ -14,6 +14,7 @@ import type { ScopeIteratorRequirements, } from "../scopeHandler.types"; import type { ScopeHandlerFactory } from "../ScopeHandlerFactory"; +import { isEveryScopeModifier } from "../util/isHintsEveryScope"; import { OneWayNestedRangeFinder } from "../util/OneWayNestedRangeFinder"; import { OneWayRangeFinder } from "../util/OneWayRangeFinder"; import { collectionItemTextualIterationScopeHandler } from "./collectionItemTextualIterationScopeHandler"; @@ -42,7 +43,7 @@ export class CollectionItemTextualScopeHandler extends BaseScopeHandler { direction: Direction, hints: ScopeIteratorRequirements, ): Iterable { - const isEveryScope = hints.containment == null && hints.skipAncestorScopes; + const isEveryScope = isEveryScopeModifier(hints); const separatorRanges = getSeparatorOccurrences(editor.document); const interiorRanges = getInteriorRanges( this.scopeHandlerFactory, diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/createTargetScope.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/createTargetScope.ts index 770440a833..8597228a28 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/createTargetScope.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/createTargetScope.ts @@ -1,7 +1,7 @@ import { type TextEditor, Range } from "@cursorless/common"; -import { getRangeLength } from "../../../../util/rangeUtils"; import { ScopeTypeTarget } from "../../../targets"; import type { TargetScope } from "../scope.types"; +import { getCollectionItemRemovalRange } from "../util/getCollectionItemRemovalRange"; export function createTargetScope( isEveryScope: boolean, @@ -15,22 +15,19 @@ export function createTargetScope( previousRange != null ? new Range(previousRange.end, contentRange.start) : undefined; + const trailingDelimiterRange = nextRange != null ? new Range(contentRange.end, nextRange.start) : undefined; - // We have both leading and trailing delimiter ranges - // If the leading one is longer/more specific, prefer to use that for removal; - // otherwise use undefined to fallback to the default behavior (often trailing) - const removalRange = - !isEveryScope && - leadingDelimiterRange != null && - trailingDelimiterRange != null && - getRangeLength(editor, leadingDelimiterRange) > - getRangeLength(editor, trailingDelimiterRange) - ? contentRange.union(leadingDelimiterRange) - : undefined; + const removalRange = getCollectionItemRemovalRange( + isEveryScope, + editor, + contentRange, + leadingDelimiterRange, + trailingDelimiterRange, + ); const insertionDelimiter = iterationRange.isSingleLine ? ", " : ",\n"; diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/BaseTreeSitterScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/BaseTreeSitterScopeHandler.ts index 7c659d3c85..cbe8912d4a 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/BaseTreeSitterScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/BaseTreeSitterScopeHandler.ts @@ -8,6 +8,7 @@ import { BaseScopeHandler } from "../BaseScopeHandler"; import { compareTargetScopes } from "../compareTargetScopes"; import type { TargetScope } from "../scope.types"; import type { ScopeIteratorRequirements } from "../scopeHandler.types"; +import { isEveryScopeModifier } from "../util/isHintsEveryScope"; import { getQuerySearchRange } from "./getQuerySearchRange"; import { mergeAdjacentBy } from "./mergeAdjacentBy"; @@ -24,6 +25,7 @@ export abstract class BaseTreeSitterScopeHandler extends BaseScopeHandler { hints: ScopeIteratorRequirements, ): Iterable { const { document } = editor; + const isEveryScope = isEveryScopeModifier(hints); /** Narrow the range within which tree-sitter searches, for performance */ const { start, end } = getQuerySearchRange( @@ -35,7 +37,7 @@ export abstract class BaseTreeSitterScopeHandler extends BaseScopeHandler { const scopes = this.query .matches(document, start, end) - .map((match) => this.matchToScope(editor, match)) + .map((match) => this.matchToScope(editor, match, isEveryScope)) .filter((scope): scope is ExtendedTargetScope => scope != null) .sort((a, b) => compareTargetScopes(direction, position, a, b)); @@ -88,12 +90,14 @@ export abstract class BaseTreeSitterScopeHandler extends BaseScopeHandler { * relevant to this scope handler * @param editor The editor in which the match was found * @param match The match to convert to a scope + * @param isEveryScope Whether the scope is being used in an "every" modifier * @returns The scope, or undefined if the match is not relevant to this scope * handler */ protected abstract matchToScope( editor: TextEditor, match: QueryMatch, + isEveryScope: boolean, ): ExtendedTargetScope | undefined; } diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterIterationScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterIterationScopeHandler.ts index 9f5acd1ebc..55c299c4cb 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterIterationScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterIterationScopeHandler.ts @@ -34,6 +34,7 @@ export class TreeSitterIterationScopeHandler extends BaseTreeSitterScopeHandler protected matchToScope( editor: TextEditor, match: QueryMatch, + _isEveryScope: boolean, ): ExtendedTargetScope | undefined { const scopeTypeType = this.iterateeScopeType.type; diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterScopeHandler.ts index 36ee018eb7..5ce6141817 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TreeSitterScopeHandler/TreeSitterScopeHandler.ts @@ -3,6 +3,7 @@ import type { TreeSitterQuery } from "../../../../languages/TreeSitterQuery"; import type { QueryMatch } from "../../../../languages/TreeSitterQuery/QueryCapture"; import { ScopeTypeTarget } from "../../../targets/ScopeTypeTarget"; import type { CustomScopeType } from "../scopeHandler.types"; +import { getCollectionItemRemovalRange } from "../util/getCollectionItemRemovalRange"; import type { ExtendedTargetScope } from "./BaseTreeSitterScopeHandler"; import { BaseTreeSitterScopeHandler } from "./BaseTreeSitterScopeHandler"; import { TreeSitterIterationScopeHandler } from "./TreeSitterIterationScopeHandler"; @@ -36,6 +37,7 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler { protected matchToScope( editor: TextEditor, match: QueryMatch, + isEveryScope: boolean, ): ExtendedTargetScope | undefined { const scopeTypeType = this.scopeType.type; @@ -51,8 +53,6 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler { const domain = getRelatedRange(match, scopeTypeType, "domain", true) ?? contentRange; - const removalRange = getRelatedRange(match, scopeTypeType, "removal", true); - const interiorRange = getRelatedRange( match, scopeTypeType, @@ -81,6 +81,22 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler { true, )?.with(contentRange.end); + let removalRange = getRelatedRange(match, scopeTypeType, "removal", true); + + if ( + removalRange == null && + (scopeTypeType === "collectionItem" || + scopeTypeType === "argumentOrParameter") + ) { + removalRange = getCollectionItemRemovalRange( + isEveryScope, + editor, + contentRange, + leadingDelimiterRange, + trailingDelimiterRange, + ); + } + return { editor, domain, diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/util/getCollectionItemRemovalRange.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/util/getCollectionItemRemovalRange.ts new file mode 100644 index 0000000000..e08b72b82a --- /dev/null +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/util/getCollectionItemRemovalRange.ts @@ -0,0 +1,29 @@ +import type { Range, TextEditor } from "@cursorless/common"; + +import { getRangeLength } from "../../../../util/rangeUtils"; + +/** + * Picks which of the leading and trailing delimiter ranges to use for removal when both are present. + */ +export function getCollectionItemRemovalRange( + isEveryScope: boolean, + editor: TextEditor, + contentRange: Range, + leadingDelimiterRange: Range | undefined, + trailingDelimiterRange: Range | undefined, +): Range | undefined { + if (isEveryScope) { + // Force a fallback to the default behavior (often trailing) + return undefined; + } + // If the leading one is longer/more specific, prefer to use that for removal + if ( + leadingDelimiterRange != null && + trailingDelimiterRange != null && + getRangeLength(editor, leadingDelimiterRange) > + getRangeLength(editor, trailingDelimiterRange) + ) { + return contentRange.union(leadingDelimiterRange); + } + return undefined; +} diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/util/isHintsEveryScope.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/util/isHintsEveryScope.ts new file mode 100644 index 0000000000..f01611acc3 --- /dev/null +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/util/isHintsEveryScope.ts @@ -0,0 +1,10 @@ +import type { ScopeIteratorRequirements } from "../scopeHandler.types"; + +/** + * Returns whether the hints belong to the every scope modifier. + */ +export function isEveryScopeModifier( + hints: ScopeIteratorRequirements, +): boolean { + return hints.containment == null && hints.skipAncestorScopes; +} diff --git a/queries/python.scm b/queries/python.scm index 6a98b0dbcd..9fd7f53697 100644 --- a/queries/python.scm +++ b/queries/python.scm @@ -589,7 +589,9 @@ . (_) @argumentOrParameter . - (_)? @_.trailing.startOf + ","? @_.trailing.start.endOf + . + (_)? @_.trailing.end.startOf ) @_dummy (#not-type? @argumentOrParameter "comment") (#single-or-multi-line-delimiter! @argumentOrParameter @_dummy ", " ",\n") @@ -603,7 +605,9 @@ . (_) @argumentOrParameter . - (_)? @_.trailing.startOf + ","? @_.trailing.start.endOf + . + (_)? @_.trailing.end.startOf ) @_dummy (#not-type? @argumentOrParameter "comment") (#single-or-multi-line-delimiter! @argumentOrParameter @_dummy ", " ",\n")