From 997197252f6ea5d4bc24dec3f90062e747ae0b41 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 30 Jan 2025 17:33:26 +0100 Subject: [PATCH] Change implementation of default bounded line for head/tail modifiers (#2799) Today's implementation is too quirky with its single versus multiline behavior. I have multiple times gotten an unexpected behavior. ## Checklist - [x] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [x] 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: Phil Cohen --- .../recorded/headTail/changeHead3.yml | 26 +++++ .../command/PartialTargetDescriptor.types.ts | 2 - .../processTargets/modifiers/HeadTailStage.ts | 102 +++++++++++++----- .../SurroundingPairInteriorScopeHandler.ts | 4 - .../src/docs/user/README.md | 2 +- 5 files changed, 102 insertions(+), 34 deletions(-) create mode 100644 data/fixtures/recorded/headTail/changeHead3.yml diff --git a/data/fixtures/recorded/headTail/changeHead3.yml b/data/fixtures/recorded/headTail/changeHead3.yml new file mode 100644 index 00000000000..609d556db44 --- /dev/null +++ b/data/fixtures/recorded/headTail/changeHead3.yml @@ -0,0 +1,26 @@ +languageId: plaintext +command: + version: 7 + spokenForm: change head + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - {type: extendThroughStartOf} + usePrePhraseSnapshot: false +initialState: + documentContents: |- + (aaa + ) + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 4} + marks: {} +finalState: + documentContents: |- + ( + ) + selections: + - anchor: {line: 0, character: 1} + active: {line: 0, character: 1} diff --git a/packages/common/src/types/command/PartialTargetDescriptor.types.ts b/packages/common/src/types/command/PartialTargetDescriptor.types.ts index 97de296c5d8..af7b98cadbc 100644 --- a/packages/common/src/types/command/PartialTargetDescriptor.types.ts +++ b/packages/common/src/types/command/PartialTargetDescriptor.types.ts @@ -253,8 +253,6 @@ export interface SurroundingPairScopeType { export interface SurroundingPairInteriorScopeType { type: "surroundingPairInterior"; delimiter: SurroundingPairName; - // If true don't yield multiline pairs - requireSingleLine?: boolean; } export interface OneOfScopeType { diff --git a/packages/cursorless-engine/src/processTargets/modifiers/HeadTailStage.ts b/packages/cursorless-engine/src/processTargets/modifiers/HeadTailStage.ts index 71af22afa95..ea66c395252 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/HeadTailStage.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/HeadTailStage.ts @@ -1,4 +1,9 @@ -import type { HeadModifier, Modifier, TailModifier } from "@cursorless/common"; +import { + NoContainingScopeError, + type HeadModifier, + type ScopeType, + type TailModifier, +} from "@cursorless/common"; import type { Target } from "../../typings/target.types"; import type { ModifierStageFactory } from "../ModifierStageFactory"; import type { ModifierStage } from "../PipelineStages.types"; @@ -6,7 +11,7 @@ import { getModifierStagesFromTargetModifiers, processModifierStages, } from "../TargetPipelineRunner"; -import { HeadTailTarget } from "../targets"; +import { HeadTailTarget, PlainTarget } from "../targets"; export class HeadTailStage implements ModifierStage { constructor( @@ -15,34 +20,11 @@ export class HeadTailStage implements ModifierStage { ) {} run(target: Target): Target[] { - const modifiers: Modifier[] = this.modifier.modifiers ?? [ - { - type: "containingScope", - scopeType: { - type: "oneOf", - scopeTypes: [ - { - type: "line", - }, - { - type: "surroundingPairInterior", - delimiter: "any", - requireSingleLine: true, - }, - ], - }, - }, - ]; - - const modifierStages = getModifierStagesFromTargetModifiers( - this.modifierStageFactory, - modifiers, - ); + const modifierStages = this.getModifierStages(); const modifiedTargets = processModifierStages(modifierStages, [target]); + const isHead = this.modifier.type === "extendThroughStartOf"; return modifiedTargets.map((modifiedTarget) => { - const isHead = this.modifier.type === "extendThroughStartOf"; - return new HeadTailTarget({ editor: target.editor, isReversed: isHead, @@ -52,4 +34,70 @@ export class HeadTailStage implements ModifierStage { }); }); } + + private getModifierStages(): ModifierStage[] { + if (this.modifier.modifiers != null) { + return getModifierStagesFromTargetModifiers( + this.modifierStageFactory, + this.modifier.modifiers, + ); + } + + return [new BoundedLineStage(this.modifierStageFactory, this.modifier)]; + } +} + +class BoundedLineStage implements ModifierStage { + constructor( + private modifierStageFactory: ModifierStageFactory, + private modifier: HeadModifier | TailModifier, + ) {} + + run(target: Target): Target[] { + const line = this.getContainingLine(target); + const pairInterior = this.getContainingPairInterior(target); + + const intersection = + pairInterior != null + ? line.contentRange.intersection(pairInterior.contentRange) + : null; + + if (intersection == null || intersection.isEmpty) { + return [line]; + } + + return [ + new PlainTarget({ + editor: target.editor, + isReversed: target.isReversed, + contentRange: intersection, + }), + ]; + } + + private getContainingPairInterior(target: Target): Target | undefined { + try { + return this.getContaining(target, { + type: "surroundingPairInterior", + delimiter: "any", + })[0]; + } catch (error) { + if (error instanceof NoContainingScopeError) { + return undefined; + } + throw error; + } + } + + private getContainingLine(target: Target): Target { + return this.getContaining(target, { + type: "line", + })[0]; + } + + private getContaining(target: Target, scopeType: ScopeType): Target[] { + return this.modifierStageFactory + .create({ type: "containingScope", scopeType }) + .run(target); + } } diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/SurroundingPairInteriorScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/SurroundingPairInteriorScopeHandler.ts index feb91a40a51..9ab05380585 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/SurroundingPairInteriorScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/SurroundingPairInteriorScopeHandler.ts @@ -45,10 +45,6 @@ export class SurroundingPairInteriorScopeHandler extends BaseScopeHandler { ); for (const scope of scopes) { - if (this.scopeType.requireSingleLine && !scope.domain.isSingleLine) { - continue; - } - yield { editor, domain: scope.domain, diff --git a/packages/cursorless-org-docs/src/docs/user/README.md b/packages/cursorless-org-docs/src/docs/user/README.md index 89be87f7d54..652d0d1b518 100644 --- a/packages/cursorless-org-docs/src/docs/user/README.md +++ b/packages/cursorless-org-docs/src/docs/user/README.md @@ -314,7 +314,7 @@ The modifiers `"head"` and `"tail"` can be used to expand a target through the b - `"take head air"`: selects the mark through to start of the line - `"take tail air"`: selects the mark through to the end of the line -When inside a single-line surrounding pair (eg parentheses, brackets, etc) the head/tail modifier will only expand to the interior of that pair instead of the whole line. You can explicitly say `"head line"` or `"tail line"` to get the line behavior. +When inside a surrounding pair (eg parentheses, brackets, etc) the head/tail modifier will only expand to the interior of that pair instead of the whole line. You can explicitly say `"head line"` or `"tail line"` to get the line behavior. When followed by a modifier, they will expand their input to the start or end of the given modifier range. For example: