verbose option should check for NaN

[joshbax189]

Bug

if the argument following -v is not a number, it is parsed as NaN, possibly corrupting the command.

For example

eask info -v --no-color

--no-color is interpreted as the argument to -v and parses to NaN. So --no-color is ignored in this case.
The same thing happens if the argument is a list of file patterns or anything else. E.g. eask compile -v foo.el will instead compile all of the files in that directory.

Fix

Check: when there is a value for -v it should be a number between 0 and 5, otherwise immediately exit with an error.

[joshbax189]

See relevant yargs doco here: https://yargs.js.org/docs/#api-reference-numberkey

[jcs090218]

I think this is simply not possible? 🤔

eask info -v --no-color

will just gives me:

Not enough arguments following: v

[jcs090218]

Just a heads-up — I’ve reverted this for now (see a7ce6aa). I’ll add it back once I confirm it’s the right approach. 🤔

The test looks good, so it's kept. :D

[joshbax189]

Seems like yargs correctly separates option args from non-option args, I didn't check that error case. The other class of errors still occurs. Try these examples of buggy commands with -v

eask install -v s

this does not install the "s" package, but installs the project package

eask uninstall -v s

similarly, this does not uninstall the "s" package, but uninstalls the project package

eask compile -v foo.el

this does not just compile foo.el but compiles all files in the directory

eask load -v file1.el file2.el

this only loads file2.el and not file1.el

[jcs090218]

I'm not sure how other CLIs handle this, but it seems reasonable to me. 🤔 Am I overlooking something?

[joshbax189]

I'm not sure how other CLIs handle this

Linux commands all exit with some sort of error

$ ls --width foo
> ls: invalid line width: ‘foo’  # status is 2

$ ps --pid foo
> error: process ID list syntax error  # status is 1

When NPM will use the default value for the option if present and print a warning

$ npm list --depth foo
npm warn invalid config depth="foo" set in command line options
npm warn invalid config Must be one of: null, numeric value
# rest of command output is the same as "npm list"

Otherwise it will fail if the command is malformed

$ npm search --searchlimit foo
npm warn invalid config searchlimit="foo" set in command line options
npm warn invalid config Must be numeric value
npm error search must be called with arguments
# status is 1

Am I overlooking something?

Yes, this is extremely frustrating UI!

The issue is that the commands above make it look like -v works with a default value, when of course it does not.
So you end up with very inconsistent behaviour:

eask compile -v file1.el file2.el  # this seems to work, I assume that -v uses a default value
eask install -v package            # surprise! this runs a completely different command
eask refresh -v                    # this fails because -v has no argument

[jcs090218]

Agreed! I have added the argv check in #369. 😉

I was hoping Yargs would handle this automatically (perhaps there’s an option for it), but I may need to spend some time reviewing their documentation. 🤔