chris48s / v8r

✔️ A command-line JSON, YAML and TOML validator that's on your wavelength
https://chris48s.github.io/v8r/
MIT License
29 stars 5 forks source link

Unable to stop `--catalogs` option from parsing filename #117

Open tpansino opened 3 years ago

tpansino commented 3 years ago

When I try to run something like: v8r --catalogs catalog.json -- file.json

I get the help message, plus the error: Not enough non-option arguments: got 0, need at least 1

I expected that it should be possible to put options before the filename positional argument, and indeed the following works: v8r --catalogs catalog.json -v file.json

I suspect the error has something to do with the Yargs configuration. The array() docs state that it should be possible to indicate the end of an array of values with --, but that doesn't seem to be working. Maybe it's a bug upstream?

I also think the help message could be improved here. Most users aren't aware that -- is how most CLI frameworks have users escape array option parsing. I think that should either be mentioned, or the default changed so that you have to specify --catalogs catalog1.json --catalogs catalog2.json to use multiple catalogs, which feels like an uncommon case.

chris48s commented 3 years ago

v8r file.json --catalogs catalog.json (with the positional arg first) works fine, so you shouldn't need to pass an extraneous arg like -v just to get --catalogs to work.

I would also expect v8r --catalogs catalog.json -- file.json should also work, but as you say this might be an issue with v8r or it could be a problem upstream with yargs. I'll have a look into it.

or the default changed so that you have to specify --catalogs catalog1.json --catalogs catalog2.json to use multiple catalogs

I've not checked it, but from memory I think v8r file.json --catalogs catalog1.json --catalogs catalog2.json is already valid - I think yargs allows both out of the box. I'm not sure there is a way to prevent yargs from allowing the --catalogs catalog1.json catalog2.json syntax too though.

tpansino commented 3 years ago

v8r file.json --catalogs catalog.json (with the positional arg first) works fine, so you shouldn't need to pass an extraneous arg like -v just to get --catalogs to work.

The reason that works is because there's nothing else for the option to grab when you write the command that way. --catalogs eats everything including the positional arg up until another option is detected, hence the -v works.

I've not checked it, but from memory I think v8r file.json --catalogs catalog1.json --catalogs catalog2.json is already valid - I think yargs allows both out of the box. I'm not sure there is a way to prevent yargs from allowing the --catalogs catalog1.json catalog2.json syntax too though.

That's correct, it's valid.

If it can be required that way, my thought here is that might be a more clear UX than the current. Similar to how --schema works, so maybe there's a setting on string()? This would be a breaking change though, technically, if that's a factor in your decision.

Thanks for all your efforts. 👍

chris48s commented 3 years ago

I had a bit of a dig into this. Although the yargs docs indicate that this should work ( https://yargs.js.org/docs/#api-reference-arraykey ) this is now not supported (see https://github.com/yargs/yargs/issues/1527 ) Positional args have to go before --options. So I think the only way to solve this would be to switch away from yargs and adopting another arg parsing library.