Skip to content

Commit

Permalink
keyboard: fix partial arg code (#2172)
Browse files Browse the repository at this point in the history
The original partial arg code constructed a partial arg where each field
of the top-level command arg was either completely filled out or
completely missing. Before #2169, that was fine, because the top-level
commands were more specific, and thus their args had more detail and the
command id itself told you more. For example, in
`modifyTargetContainingScopeType`, you had a specific scope type that
was missing, but you knew from command id that it was "containing".

As of #2169, we do a lot more in the lower level captures. For example
`modifyTargetContainingScopeType` and `modifyTargetEveryScopeType` are
now just `modifyTarget`, which only has a `modifier` key at the top
level, so you don't know until the very end what you're dealing with,
because every modifier just looks like `{command: "modifyTarget",
partialArg: {modifier: undefined}}`.

This PR deeply reconstructs the partial target, so that you can get much
more information. See the new test cases for examples of these partial
arguments

- Depends on #2169

## Checklist

- [x] Switch to `CURRENT` for default arg
- [x] Remove unique queue file
- [x] I have added
[tests](https://www.cursorless.org/docs/contributing/test-case-recorder/)
- [-] 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
  • Loading branch information
pokey authored Jan 20, 2024
1 parent 2b596eb commit 9b98f53
Show file tree
Hide file tree
Showing 5 changed files with 271 additions and 69 deletions.
27 changes: 0 additions & 27 deletions packages/cursorless-vscode/src/keyboard/grammar/UniqueWorkQueue.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import assert from "assert";
import { Grammar, Parser } from "nearley";
import { KeyDescriptor } from "../TokenTypeHelpers";
import grammar from "./generated/grammar";
import {
AcceptableTokenType,
MISSING,
NEXT,
getAcceptableTokenTypes,
} from "./getAcceptableTokenTypes";
import { isEqual } from "lodash";
import { stringifyTokens } from "./stringifyTokens";

interface TestCase {
/**
* The tokens to feed to the parser before checking for acceptable token types
*/
tokens: KeyDescriptor[];

/**
* Expect these token types to be acceptable; note that this list doesn't need
* to include all acceptable token types, just the ones that we want to test
* for.
*/
expected: AcceptableTokenType[];
}

const testCases: TestCase[] = [
{
tokens: [],
expected: [
{
type: "shape",
command: "targetDecoratedMark",
partialArg: {
decoratedMark: {
shape: NEXT,
},
mode: "replace",
},
},
{
type: "combineColorAndShape",
command: "targetDecoratedMark",
partialArg: {
decoratedMark: {
color: MISSING,
shape: MISSING,
},
mode: "replace",
},
},
{
type: "digit",
command: "modifyTarget",
partialArg: {
modifier: {
type: "relativeScope",
length: MISSING,
offset: 0,
direction: "forward",
scopeType: MISSING,
},
},
},
{
type: "digit",
command: "modifyTarget",
partialArg: {
modifier: {
type: "relativeScope",
length: MISSING,
offset: MISSING,
direction: "forward",
scopeType: MISSING,
},
},
},
{
type: "nextPrev",
command: "modifyTarget",
partialArg: {
modifier: {
type: "relativeScope",
length: MISSING,
offset: 1,
direction: "forward",
scopeType: MISSING,
},
},
},
],
},
{
tokens: [{ type: "digit", value: 3 }],
expected: [
{
type: "simpleScopeTypeType",
command: "modifyTarget",
partialArg: {
modifier: {
type: "relativeScope",
length: 3,
offset: 0,
direction: "forward",
scopeType: {
type: NEXT,
},
},
},
},
{
type: "nextPrev",
command: "modifyTarget",
partialArg: {
modifier: {
type: "relativeScope",
length: MISSING,
offset: 3,
direction: "forward",
scopeType: MISSING,
},
},
},
],
},
];

suite("keyboard.getAcceptableTokenTypes", () => {
let parser: Parser;
setup(() => {
parser = new Parser(Grammar.fromCompiled(grammar));
});

testCases.forEach(({ tokens, expected }) => {
test(`after \`${stringifyTokens(tokens)}\``, () => {
parser.feed(tokens);
for (const value of expected) {
// filter by type first for shorter error messages
const candidates = getAcceptableTokenTypes(parser).filter(
({ type }) => type === value.type,
);
const fullValue = {
type: value.type,
command: value.command,
partialArg: {
type: value.command,
arg: value.partialArg,
},
};
assert(
candidates.some((result) => isEqual(result, fullValue)),
"Relevant candidates (note that symbols will be missing):\n" +
JSON.stringify(candidates, null, 2),
);
}
});
});
});
Original file line number Diff line number Diff line change
@@ -1,9 +1,28 @@
import nearley, { State } from "nearley";
import { isEqual } from "lodash";
import { isEqual, times } from "lodash";
import { CommandRulePostProcessor } from "./CommandRulePostProcessor";
import { UniqueWorkQueue } from "./UniqueWorkQueue";
import { uniqWithHash } from "@cursorless/common";
import { DefaultMap, uniqWithHash } from "@cursorless/common";
import { KeyboardCommandHandler } from "../KeyboardCommandHandler";
import { TokenType } from "../TokenTypeHelpers";

export interface AcceptableTokenType {
/**
* The token type, eg "color".
*/
type: TokenType;

/**
* The command that wants the token type
*/
command: keyof KeyboardCommandHandler;

/**
* A partial argument for the command that wants the token type, where
* {@link NEXT} indicates where the next token would go and {@link MISSING}
* indicates arguments that haven't been typed yet.
*/
partialArg: any;
}

/**
* Given a parser, returns a list of acceptable token types at the current state
Expand All @@ -17,18 +36,22 @@ import { KeyboardCommandHandler } from "../KeyboardCommandHandler";
* @returns A list of acceptable token types, along with information about which
* top-level rules want them
*/
export function getAcceptableTokenTypes(parser: nearley.Parser) {
export function getAcceptableTokenTypes(
parser: nearley.Parser,
): AcceptableTokenType[] {
return uniqWithHash(
parser.table[parser.table.length - 1].scannable.flatMap(
(scannableState) => {
/** The token type */
const { type } = scannableState.rule.symbols[scannableState.dot];

return getRootStates(scannableState).map((root) => ({
type,
command: getMetadata(root).type,
partialArg: computePartialArg(root),
}));
return computeRootStatePartialArgs(scannableState).map(
({ state, partialArg }) => ({
type,
command: getMetadata(state).type,
partialArg,
}),
);
},
),
isEqual,
Expand All @@ -43,39 +66,86 @@ function getMetadata<T extends keyof KeyboardCommandHandler>(
.metadata;
}

// Prevent infinite recursion by limiting the number of times we visit a state
// to 3. Note that this is a pretty arbitrary number, and in theory we could
// need to visit the same state more than 3 times. However, in practice, we
// don't expect to need to visit the same state more than 3 times.
const MAX_VISITS = 3;

/** Indicates that the given token hasn't been typed yet */
export const MISSING = Symbol("missing");

/** The next token to be consumed */
export const NEXT = Symbol("next");

/**
* Given a state, returns all root states that are reachable from it. We use this
* to find out which top-level rules want a given token type.
* Given a state, returns all root states that are reachable from it, including
* partially filled out arguments to the given rule. We use this to find out
* which top-level rules want a given token type.
*
* @param state The state to get the root states of
* @param lastSymbol The partially filled out symbol that is currently being
* constructed
* @param visitCounts A map of states to the number of times they've been
* visited. We use this to prevent infinite recursion
* @param roots The list of root states
* @returns A list of root states
*/
function getRootStates(state: nearley.State) {
/** A queue of states to process; ensures we don't try to process state twice */
const queue = new UniqueWorkQueue(state);
const roots: State[] = [];
function computeRootStatePartialArgs(
state: nearley.State,
lastSymbol: any = NEXT,
visitCounts = new DefaultMap<State, number>(() => 0),
roots: { state: State; partialArg: any }[] = [],
) {
const visitCount = visitCounts.get(state);
if (visitCount > MAX_VISITS) {
return roots;
}
visitCounts.set(state, visitCount + 1);

let partialArg: any;
try {
const argList = [...getCompletedSymbols(state), lastSymbol];
argList.push(
...times(state.rule.symbols.length - argList.length, () => MISSING),
);
partialArg = state.rule.postprocess?.(argList) ?? argList;
} catch (err) {
// If we can't construct the partial argument because the rule's postprocess
// wasn't designed to handle partial arguments, then we just replace it with
// MISSING
partialArg = MISSING;
}

for (const state of queue) {
queue.push(...state.wantedBy);
if (state.wantedBy.length === 0) {
roots.push({ state, partialArg });
}

if (state.wantedBy.length === 0) {
roots.push(state);
}
for (const parent of state.wantedBy) {
// If we're not a root state, we recurse on all states that want this state,
// passing them our partial argument so that they can use it when constructing
// their partial argument
computeRootStatePartialArgs(parent, partialArg, visitCounts, roots);
}

return roots;
}

/**
* Given a root state, returns a partial argument for the command that the state
* represents. We could use this info to display which arguments have already
* been filled out while a user is typing a command.
* @param state A root state
* @returns A partial argument for the command that the state represents
* Given a state, returns a list of the symbols that have already been
* completed.
*
* @param state A state
* @returns A list of completed symbols
*/
function computePartialArg<T extends keyof KeyboardCommandHandler>(
_state: nearley.State,
): Partial<Record<T, any>> {
// FIXME: Fill this out
return {};
function getCompletedSymbols(state: nearley.State) {
let currentState = state;
const completedSymbols: any[] = [];

while (currentState.dot > 0) {
completedSymbols.push(currentState.right!.data);
currentState = currentState.left!;
}

return completedSymbols;
}
13 changes: 1 addition & 12 deletions packages/cursorless-vscode/src/keyboard/grammar/grammar.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import assert from "assert";
import { KeyDescriptor } from "../TokenTypeHelpers";
import { KeyboardCommandHandler } from "../KeyboardCommandHandler";
import { KeyboardCommand } from "../KeyboardCommandTypeHelpers";
import { stringifyTokens } from "./stringifyTokens";

interface TestCase {
tokens: KeyDescriptor[];
Expand Down Expand Up @@ -158,15 +159,3 @@ suite("keyboard grammar", () => {
});
});
});

function stringifyTokens(tokens: any[]) {
return tokens
.map((token) => {
let ret = token.type;
if (token.value != null) {
ret += `:${JSON.stringify(token.value)}`;
}
return ret;
})
.join(" ");
}
11 changes: 11 additions & 0 deletions packages/cursorless-vscode/src/keyboard/grammar/stringifyTokens.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export function stringifyTokens(tokens: any[]) {
return tokens
.map((token) => {
let ret = token.type;
if (token.value != null) {
ret += `:${JSON.stringify(token.value)}`;
}
return ret;
})
.join(" ");
}

0 comments on commit 9b98f53

Please sign in to comment.