Skip to content

Added "its" modifier to use with inference #1024

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

Merged
merged 4 commits into from
Oct 14, 2022
Merged
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
1 change: 1 addition & 0 deletions cursorless-talon/src/modifiers/modifiers.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"trailing": "trailing",
"content": "keepContentFilter",
"empty": "keepEmptyFilter",
"its": "inferPreviousMark",
}

mod.list(
Expand Down
8 changes: 8 additions & 0 deletions docs/user/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,14 @@ For example:

If your cursor / mark is between two delimiters (not adjacent to one), then saying either "left" or "right" will cause cursorless to just expand to the nearest delimiters on either side, without trying to determine whether they are opening or closing delimiters.

#### `"its"`

The the modifier `"its"` is intended to be used as part of a compound target, and will tell Cursorless to use the previously mentioned mark in the compound target.

For example, `"take air past end of its line"` selects the range from the token containing letter `a` to the end of the line containing the same token. This is in contrast from `"take air past end of line"` which selects the range from the token containing letter `a` to the end of the line containing the current selection.

Another example is `"bring air to its value"`, which would cause the token with a hat over `a` to replace the return value containing it.

### Compound targets

Individual targets can be combined into compound targets to make bigger targets or refer to multiple selections at the same time.
Expand Down
9 changes: 8 additions & 1 deletion src/core/commandRunner/CommandRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import {
SelectionWithEditor,
} from "../../typings/Types";
import { isString } from "../../util/type";
import { canonicalizeAndValidateCommand } from "../commandVersionUpgrades/canonicalizeAndValidateCommand";
import {
canonicalizeAndValidateCommand,
checkForOldInference,
} from "../commandVersionUpgrades/canonicalizeAndValidateCommand";
import { PartialTargetV0V1 } from "../commandVersionUpgrades/upgradeV1ToV2/commandV1.types";
import inferFullTargets from "../inferFullTargets";
import { ThatMark } from "../ThatMark";
Expand Down Expand Up @@ -135,6 +138,10 @@ export default class CommandRunner {
);
}

// NB: We do this once test recording has started so that we can capture
// warning.
checkForOldInference(this.graph, partialTargetDescriptors);

const targets = processTargets(
processedTargetsContext,
targetDescriptors
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { window } from "vscode";
import { ActionType } from "../../actions/actions.types";
import { OutdatedExtensionError } from "../../errors";
import {
Modifier,
PartialTargetDescriptor,
SimpleScopeTypeType,
} from "../../typings/targetDescriptor.types";
import { Graph } from "../../typings/Types";
import { getPartialPrimitiveTargets } from "../../util/getPrimitiveTargets";
import { globalStateKeys } from "../../util/globalStateKeys";
import {
Command,
CommandComplete,
Expand Down Expand Up @@ -84,7 +87,7 @@ function upgradeCommand(command: Command): CommandLatest {
return command;
}

export function validateCommand(
function validateCommand(
actionName: ActionType,
partialTargets: PartialTargetDescriptor[]
) {
Expand All @@ -110,3 +113,40 @@ function usesScopeType(
)
);
}

export async function checkForOldInference(
graph: Graph,
partialTargets: PartialTargetDescriptor[]
) {
const hasOldInference = partialTargets.some((target) => {
return (
target.type === "range" &&
target.active.mark == null &&
target.active.modifiers?.some((m) => m.type === "position") &&
!target.active.modifiers?.some((m) => m.type === "inferPreviousMark")
);
});

if (hasOldInference) {
const hideInferenceWarning =
graph.extensionContext.globalState.get<boolean>(
globalStateKeys.hideInferenceWarning,
false
);

if (!hideInferenceWarning) {
const pressed = await graph.ide.messages.showWarning(
"deprecatedPositionInference",
'The "past start of" / "past end of" form has changed behavior. For the old behavior, you can now say "past start of its" / "past end of its". For example, "take air past end of its line". You may also consider using "head" / "tail" instead; see https://www.cursorless.org/docs/#head-and-tail',
"Don't show again"
);

if (pressed) {
graph.extensionContext.globalState.update(
globalStateKeys.hideInferenceWarning,
true
);
}
}
}
}
47 changes: 28 additions & 19 deletions src/core/inferFullTargets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,17 @@ function inferPrimitiveTarget(
}

const ownPositionModifier = getPositionModifier(target);
const ownNonPositionModifiers = getNonPositionModifiers(target);
const ownModifiers = getPreservedModifiers(target);

// Position without a mark can be something like "take air past end of line"
// We will remove this case when we implement #736
const mark = target.mark ??
(ownPositionModifier == null ? null : getPreviousMark(previousTargets)) ?? {
(shouldInferPreviousMark(target)
? getPreviousMark(previousTargets)
: null) ?? {
type: "cursor",
};

const modifiers =
ownNonPositionModifiers ??
getPreviousNonPositionModifiers(previousTargets) ??
[];
ownModifiers ?? getPreviousPreservedModifiers(previousTargets) ?? [];

const positionModifier =
ownPositionModifier ?? getPreviousPositionModifier(previousTargets);
Expand Down Expand Up @@ -143,22 +141,33 @@ function getPositionModifier(
: (target.modifiers[positionModifierIndex] as PositionModifier);
}

function shouldInferPreviousMark(
target: PartialPrimitiveTargetDescriptor
): boolean {
return target.modifiers?.some((m) => m.type === "inferPreviousMark") ?? false;
}

/**
* Return a list of non-positional modifiers on the given target. We return
* undefined if there are none. Note that we will never return an empty list; we
* will always return `undefined` if there are no non-positional modifiers.
* @param target The target from which to get the non-positional modifiers
* @returns A list of non-positional modifiers or `undefined` if there are none
* Return a list of modifiers that should not be removed during inference.
* Today, we remove positional modifiers, because they have their own field on
* the full targets. We also remove modifiers that only impact inference, such
* as `inferPreviousMark`.
*
* We return `undefined` if there are no preserved modifiers. Note that we will
* never return an empty list; we will always return `undefined` if there are no
* preserved modifiers.
* @param target The target from which to get the modifiers
* @returns A list of preserved modifiers or `undefined` if there are none
*/
function getNonPositionModifiers(
function getPreservedModifiers(
target: PartialPrimitiveTargetDescriptor
): Modifier[] | undefined {
const nonPositionModifiers = target.modifiers?.filter(
(modifier) => modifier.type !== "position"
const preservedModifiers = target.modifiers?.filter(
(modifier) => !["position", "inferPreviousMark"].includes(modifier.type)
);
return nonPositionModifiers == null || nonPositionModifiers.length === 0
return preservedModifiers == null || preservedModifiers.length === 0
? undefined
: nonPositionModifiers;
: preservedModifiers;
}

function getPreviousMark(
Expand All @@ -170,10 +179,10 @@ function getPreviousMark(
);
}

function getPreviousNonPositionModifiers(
function getPreviousPreservedModifiers(
previousTargets: PartialTargetDescriptor[]
): Modifier[] | undefined {
return getPreviousTargetAttribute(previousTargets, getNonPositionModifiers);
return getPreviousTargetAttribute(previousTargets, getPreservedModifiers);
}

function getPreviousPositionModifier(
Expand Down
6 changes: 6 additions & 0 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ThatMark } from "./core/ThatMark";
import isTesting from "./testUtil/isTesting";
import { Graph } from "./typings/Types";
import { getCommandServerApi, getParseTreeApi } from "./util/getExtensionApi";
import { globalStateKeys } from "./util/globalStateKeys";
import graphFactories from "./util/graphFactories";
import makeGraph, { FactoryMap } from "./util/makeGraph";

Expand Down Expand Up @@ -36,6 +37,11 @@ export async function activate(context: vscode.ExtensionContext) {
graph.cheatsheet.init();
graph.statusBarItem.init();

// Mark these keys for synchronization
graph.extensionContext.globalState.setKeysForSync([
globalStateKeys.hideInferenceWarning,
]);

const thatMark = new ThatMark();
const sourceMark = new ThatMark();

Expand Down
25 changes: 0 additions & 25 deletions src/ide/spies/SpyConfiguration.ts

This file was deleted.

17 changes: 8 additions & 9 deletions src/ide/spies/SpyIDE.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
import { values } from "lodash";
import { pickBy, values } from "lodash";
import { Graph } from "../../typings/Types";
import { Disposable, IDE } from "../ide.types";
import SpyConfiguration from "./SpyConfiguration";
import { Configuration, Disposable, IDE } from "../ide.types";
import SpyMessages, { Message } from "./SpyMessages";

export interface SpyIDERecordedValues {
configuration: undefined;
messages: Message[] | undefined;
messages?: Message[];
}

export default class SpyIDE implements IDE {
configuration: SpyConfiguration;
configuration: Configuration;
messages: SpyMessages;

constructor(private original: IDE) {
this.configuration = new SpyConfiguration(original.configuration);
this.configuration = original.configuration;
this.messages = new SpyMessages(original.messages);
}

Expand All @@ -24,11 +22,12 @@ export default class SpyIDE implements IDE {

getSpyValues(): SpyIDERecordedValues | undefined {
const ret = {
configuration: this.configuration.getSpyValues(),
messages: this.messages.getSpyValues(),
};

return values(ret).every((value) => value == null) ? undefined : ret;
return values(ret).every((value) => value == null)
? undefined
: pickBy(ret, (value) => value != null);
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/processTargets/getModifierStage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ export default (modifier: Modifier): ModifierStage => {
return new ModifyIfUntypedStage(modifier);
case "range":
return new RangeModifierStage(modifier);
case "inferPreviousMark":
throw Error(
`Unexpected modifier '${modifier.type}'; it should have been removed during inference`
);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,6 @@ finalState:
- anchor: {line: 0, character: 8}
active: {line: 0, character: 11}
fullTargets: [{type: range, excludeStart: false, excludeEnd: false, start: {type: primitive, mark: {type: cursor}, selectionType: token, position: contents, modifier: {type: identity}, insideOutsideType: inside}, end: {type: primitive, mark: {type: cursorToken}, selectionType: token, position: after, modifier: {type: identity}, insideOutsideType: inside}}]
ide:
messages:
- {type: warning, id: deprecatedPositionInference}
Comment on lines +24 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we capture the warning message here

Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,6 @@ finalState:
- anchor: {line: 0, character: 8}
active: {line: 0, character: 6}
fullTargets: [{type: range, excludeStart: false, excludeEnd: false, start: {type: primitive, mark: {type: cursor}, selectionType: token, position: contents, modifier: {type: identity}, insideOutsideType: inside}, end: {type: primitive, mark: {type: cursorToken}, selectionType: token, position: before, modifier: {type: identity}, insideOutsideType: inside}}]
ide:
messages:
- {type: warning, id: deprecatedPositionInference}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
languageId: typescript
command:
spokenForm: bring batt before funk
version: 3
targets:
- type: primitive
mark: {type: decoratedSymbol, symbolColor: default, character: b}
- type: primitive
modifiers:
- {type: position, position: before}
- type: containingScope
scopeType: {type: namedFunction}
usePrePhraseSnapshot: true
action: {name: replaceWithTarget}
initialState:
documentContents: |-
function foo() {
return "";
}

function bar() {
return "";
}
selections:
- anchor: {line: 1, character: 12}
active: {line: 1, character: 12}
marks:
default.b:
start: {line: 4, character: 9}
end: {line: 4, character: 12}
finalState:
documentContents: |-
bar

function foo() {
return "";
}

function bar() {
return "";
}
selections:
- anchor: {line: 3, character: 12}
active: {line: 3, character: 12}
fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: b}, modifiers: []}, {type: primitive, mark: {type: cursor}, modifiers: [{type: containingScope, scopeType: {type: namedFunction}}], positionModifier: {type: position, position: before}}]
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
languageId: typescript
command:
spokenForm: bring batt before its funk
version: 3
targets:
- type: primitive
mark: {type: decoratedSymbol, symbolColor: default, character: b}
- type: primitive
modifiers:
- {type: position, position: before}
- {type: inferPreviousMark}
- type: containingScope
scopeType: {type: namedFunction}
usePrePhraseSnapshot: true
action: {name: replaceWithTarget}
initialState:
documentContents: |-
function foo() {
return "";
}

function bar() {
return "";
}
selections:
- anchor: {line: 1, character: 12}
active: {line: 1, character: 12}
marks:
default.b:
start: {line: 4, character: 9}
end: {line: 4, character: 12}
finalState:
documentContents: |-
function foo() {
return "";
}

bar

function bar() {
return "";
}
selections:
- anchor: {line: 1, character: 12}
active: {line: 1, character: 12}
fullTargets: [{type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: b}, modifiers: []}, {type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: b}, modifiers: [{type: containingScope, scopeType: {type: namedFunction}}], positionModifier: {type: position, position: before}}]
Loading