diff --git a/data/fixtures/recorded/relativeScopes/changeNextPair.yml b/data/fixtures/recorded/relativeScopes/changeNextPair.yml new file mode 100644 index 0000000000..d7535b6056 --- /dev/null +++ b/data/fixtures/recorded/relativeScopes/changeNextPair.yml @@ -0,0 +1,33 @@ +languageId: plaintext +command: + version: 7 + spokenForm: change next pair + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: relativeScope + scopeType: {type: surroundingPair, delimiter: any} + offset: 1 + length: 1 + direction: forward + usePrePhraseSnapshot: false +initialState: + documentContents: |- + ( + () + ) + () + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} + marks: {} +finalState: + documentContents: | + ( + () + ) + selections: + - anchor: {line: 3, character: 0} + active: {line: 3, character: 0} diff --git a/data/fixtures/recorded/relativeScopes/changeNextPair2.yml b/data/fixtures/recorded/relativeScopes/changeNextPair2.yml new file mode 100644 index 0000000000..b56d92c557 --- /dev/null +++ b/data/fixtures/recorded/relativeScopes/changeNextPair2.yml @@ -0,0 +1,34 @@ +languageId: plaintext +command: + version: 7 + spokenForm: change next pair + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: relativeScope + scopeType: {type: surroundingPair, delimiter: any} + offset: 1 + length: 1 + direction: forward + usePrePhraseSnapshot: false +initialState: + documentContents: |- + ( + + () + ) + selections: + - anchor: {line: 1, character: 4} + active: {line: 1, character: 4} + marks: {} +finalState: + documentContents: |- + ( + + + ) + selections: + - anchor: {line: 2, character: 4} + active: {line: 2, character: 4} diff --git a/data/fixtures/recorded/relativeScopes/changeNextState.yml b/data/fixtures/recorded/relativeScopes/changeNextState.yml new file mode 100644 index 0000000000..3aefc7f3b9 --- /dev/null +++ b/data/fixtures/recorded/relativeScopes/changeNextState.yml @@ -0,0 +1,33 @@ +languageId: typescript +command: + version: 7 + spokenForm: change next state + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: relativeScope + scopeType: {type: statement} + offset: 1 + length: 1 + direction: forward + usePrePhraseSnapshot: false +initialState: + documentContents: |- + if (true) { + const a = 1; + } + const b = 2; + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} + marks: {} +finalState: + documentContents: | + if (true) { + const a = 1; + } + selections: + - anchor: {line: 3, character: 0} + active: {line: 3, character: 0} diff --git a/data/fixtures/recorded/relativeScopes/changeNextState2.yml b/data/fixtures/recorded/relativeScopes/changeNextState2.yml new file mode 100644 index 0000000000..dff245025a --- /dev/null +++ b/data/fixtures/recorded/relativeScopes/changeNextState2.yml @@ -0,0 +1,34 @@ +languageId: typescript +command: + version: 7 + spokenForm: change next state + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: relativeScope + scopeType: {type: statement} + offset: 1 + length: 1 + direction: forward + usePrePhraseSnapshot: false +initialState: + documentContents: |- + if (true) { + + const a = 1; + } + selections: + - anchor: {line: 1, character: 3} + active: {line: 1, character: 3} + marks: {} +finalState: + documentContents: |- + if (true) { + + + } + selections: + - anchor: {line: 2, character: 3} + active: {line: 2, character: 3} diff --git a/data/fixtures/recorded/relativeScopes/changeNextState3.yml b/data/fixtures/recorded/relativeScopes/changeNextState3.yml new file mode 100644 index 0000000000..edbdac5985 --- /dev/null +++ b/data/fixtures/recorded/relativeScopes/changeNextState3.yml @@ -0,0 +1,39 @@ +languageId: typescript +command: + version: 7 + spokenForm: change next state + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: relativeScope + scopeType: {type: statement} + offset: 1 + length: 1 + direction: forward + usePrePhraseSnapshot: false +initialState: + documentContents: |- + if (true) { + + } + else if (false) { + const a = 1; + } + const b = 2; + selections: + - anchor: {line: 1, character: 3} + active: {line: 1, character: 3} + marks: {} +finalState: + documentContents: | + if (true) { + + } + else if (false) { + const a = 1; + } + selections: + - anchor: {line: 6, character: 0} + active: {line: 6, character: 0} diff --git a/data/fixtures/recorded/relativeScopes/changePreviousPair.yml b/data/fixtures/recorded/relativeScopes/changePreviousPair.yml new file mode 100644 index 0000000000..d7c32bed48 --- /dev/null +++ b/data/fixtures/recorded/relativeScopes/changePreviousPair.yml @@ -0,0 +1,34 @@ +languageId: plaintext +command: + version: 7 + spokenForm: change previous pair + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: relativeScope + scopeType: {type: surroundingPair, delimiter: any} + offset: 1 + length: 1 + direction: backward + usePrePhraseSnapshot: false +initialState: + documentContents: | + () + ( + () + ) + selections: + - anchor: {line: 3, character: 1} + active: {line: 3, character: 1} + marks: {} +finalState: + documentContents: | + + ( + () + ) + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} diff --git a/data/fixtures/recorded/relativeScopes/changePreviousPair2.yml b/data/fixtures/recorded/relativeScopes/changePreviousPair2.yml new file mode 100644 index 0000000000..f58911dedd --- /dev/null +++ b/data/fixtures/recorded/relativeScopes/changePreviousPair2.yml @@ -0,0 +1,34 @@ +languageId: plaintext +command: + version: 7 + spokenForm: change previous pair + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: relativeScope + scopeType: {type: surroundingPair, delimiter: any} + offset: 1 + length: 1 + direction: backward + usePrePhraseSnapshot: false +initialState: + documentContents: |- + ( + () + + ) + selections: + - anchor: {line: 2, character: 4} + active: {line: 2, character: 4} + marks: {} +finalState: + documentContents: |- + ( + + + ) + selections: + - anchor: {line: 1, character: 4} + active: {line: 1, character: 4} diff --git a/data/fixtures/recorded/relativeScopes/changePreviousState.yml b/data/fixtures/recorded/relativeScopes/changePreviousState.yml new file mode 100644 index 0000000000..584484db3d --- /dev/null +++ b/data/fixtures/recorded/relativeScopes/changePreviousState.yml @@ -0,0 +1,34 @@ +languageId: typescript +command: + version: 7 + spokenForm: change previous state + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: relativeScope + scopeType: {type: statement} + offset: 1 + length: 1 + direction: backward + usePrePhraseSnapshot: false +initialState: + documentContents: |- + const b = 2; + if (true) { + const a = 1; + } + selections: + - anchor: {line: 3, character: 1} + active: {line: 3, character: 1} + marks: {} +finalState: + documentContents: |- + + if (true) { + const a = 1; + } + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} diff --git a/data/fixtures/recorded/relativeScopes/changePreviousState2.yml b/data/fixtures/recorded/relativeScopes/changePreviousState2.yml new file mode 100644 index 0000000000..a1ff4beb3b --- /dev/null +++ b/data/fixtures/recorded/relativeScopes/changePreviousState2.yml @@ -0,0 +1,34 @@ +languageId: typescript +command: + version: 7 + spokenForm: change previous state + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: relativeScope + scopeType: {type: statement} + offset: 1 + length: 1 + direction: backward + usePrePhraseSnapshot: false +initialState: + documentContents: |- + if (true) { + const a = 1; + + } + selections: + - anchor: {line: 2, character: 3} + active: {line: 2, character: 3} + marks: {} +finalState: + documentContents: |- + if (true) { + + + } + selections: + - anchor: {line: 1, character: 3} + active: {line: 1, character: 3} diff --git a/data/fixtures/recorded/relativeScopes/changePreviousState3.yml b/data/fixtures/recorded/relativeScopes/changePreviousState3.yml new file mode 100644 index 0000000000..811a47c7f9 --- /dev/null +++ b/data/fixtures/recorded/relativeScopes/changePreviousState3.yml @@ -0,0 +1,40 @@ +languageId: typescript +command: + version: 7 + spokenForm: change previous state + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: relativeScope + scopeType: {type: statement} + offset: 1 + length: 1 + direction: backward + usePrePhraseSnapshot: false +initialState: + documentContents: |- + const b = 2; + if (true) { + const a = 1; + } + else if (false) { + + } + selections: + - anchor: {line: 5, character: 3} + active: {line: 5, character: 3} + marks: {} +finalState: + documentContents: |- + + if (true) { + const a = 1; + } + else if (false) { + + } + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} diff --git a/data/fixtures/recorded/surroundingPair/changeNextRound4.yml b/data/fixtures/recorded/surroundingPair/changeNextRound4.yml index 92e38103ad..2d52b2add6 100644 --- a/data/fixtures/recorded/surroundingPair/changeNextRound4.yml +++ b/data/fixtures/recorded/surroundingPair/changeNextRound4.yml @@ -20,7 +20,7 @@ initialState: active: {line: 0, character: 0} marks: {} finalState: - documentContents: () () + documentContents: "(()) " selections: - - anchor: {line: 0, character: 1} - active: {line: 0, character: 1} + - anchor: {line: 0, character: 5} + active: {line: 0, character: 5} diff --git a/packages/cursorless-engine/src/processTargets/modifiers/HeadTailStage.ts b/packages/cursorless-engine/src/processTargets/modifiers/HeadTailStage.ts index f5a09216cf..8d41f1b9bf 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/HeadTailStage.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/HeadTailStage.ts @@ -15,7 +15,7 @@ import { processModifierStages, } from "../TargetPipelineRunner"; import { HeadTailTarget, PlainTarget } from "../targets"; -import { createContainingInteriorStage } from "./InteriorStage"; +import { compoundInteriorScopeType } from "./InteriorStage"; class HeadTailStage implements ModifierStage { constructor( @@ -99,10 +99,12 @@ class BoundedLineStage implements ModifierStage { options: ModifierStateOptions, ): Target | undefined { try { - return createContainingInteriorStage(this.modifierStageFactory).run( - target, - options, - )[0]; + return this.modifierStageFactory + .create({ + type: "containingScope", + scopeType: compoundInteriorScopeType, + }) + .run(target, options)[0]; } catch (error) { if (error instanceof NoContainingScopeError) { return undefined; diff --git a/packages/cursorless-engine/src/processTargets/modifiers/InteriorStage.ts b/packages/cursorless-engine/src/processTargets/modifiers/InteriorStage.ts index 6d6b400608..796f2beab4 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/InteriorStage.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/InteriorStage.ts @@ -1,6 +1,8 @@ import { NoContainingScopeError, + UnsupportedScopeError, type InteriorOnlyModifier, + type ScopeType, } from "@cursorless/common"; import type { Target } from "../../typings/target.types"; import type { ModifierStageFactory } from "../ModifierStageFactory"; @@ -11,7 +13,7 @@ import type { export class InteriorOnlyStage implements ModifierStage { constructor( - private modifierHandlerFactory: ModifierStageFactory, + private modifierStageFactory: ModifierStageFactory, private modifier: InteriorOnlyModifier, ) {} @@ -34,22 +36,31 @@ export class InteriorOnlyStage implements ModifierStage { // most cases, as long as the nearest interior is what we expect, which it // usually is. if (target.hasExplicitScopeType) { - const everyModifier = this.modifierHandlerFactory.create({ + const everyModifier = this.modifierStageFactory.create({ type: "everyScope", scopeType: { type: "interior", }, }); - return everyModifier.run(target, options); + try { + return everyModifier.run(target, options); + } catch (e) { + if (e instanceof UnsupportedScopeError) { + throw new NoContainingScopeError("interior"); + } + throw e; + } } // eg "inside air" try { - return createContainingInteriorStage(this.modifierHandlerFactory).run( - target, - options, - ); + return this.modifierStageFactory + .create({ + type: "containingScope", + scopeType: compoundInteriorScopeType, + }) + .run(target, options); } catch (e) { if (e instanceof NoContainingScopeError) { throw new NoContainingScopeError("interior"); @@ -59,22 +70,15 @@ export class InteriorOnlyStage implements ModifierStage { } } -export function createContainingInteriorStage( - modifierHandlerFactory: ModifierStageFactory, -): ModifierStage { - return modifierHandlerFactory.create({ - type: "containingScope", - scopeType: { - type: "oneOf", - scopeTypes: [ - { - type: "interior", - }, - { - type: "surroundingPairInterior", - delimiter: "any", - }, - ], +export const compoundInteriorScopeType: ScopeType = { + type: "oneOf", + scopeTypes: [ + { + type: "interior", }, - }); -} + { + type: "surroundingPairInterior", + delimiter: "any", + }, + ], +}; diff --git a/packages/cursorless-engine/src/processTargets/modifiers/RelativeScopeStage.ts b/packages/cursorless-engine/src/processTargets/modifiers/RelativeScopeStage.ts index 27641d1bfb..c02687ca9e 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/RelativeScopeStage.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/RelativeScopeStage.ts @@ -1,10 +1,15 @@ -import { - NoContainingScopeError, - type RelativeScopeModifier, +import type { + Direction, + Position, + Range, + RelativeScopeModifier, + TextEditor, } from "@cursorless/common"; -import { islice, itake } from "itertools"; +import { NoContainingScopeError } from "@cursorless/common"; +import { find, ifilter, islice, itake } from "itertools"; import type { Target } from "../../typings/target.types"; import type { ModifierStage } from "../PipelineStages.types"; +import { InteriorTarget } from "../targets"; import { constructScopeRangeTarget } from "./constructScopeRangeTarget"; import { getPreferredScopeTouchingPosition } from "./getPreferredScopeTouchingPosition"; import { OutOfRangeError } from "./listUtils"; @@ -36,7 +41,12 @@ export class RelativeScopeStage implements ModifierStage { const scopes = Array.from( this.modifier.offset === 0 ? generateScopesInclusive(scopeHandler, target, this.modifier) - : generateScopesExclusive(scopeHandler, target, this.modifier), + : generateScopesExclusive( + this.scopeHandlerFactory, + scopeHandler, + target, + this.modifier, + ), ); if (scopes.length < this.modifier.length) { @@ -113,6 +123,7 @@ function generateScopesInclusive( * first scope if input range is empty and is at start of that scope. */ function generateScopesExclusive( + scopeHandlerFactory: ScopeHandlerFactory, scopeHandler: ScopeHandler, target: Target, modifier: RelativeScopeModifier, @@ -130,12 +141,167 @@ function generateScopesExclusive( ? "disallowed" : "disallowedIfStrict"; - return islice( + let scopes = scopeHandler.generateScopes(editor, initialPosition, direction, { + containment, + skipAncestorScopes: true, + }); + + const interiorRanges = getExcludedInteriorRanges( + scopeHandlerFactory, + scopeHandler, + editor, + initialPosition, + direction, + ); + + if (interiorRanges.length > 0) { + scopes = ifilter( + scopes, + (s) => !interiorRanges.some((r) => r.contains(s.domain)), + ); + } + + return islice(scopes, offset - 1, offset + desiredScopeCount - 1); +} + +/** + * Gets the interior scope range(s) within the containing scope of + * {@link initialPosition} that should be used to exclude next / previous + * scopes. + * + * The idea is that when you're in the headline of an if statement / function / + * etc, you're thinking at the same level as that scope, so the next scope + * should be outside of it. But when you're inside the body, the next scope + * should be within it. + * + * For example, in the following code: + * + * ```typescript + * if (aaa) { + * bbb(); + * ccc(); + * } + * ddd(); + * ``` + * + * The target `"next state air"` should refer to `ddd();`, not `bbb();`. + * However, `"next state bat"` should refer to `ccc();`. + */ +function getExcludedInteriorRanges( + scopeHandlerFactory: ScopeHandlerFactory, + scopeHandler: ScopeHandler, + editor: TextEditor, + initialPosition: Position, + direction: Direction, +): Range[] { + const interiorTargets = + scopeHandler.scopeType?.type === "surroundingPair" + ? getSurroundingPairInteriorTargets( + scopeHandler, + editor, + initialPosition, + direction, + ) + : getLanguageInteriorTargets( + scopeHandlerFactory, + scopeHandler, + editor, + initialPosition, + direction, + ); + + // Interiors containing the initial position are excluded. This happens when + // you are in the body of an if statement and use `next state` and in that + // case we don't want to exclude scopes within the same interior. + return interiorTargets + .map((t) => + t instanceof InteriorTarget ? t.fullInteriorRange : t.contentRange, + ) + .filter((r) => !r.contains(initialPosition)); +} + +function getSurroundingPairInteriorTargets( + scopeHandler: ScopeHandler, + editor: TextEditor, + initialPosition: Position, + direction: Direction, +): Target[] { + const containingScope = getContainingScope( + scopeHandler, + editor, + initialPosition, + direction, + ); + + if (containingScope == null) { + return []; + } + + return containingScope + .getTargets(false) + .flatMap((t) => t.getInterior() ?? []); +} + +function getLanguageInteriorTargets( + scopeHandlerFactory: ScopeHandlerFactory, + scopeHandler: ScopeHandler, + editor: TextEditor, + initialPosition: Position, + direction: Direction, +): Target[] { + const interiorScopeHandler = scopeHandlerFactory.maybeCreate( + { type: "interior" }, + editor.document.languageId, + ); + + if (interiorScopeHandler == null) { + return []; + } + + const containingScope = getContainingScope( + scopeHandler, + editor, + initialPosition, + direction, + ); + + if (containingScope == null) { + return []; + } + + const containingInitialPosition = + direction === "forward" + ? containingScope.domain.start + : containingScope.domain.end; + const containingDistalPosition = + direction === "forward" + ? containingScope.domain.end + : containingScope.domain.start; + + const interiorScopes = interiorScopeHandler.generateScopes( + editor, + containingInitialPosition, + direction, + { + skipAncestorScopes: true, + distalPosition: containingDistalPosition, + }, + ); + + return Array.from(interiorScopes).flatMap((s) => s.getTargets(false)); +} + +function getContainingScope( + scopeHandler: ScopeHandler, + editor: TextEditor, + initialPosition: Position, + direction: Direction, +): TargetScope | undefined { + return find( scopeHandler.generateScopes(editor, initialPosition, direction, { - containment, + containment: "required", + allowAdjacentScopes: true, skipAncestorScopes: true, }), - offset - 1, - offset + desiredScopeCount - 1, ); } diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactoryImpl.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactoryImpl.ts index e3b85c0a6a..a78e17bc4d 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactoryImpl.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactoryImpl.ts @@ -119,10 +119,8 @@ export class ScopeHandlerFactoryImpl implements ScopeHandlerFactory { languageId, ); case "interior": - return new InteriorScopeHandler( - this, + return InteriorScopeHandler.maybeCreate( this.languageDefinitions, - scopeType, languageId, ); case "custom": diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SortedScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SortedScopeHandler.ts index b5dc7fe7f5..1941877233 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SortedScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SortedScopeHandler.ts @@ -26,9 +26,17 @@ export class SortedScopeHandler extends BaseScopeHandler { scopeType: OneOfScopeType, languageId: string, ): ScopeHandler { - const scopeHandlers: ScopeHandler[] = scopeType.scopeTypes.map( - (scopeType) => scopeHandlerFactory.create(scopeType, languageId), - ); + const scopeHandlers = scopeType.scopeTypes + .map((scopeType) => + scopeHandlerFactory.maybeCreate(scopeType, languageId), + ) + .filter( + (scopeHandler): scopeHandler is ScopeHandler => scopeHandler != null, + ); + + if (scopeHandlers.length === 1) { + return scopeHandlers[0]; + } return this.createFromScopeHandlers( scopeHandlerFactory, diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/InteriorScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/InteriorScopeHandler.ts index c0baf6efc7..7d0bba3a5e 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/InteriorScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/SurroundingPairScopeHandler/InteriorScopeHandler.ts @@ -14,17 +14,28 @@ import type { ComplexScopeType, ScopeIteratorRequirements, } from "../scopeHandler.types"; -import type { ScopeHandlerFactory } from "../ScopeHandlerFactory"; +import type { TreeSitterScopeHandler } from "../TreeSitterScopeHandler"; export class InteriorScopeHandler extends BaseScopeHandler { + public readonly scopeType = { type: "interior" } as const; protected isHierarchical = true; - constructor( - private scopeHandlerFactory: ScopeHandlerFactory, - private languageDefinitions: LanguageDefinitions, - public readonly scopeType: ScopeType, - private languageId: string, - ) { + static maybeCreate( + languageDefinitions: LanguageDefinitions, + languageId: string, + ): InteriorScopeHandler | undefined { + const scopeHandler = languageDefinitions + .get(languageId) + ?.getScopeHandler({ type: "interior" }); + + if (scopeHandler == null) { + return undefined; + } + + return new InteriorScopeHandler(scopeHandler); + } + + private constructor(private scopeHandler: TreeSitterScopeHandler) { super(); } @@ -40,15 +51,7 @@ export class InteriorScopeHandler extends BaseScopeHandler { direction: Direction, hints: ScopeIteratorRequirements, ): Iterable { - const scopeHandler = this.languageDefinitions - .get(this.languageId) - ?.getScopeHandler(this.scopeType); - - if (scopeHandler == null) { - return; - } - - const scopes = scopeHandler.generateScopes( + const scopes = this.scopeHandler.generateScopes( editor, position, direction, diff --git a/packages/cursorless-neovim-e2e/src/shouldRunTest.ts b/packages/cursorless-neovim-e2e/src/shouldRunTest.ts index b2d4262772..42f3417ed1 100644 --- a/packages/cursorless-neovim-e2e/src/shouldRunTest.ts +++ b/packages/cursorless-neovim-e2e/src/shouldRunTest.ts @@ -14,6 +14,8 @@ const failingFixtures = [ "recorded/implicitExpansion/cloneThat2", "recorded/implicitExpansion/cloneThis", "recorded/implicitExpansion/cloneThis2", + // Incorrect final state + "recorded/relativeScopes/changePreviousPair", ]; function isFailingFixture(name: string, fixture: TestCaseFixtureLegacy) {