Skip to content

Don't discard everyScope selections for multiple targets #2893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions data/fixtures/recorded/everyScope/takeEveryToken.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
languageId: plaintext
command:
version: 7
spokenForm: take every token
action:
name: setSelection
target:
type: primitive
modifiers:
- type: everyScope
scopeType: {type: token}
usePrePhraseSnapshot: false
initialState:
documentContents: |-
aaa bbb ccc
ddd eee
selections:
- anchor: {line: 0, character: 4}
active: {line: 0, character: 11}
- anchor: {line: 1, character: 4}
active: {line: 1, character: 7}
marks: {}
finalState:
documentContents: |-
aaa bbb ccc
ddd eee
selections:
- anchor: {line: 0, character: 4}
active: {line: 0, character: 7}
- anchor: {line: 0, character: 8}
active: {line: 0, character: 11}
- anchor: {line: 1, character: 4}
active: {line: 1, character: 7}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import type {
SimpleScopeTypeType,
SnippetDefinition,
} from "@cursorless/common";
import type { ModifierStageFactory } from "../../processTargets/ModifierStageFactory";
import type { ModifierStateOptions } from "../../processTargets/PipelineStages.types";
import {
Placeholder,
Text,
Variable,
type TextmateSnippet,
} from "../../snippets/vendor/vscodeSnippet/snippetParser";
import { KnownSnippetVariableNames } from "../../snippets/vendor/vscodeSnippet/snippetVariables";
import type { ModifierStageFactory } from "../../processTargets/ModifierStageFactory";
import type { Target } from "../../typings/target.types";

/**
Expand Down Expand Up @@ -138,6 +139,10 @@ function findMatchingSnippetDefinitionForSingleTarget(
): number {
const languageId = target.editor.document.languageId;

const options: ModifierStateOptions = {
multipleTargets: false,
};

// We want to find the first definition that matches the given context.
// Note that we just use the first match we find because the definitions are
// guaranteed to come sorted in precedence order.
Expand Down Expand Up @@ -165,7 +170,7 @@ function findMatchingSnippetDefinitionForSingleTarget(
type: "containingScope",
scopeType: { type: scopeTypeType },
})
.run(target)[0];
.run(target, options)[0];

if (target.contentRange.isRangeEqual(containingTarget.contentRange)) {
// Skip this scope if the target is exactly the same as the
Expand All @@ -177,7 +182,7 @@ function findMatchingSnippetDefinitionForSingleTarget(
scopeType: { type: scopeTypeType },
ancestorIndex: 1,
})
.run(target)[0];
.run(target, options)[0];
}

if (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ export interface MarkStage {
run(): Target[];
}

export interface ModifierStateOptions {
multipleTargets: boolean;
}

export interface ModifierStage {
run(target: Target): Target[];
run(target: Target, options: ModifierStateOptions): Target[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import type {
import type { Target } from "../typings/target.types";
import type { MarkStageFactory } from "./MarkStageFactory";
import type { ModifierStageFactory } from "./ModifierStageFactory";
import type { MarkStage, ModifierStage } from "./PipelineStages.types";
import type {
MarkStage,
ModifierStage,
ModifierStateOptions,
} from "./PipelineStages.types";
import { createContinuousRangeTarget } from "./createContinuousRangeTarget";
import { ImplicitStage } from "./marks/ImplicitStage";
import { ContainingTokenIfUntypedEmptyStage } from "./modifiers/ConditionalModifierStages";
Expand Down Expand Up @@ -296,7 +300,10 @@ export function processModifierStages(
// one-by-one and concatenating the results before passing them on to the
// next stage.
modifierStages.forEach((stage) => {
targets = targets.flatMap((target) => stage.run(target));
const options: ModifierStateOptions = {
multipleTargets: targets.length > 1,
};
targets = targets.flatMap((target) => stage.run(target, options));
});

// Then return the output from the final stage
Expand All @@ -309,6 +316,9 @@ function getExcludedScope(
scopeType: ScopeType,
direction: Direction,
): Target {
const options: ModifierStateOptions = {
multipleTargets: false,
};
return (
modifierStageFactory
.create({
Expand All @@ -321,7 +331,7 @@ function getExcludedScope(
// NB: The following line assumes that content range is always
// contained by domain, so that "every" will properly reconstruct
// the target from the content range.
.run(target)[0]
.run(target, options)[0]
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import type { CascadingModifier } from "@cursorless/common";
import type { Target } from "../../typings/target.types";
import type { ModifierStageFactory } from "../ModifierStageFactory";
import type { ModifierStage } from "../PipelineStages.types";
import type {
ModifierStage,
ModifierStateOptions,
} from "../PipelineStages.types";

/**
* Tries each of the given modifiers in turn until one of them doesn't throw an
Expand All @@ -25,10 +28,10 @@ export class CascadingStage implements ModifierStage {
return this.nestedStages_;
}

run(target: Target): Target[] {
run(target: Target, options: ModifierStateOptions): Target[] {
for (const nestedStage of this.nestedStages) {
try {
return nestedStage.run(target);
return nestedStage.run(target, options);
} catch (_error) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import type { Modifier, ModifyIfUntypedModifier } from "@cursorless/common";
import type { Target } from "../../typings/target.types";
import type { ModifierStageFactory } from "../ModifierStageFactory";
import type { ModifierStage } from "../PipelineStages.types";
import type {
ModifierStage,
ModifierStateOptions,
} from "../PipelineStages.types";

abstract class ConditionalModifierBaseStage implements ModifierStage {
private nestedStage_?: ModifierStage;
Expand All @@ -12,12 +15,12 @@ abstract class ConditionalModifierBaseStage implements ModifierStage {
private nestedModifier: Modifier,
) {}

run(target: Target): Target[] {
run(target: Target, options: ModifierStateOptions): Target[] {
if (this.shouldModify(target)) {
// Modify this target
try {
return this.nestedStage
.run(target)
.run(target, options)
.map((newTarget) => newTarget.withThatTarget(target));
} catch (ex) {
// suppressErrors === true => Allow this target to be returned unmodified
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ import type { ContainingScopeModifier } from "@cursorless/common";
import { NoContainingScopeError } from "@cursorless/common";
import type { Target } from "../../typings/target.types";
import type { ModifierStageFactory } from "../ModifierStageFactory";
import type { ModifierStage } from "../PipelineStages.types";
import type { ScopeHandlerFactory } from "./scopeHandlers/ScopeHandlerFactory";
import type {
ModifierStage,
ModifierStateOptions,
} from "../PipelineStages.types";
import { getContainingScopeTarget } from "./getContainingScopeTarget";
import type { ScopeHandlerFactory } from "./scopeHandlers/ScopeHandlerFactory";

/**
* This modifier stage expands from the input target to the smallest containing
Expand All @@ -31,7 +34,7 @@ export class ContainingScopeStage implements ModifierStage {
private modifier: ContainingScopeModifier,
) {}

run(target: Target): Target[] {
run(target: Target, options: ModifierStateOptions): Target[] {
const { scopeType, ancestorIndex = 0 } = this.modifier;

const scopeHandler = this.scopeHandlerFactory.maybeCreate(
Expand All @@ -42,7 +45,7 @@ export class ContainingScopeStage implements ModifierStage {
if (scopeHandler == null) {
return this.modifierStageFactory
.getLegacyScopeStage(this.modifier)
.run(target);
.run(target, options);
}

const containingScopes = getContainingScopeTarget(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import type { EveryScopeModifier, TextEditor, Range } from "@cursorless/common";
import type { EveryScopeModifier, Range, TextEditor } from "@cursorless/common";
import { NoContainingScopeError } from "@cursorless/common";
import type { Target } from "../../typings/target.types";
import type { ModifierStageFactory } from "../ModifierStageFactory";
import type { ModifierStage } from "../PipelineStages.types";
import type {
ModifierStage,
ModifierStateOptions,
} from "../PipelineStages.types";
import { getContainingScopeTarget } from "./getContainingScopeTarget";
import type { ScopeHandlerFactory } from "./scopeHandlers/ScopeHandlerFactory";
import type { TargetScope } from "./scopeHandlers/scope.types";
Expand Down Expand Up @@ -35,7 +38,7 @@ export class EveryScopeStage implements ModifierStage {
private modifier: EveryScopeModifier,
) {}

run(target: Target): Target[] {
run(target: Target, options: ModifierStateOptions): Target[] {
const { scopeType } = this.modifier;
const { editor, isReversed } = target;

Expand All @@ -47,7 +50,7 @@ export class EveryScopeStage implements ModifierStage {
if (scopeHandler == null) {
return this.modifierStageFactory
.getLegacyScopeStage(this.modifier)
.run(target);
.run(target, options);
}

let scopes: TargetScope[] | undefined;
Expand All @@ -62,7 +65,8 @@ export class EveryScopeStage implements ModifierStage {
if (
scopes.length === 1 &&
scopes[0].domain.contains(target.contentRange) &&
!target.hasExplicitScopeType
!target.hasExplicitScopeType &&
!options.multipleTargets
) {
// If the only scope that came back completely contains the input target
// range, we treat the input as if it had no explicit range, expanding
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import {
} from "@cursorless/common";
import type { Target } from "../../typings/target.types";
import type { ModifierStageFactory } from "../ModifierStageFactory";
import type { ModifierStage } from "../PipelineStages.types";
import type {
ModifierStage,
ModifierStateOptions,
} from "../PipelineStages.types";
import {
getModifierStagesFromTargetModifiers,
processModifierStages,
Expand Down Expand Up @@ -53,9 +56,9 @@ class BoundedLineStage implements ModifierStage {
private modifier: HeadModifier | TailModifier,
) {}

run(target: Target): Target[] {
const line = this.getContainingLine(target);
const pairInterior = this.getContainingPairInterior(target);
run(target: Target, options: ModifierStateOptions): Target[] {
const line = this.getContainingLine(target, options);
const pairInterior = this.getContainingPairInterior(target, options);

const intersection =
pairInterior != null
Expand All @@ -75,9 +78,12 @@ class BoundedLineStage implements ModifierStage {
];
}

private getContainingPairInterior(target: Target): Target | undefined {
private getContainingPairInterior(
target: Target,
options: ModifierStateOptions,
): Target | undefined {
try {
return this.getContaining(target, {
return this.getContaining(target, options, {
type: "surroundingPairInterior",
delimiter: "any",
})[0];
Expand All @@ -89,15 +95,22 @@ class BoundedLineStage implements ModifierStage {
}
}

private getContainingLine(target: Target): Target {
return this.getContaining(target, {
private getContainingLine(
target: Target,
options: ModifierStateOptions,
): Target {
return this.getContaining(target, options, {
type: "line",
})[0];
}

private getContaining(target: Target, scopeType: ScopeType): Target[] {
private getContaining(
target: Target,
options: ModifierStateOptions,
scopeType: ScopeType,
): Target[] {
return this.modifierStageFactory
.create({ type: "containingScope", scopeType })
.run(target);
.run(target, options);
}
}
Loading
Loading