deno_std
deno_std copied to clipboard
bug(flags): collect is prioritised over boolean
Describe the bug
Not sure if intentional or a bug: if boolean and collect is set for a flag, it will return [true]
. boolean
should probably be higher prioritised than collect
. Else it should be documented.
Steps to Reproduce
import { parse } from "https://deno.land/[email protected]/flags/mod.ts";
const args = parse(["--reload"], { boolean: ["reload"], collect: ["reload"] });
parse returns { _:[], reload: [true] }
Expected behavior
parse should probably return { _:[], reload: true }
Environment
- OS: MacOS 12.4
- deno version: 1.22.0
- std version: 0.143.0
it takes this path https://github.com/denoland/deno_std/blob/main/flags/mod.ts#L340 setArg calls setkey which has collect=true as default
If you specify arg name in collect
, that arg is always parsed as array. So the above behavior is intentional.
options.collect - ... All values will be collected into an array.
The above description should explain it. If you find the docs misleading/not clear, suggestions to the docs are welcome
@kt3k But what is the point of collecting a boolean flag which always returns [true]
?
You can specify the same arg multiple times, like command -vvv
, and you'll get different number of items in the array
import { parse } from "https://deno.land/[email protected]/flags/mod.ts";
const a = parse(["-vvv"], { boolean: "verbose", collect: "verbose", alias: { v: "verbose" } });
console.log(a);
// => { _: [], v: [ true, true, true ], verbose: [ true, true, true ] }
Is this a real use case? I have not encountered anything like that before. Looks more like it should throw an error since alias is not one char long.
memcached has -v
-vv
-vvv
for different levels of verbosity https://docs.oracle.com/cd/E17952_01/mysql-5.6-en/ha-memcached-cmdline-options.html
symfony (php tool) also seems having them https://symfony.com/doc/current/console/verbosity.html
Looks to me like the aliases -vv
and -vvv
should be declared separately (. With that behavior this is valid:
const args = parse(["-rrr"], { boolean: ["reload"], alias: { reload: "r"} });
as is
const args = parse(["-rrr --reload"], { boolean: ["reload"], alias: { reload: "r"} });
Where as deno run -rrr …
and deno run -r --reload …
it rightfully throws an error:
error: The argument '--reload[=<CACHE_BLOCKLIST>...]' was provided more than once, but cannot be used multiple times
So imo this is also a bug.
In any case, the flag mod seems overly complicated for what it does with boolean arrays and alias maps etc. I would suggest to deprecate it in favour of an object oriented flag mod: https://github.com/denoland/deno_std/issues/2341#issuecomment-1155703919
In any case, the flag mod seems overly complicated for what it does with boolean arrays and alias maps etc. I would suggest to deprecate it in favour of an object oriented flag mod:
I'm not in favor of changing flags
in that way. There's no trivial way to migrate from existing one to that version. So that will cause huge confusion in the ecosystem.
Also std/flags is based on minimist, which has 20K+ dependent libraries. So the design can be considered very well battle tested. Changing this to completely new one only by a speculative reasoning doesn't look like an option to me.
If we completely migrate to a new design, I think we should rather consider adopting another battle tested library such as yargs
, commander
, etc. (cac
, cliffy/flags
also might be good candidates, but they have less usages compared to other ones. util.parseArgs
of Node.js also might be a candidate, but it's very new)