Skip to content
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.

Sign up for GitHub

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

simplify logic on no arg commands #1584

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
+36 −56
Open

simplify logic on no arg commands #1584

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6bfa13
simplify logic on no arg commands
joswig Dec 18, 2024
7bd509c
Bug fix for `getFromAndTo`
goetzrrGit Dec 29, 2024
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
85 changes: 31 additions & 54 deletions src/utilities/sequence-editor/sequence-linter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1212,79 +1212,56 @@ function validateAndLintArguments(
* Validates the command structure.
* @param stemNode - The SyntaxNode representing the command stem.
* @param argsNode - The SyntaxNode representing the command arguments.
* @param exactArgSize - The expected number of arguments.
* @param expectedArgSize - The expected number of arguments.
* @param addDefault - The function to add default arguments.
* @returns A Diagnostic object representing the validation error, or undefined if there is no error.
*/
function validateCommandStructure(
stemNode: SyntaxNode,
argsNode: SyntaxNode[] | null,
exactArgSize: number,
expectedArgSize: number,
addDefault: (view: any) => any,
): Diagnostic | undefined {
if (arguments.length > 0) {
if (!argsNode || argsNode.length === 0) {
return {
actions: [],
from: stemNode.from,
message: `The stem is missing arguments.`,
severity: 'error',
to: stemNode.to,
};
}
if (argsNode.length > exactArgSize) {
const extraArgs = argsNode.slice(exactArgSize);
const { from, to } = getFromAndTo(extraArgs);
return {
actions: [
{
apply(view, from, to) {
view.dispatch({ changes: { from, to } });
},
name: `Remove ${extraArgs.length} extra argument${extraArgs.length > 1 ? 's' : ''}`,
},
],
from,
message: `Extra arguments, definition has ${exactArgSize}, but ${argsNode.length} are present`,
severity: 'error',
to,
};
}
if (argsNode.length < exactArgSize) {
const { from, to } = getFromAndTo(argsNode);
const pluralS = exactArgSize > argsNode.length + 1 ? 's' : '';
return {
actions: [
{
apply(view) {
addDefault(view);
},
name: `Add default missing argument${pluralS}`,
},
],
from,
message: `Missing argument${pluralS}, definition has ${argsNode.length}, but ${exactArgSize} are present`,
severity: 'error',
to,
};
}
} else if (argsNode && argsNode.length > 0) {
const { from, to } = getFromAndTo(argsNode);
if ((!argsNode || argsNode.length === 0) && expectedArgSize === 0) {
return undefined;
}
if (argsNode && argsNode.length > expectedArgSize) {
const extraArgs = argsNode.slice(expectedArgSize);
const { from, to } = getFromAndTo(extraArgs);
return {
actions: [
{
apply(view, from, to) {
view.dispatch({ changes: { from, to } });
},
name: `Remove argument${argsNode.length > 1 ? 's' : ''}`,
name: `Remove ${extraArgs.length} extra argument${extraArgs.length > 1 ? 's' : ''}`,
},
],
from: from,
message: 'The command should not have arguments',
from,
message: `Extra arguments, definition has ${expectedArgSize}, but ${argsNode.length} are present`,
severity: 'error',
to: to,
to,
};
}
if ((argsNode && argsNode.length < expectedArgSize) || (!argsNode && expectedArgSize > 0)) {
const { from, to } = getFromAndTo(argsNode ?? [stemNode]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Change this to

const { from, to } = getFromAndTo(argsNode?.length ? argsNode : [stemNode]);

That way it will highlight the command stem instead of the TimeTag

Copy link
Contributor

Choose a reason for hiding this comment

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

here is the commit for the code pushed:

7bd509c

const pluralS = expectedArgSize - (argsNode?.length ?? 0) > 1 ? 's' : '';
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a pluralize utility function in utilities/text.ts if you'd like to use that instead.

return {
actions: [
{
apply(view) {
addDefault(view);
},
name: `Add default missing argument${pluralS}`,
},
],
from,
message: `Missing argument${pluralS}, definition has ${expectedArgSize}, but ${argsNode?.length ?? 0} are present`,
severity: 'error',
to,
};
}

return undefined;
}

Expand Down
Loading