feat(cli/unstable): add parse() with descriptive schema
#4362
Conversation
iuioiua
added
the
feedback welcome
|
This is the 2nd set of API for parsing cli arguments. It feels confusing to have 2 different sets of APIs for the same purpose. I'd recommend you develop this tool as a 3rd party tool. If that tool got popularity and adoption in the ecosystem, we would be able to consider adopting it in std |
I agree, two wouldn't make too much sense, though the one currently implemented is based on minimist and is not capable of some use cases, especially comparing it to other poplar tools like cliffy, nor compatible with such a schema based approach (as pointed out in #4272)
There are plenty of popular 3rd party tools out there (commander, yargs, cliffy, etc.) that have this declarative approach implemented and are far more popular than minimist. So I think publishing another 3rd party tool wouldn't make much sense to prove that point. The declarative approach this implements avoids the function call chains and replaces them with native objects and arrays to make it less framework-ish, but is similar to the named modules. ...
.option("--foo")
.option("--bar")=> { ...
options: [
{ name: "foo", },
{ name: "bar", },
]
} |
|
Deno is fantastic tool for writing scripts and CLIs in general. I've been using it a lot lately and find that I can get a lot of mileage using just Deno + For parsing args, the @std The direction @timreichen is taking here would be much more preferred imo then the existing |
How do you measure the popularity of packages? Also I don't see what
Without being tested as 3rd party library, how can we be convinced that this design is better than minimist? |
Sidenote: What I mean by declarative approachI mean having all declarations in one object structured in a certain way: Option propertiesWhile property->objectName parseArgs(Deno.args, {
boolean: ["color"], // type is declared here
default: { color: true }, // default value is declared here
negatable: ["color"], // negatable is declared here
});
object->properties parse(Deno.args, {
options: [
{ name: "color", type: Boolean, default: true, negatable: true } // all option properties declared in one object
]
});SubcommandsAs explored here, minimist does not support subcommand parsing and a possible implementation would lead to messy code due to the nature of property->objectName. Named valuesSince there is no option object but a collection of properties declared, it is not possible to have named values for options. This is a problem, when one wants to be able to print help for a cli. property->objectName ???object->properties parse(Deno.args, {
options: [
{ name: "foo", value: { name: "VALUE" } }
],
});
Yes, but
I was talking about the fact that |
I don't see this argument very well. import { parseArgs } from "jsr:@std/cli/parse-args";
const args = parseArgs(Deno.args, {
boolean: ["help"],
});
// This becomes { _: [], help: true }, { _: [], help: false }, etc.
if (args.help) {
console.log("Usage");
Deno.exit(0);
}Isn't Also I don't see well what
But this suggested API design isn't similar to |
|
@timreichen BTW what do you think about |
I think it is very bare-bone. But I like how one defines the options as an object with properties. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4362 +/- ##
==========================================
- Coverage 96.26% 96.20% -0.06%
==========================================
Files 470 471 +1
Lines 38243 38470 +227
Branches 5546 5612 +66
==========================================
+ Hits 36813 37011 +198
- Misses 1389 1417 +28
- Partials 41 42 +1 ☔ View full report in Codecov by Sentry. |
| await t.step("strip single quoted value", () => { | ||
| const expected = { options: { foo: "bar" }, arguments: {} }; | ||
| const actual = parse(["--foo", "'bar'"], { | ||
| options: [ | ||
| { name: "foo", value: { name: "VALUE" } }, | ||
| ], | ||
| }); | ||
| assertEquals(actual, expected); | ||
| }); |
There was a problem hiding this comment.
Why? Does the current parseArgs do this? Bash will already remove the quotes. You can just leave them if someone passes them anyway by escaping them.
It reminds me of PHP automatically escaping and unescaping random strings without my consent.
There was a problem hiding this comment.
Again, fair point.
| const expected = { options: { foo: ["bar", "baz"] }, arguments: {} }; | ||
| const actual = parse(["--foo", "bar", "baz"], { | ||
| options: [{ name: "foo", value: { name: "VALUE", multiple: true } }], | ||
| }); |
There was a problem hiding this comment.
I would expect to require passing --foo bar --foo baz here. The current parseArgs works that way.
Same with all the other cases with multiple.
| Deno.test("parse() handles commands", async (t) => { | ||
| await t.step("fn() is called", () => { | ||
| let called = false; | ||
| parse(["run", "--foo"], { |
There was a problem hiding this comment.
I think parse with command should return which command was called.
This parse should return { command: ["run"], options: {...}, arguments: {...} }.
Other examples:
[]- no command was called.["foo"]- foo command was called.["foo", "bar"]- a nested command was called.
You would be able to discriminate on this property to get the correct types of options for a particular command.
| value?: { | ||
| name: string; | ||
| optional?: boolean; | ||
| multiple?: boolean; | ||
| requireEquals?: boolean; | ||
| }; |
There was a problem hiding this comment.
I don't think this nesting is necessary and the value's name field can be removed (does it even do anything?)
| export interface Argument<T = unknown> { | ||
| name: string; | ||
| description?: string; | ||
| multiple?: boolean; | ||
| optional?: boolean; | ||
| fn?: (value: T) => T | void; | ||
| } |
There was a problem hiding this comment.
2 things:
-
Do the positional arguments need a name? I guess it's useful for generating help later, but they might as well be returned as a tuple. I think it's simpler.
-
Maybe instead of
multipleandoptionalflags on arguments, have separate lists ofrequiredArguments,optionalArguments, and a singlerestArgumentproperty onCommand. That way it can be enforced that required arguments always come first and there's at most one rest (multiple) argument at the end. It doesn't make sense to have them in any other order.
There was a problem hiding this comment.
- That is exactly right, the name property as well as the nesting is intended for the (future) stringification of an option.
- That gets more complicated, if one intends to extend the functionality with lets for example
standaloneoptions orconflictingWithoptions. So I think it is good to have these properties in the option object itself.
| export interface Command<T> { | ||
| name: string; | ||
| description?: string; | ||
| options?: ReadonlyArray<Option<T>>; | ||
| commands?: ReadonlyArray<Command<T>>; | ||
| arguments?: ReadonlyArray<Argument>; | ||
| fn?: (result: ParseResult) => void; | ||
| } |
There was a problem hiding this comment.
Maybe consider options and commands to be a Record where keys are names?
There was a problem hiding this comment.
I have considered that, but if an option doesn't have properties, They would need to be declared with an empty object, which seems strange:
parse(["--foo"], { options: { foo: {} } })parse(["run", "--foo"], {
commands: {
run: { options: { foo: {} } }
}
});Or is there a better way?
kt3k
changed the title
parse() with descriptive schemaparse() with descriptive schema
|
Can you also write some short usage guide which showcases 2, 3 typical usages of |
|
I'll be closing this without merge. I think your PR is heading in the right direction, and the design seems superior to the current implementation. However, the case for the addition must be sufficiently strong for the Deno core team to accept. So I strongly recommend porting this implementation to your own package, and making the design, implementation, documentation, and testing as best as possible. Users should feel a tangible benefit from migrating too. It's probably best done accomplished with others. Then, once ready, present it at sometime in the future as a PR. That all said, we greatly appreciate the thought and effort you've gone through with this, and other, PRs. Thank you very much. |
ref: #4272
This PR adds
parse()that takes a descriptive schema for parsing args.This is inspired by rusts
clap,yargsandcommanderjs.This needs more tests for edge cases and some discussions about features like
Option.value.accepted?: T[]for allowed values as a shortcut for`Option.fn: (value) => { if (!accepted.includes(value)) { throw new Error("...")}
Option.value.min?: numberfor a minimum of arguments takenOption.value.max?: numberfor a maximum of arguments takenThe type generics also need some rework to imply correct types based on
Option.defaultandOption.typefor returnedresultand insideOption.fn(value: T)Feedback and suggestions are very welcome. @iuioiua @ngdangtu-vn