Disallow multiple arguments (array) for a key

[Congelli501]

I would be great to have a way to prevent Array transformation of a duplicated argument.

For example, --file foo.xml --file bar.xml would lead to an error, instead of the array ['foo.xml', 'bar.xml']. Most of the time, I don't want this to append, as my code assume it will get a string, and not an array.

Something like demand(<key>, <min>, [max]) for each arguments would be great.

demand('file', 0, 1) : no file arguments, or only one
demand('file', 1, 1) would require the argument, and only one (no Array)
demand('file', 1) would require at least one argument (same behaviour as the current one)
demand('file', 1, 3) would require one to 3 file arguments. For this one, the returned result should only be an array, as we are waiting for multiple arguments.

[nexdrew]

@Congelli501 Thanks for the suggestion!

Seems reasonable enough, but I'd probably prefer not to overload the current demand() method anymore than it already is. Here's a couple different ways we could tackle this.

1. New methods, similar to what you suggest, like demandCount(key, min, [max]), or even minCount(key, min) and maxCount(key, max) (though that may be too verbose). This would affect usage and validation logic.
2. Introduce some kind of strict typing, like strict([key], [type]) (or adding strict: <type> or strict: true to the option() object) that would cause an error if the parsed option did not match the required type, such as receiving an array when a string is required. This would affect validation logic and possibly usage logic as well.
3. Introduce some kind of value coercion or transform logic mechanism, such as described in coercion in yargs? #191, possibly with some default transform methods provided by yargs, where, instead of necessarily throwing an error, mismatched types could be converted or truncated to the required type (e.g. join an array or splice out the first/last value). This would probably affect parsing logic or otherwise require some kind of post-parsing processing (say that 10 times fast) between parsing and validation logic.

I'm not sure which would provide the most bang for the buck. Thoughts?

[Congelli501]

1. I liked the overload of the demand() method because it matches the current usage.
   demand(count, [max], [msg]) would be an alias of demand('_', count, [max], [msg]), where '_' represents the non-hyphenated arguments, and could be any key.
2. I like the demandCount(key, min, [max]) method, I agree that minCount and maxCount are too verbose.
3. I don't really like this for these reasons:
   • We don't now if null (no argument) will be allowed or not
   • If I want, let's say, between 1 and 3 arguments for the key file, then only one argument will lead to a string, but 2 and 3 would return an array, so you can't do that check
4. I don't like transforms that would loose information (like truncating a string, splicing an array...), I don't see any usecase for that. coercion in yargs? #191 seems more like "sub-parsing" than transform (parsing a single argument, like 2..5, into two values, 2 and 5). Yargs is simple, and I think it should be kept that way. Having transforms could easily bloat it and confuse the user.

So I would prefer method 0 or 1.

[nexdrew]

@Congelli501 Ok.

We can possibly overload demand() some more, but I'm concerned with possible ambiguous arguments. As it is now, the method accepts the following:

1. demand('this')
2. demand('this', 'error message')
3. demand('this', true)
4. demand('this', 2) - same as 1, count currently ignored
5. demand('this', 2, 'error message') - same as 2, count currently ignored
6. demand('this', 2, true) - same as 3, count currently ignored
7. demand(['this', 'that'])
8. demand(['this', 'that'], 'error message')
9. demand(['this', 'that'], true)
10. demand(['this', 'that'], 2) - same as 7, count currently ignored
11. demand(['this', 'that'], 2, 'error message') - same as 8, count currently ignored
12. demand(['this', 'that'], 2, true) - same as 9, count currently ignored
13. demand(2)
14. demand(2, 3)
15. demand(2, 'error message')
16. demand(2, 3, 'error message')

From this, we'd have to fix at least 4, 5, 10, and 11 (stop ignoring count) and add the following:

• demand('this', 2, 3) - must have
• demand('this', 2, 3, 'error message') - must have
• demand(['this', 'that'], 2, 3) - might as well
• demand(['this', 'that'], 2, 3, 'error message') - might as well

I'd be a little concerned about how to properly document this. Just add one of these?

demand(key, count, [max], [msg])

demand(key, count, [max | msg], [msg])

I guess as long as we always require two number arguments in order to specify a max count, then this is doable.

[bcoe]

My two cents, I don't particularly want to add many more dials and knobs to demand or option for this use-case, it seems like a bit of an edge-case that someone passes multiple arguments, when only a single argument is expected.

If this is something you're concerned about in your application, I like @nexdrew's suggestion of coercion -- which is a feature I think we can add fairly elegantly, and addresses other people's issues.

alternatively, perhaps we can introduce an additional argument to string, boolean, and the other type operators?

[Congelli501]

I think this issue is very important, and could even be considered a security issue:

• the string and array API are very similar.

if (path[0] === '/') {
    throw new Error('must be relative')
}

• a lot of module can take a string (action on a single element) or an array of string

// Can remove path or each elements of path
deepRemove(path, callback);

• nodejs is more and more used in sysadmin scripting, and even in setuid scripts
• most yargs users (probably) don't know and don't expect the array feature

This 3 factors cocktail can lead to very dangerous behaviour, or an application crash (which I also consider a security problem).

IMAO, the Array feature should be only activated explicitly, but that would break backward compatibility.

[nexdrew]

Note that we're planning on adding coercion to the API in 5.x. See #586.

[laggingreflex]

Related #530 The Array feature can now be disabled.

[bcoe]

@Congelli501 as @laggingreflex indicates, the array feature can now be disabled in configuration.