yargs / yargs-parser

:muscle: the mighty option parser used by yargs
http://yargs.js.org/
ISC License
493 stars 118 forks source link

Repeated config arguments results in a confusing error #430

Open nwf opened 2 years ago

nwf commented 2 years ago

I am unsure if this belongs here or over in https://github.com/yargs/yargs instead, but I think it's mostly an issue here in the parser.

In any case, I went and wrote a little CLI tool using nodejs and yargs and in particular its .config() mechanism; so far, so good. However, if I pass --config more than once (e.g. --config a.json --config b.json), it dies with the help message and the confusing report Invalid JSON config file: [ './a.json', './b.json' ]. A little brute force shows that that report arises from https://github.com/yargs/yargs-parser/blob/217aa62906d0bbd40e06d675b8a23df8d49a5ad4/lib/yargs-parser.ts#L688 but is, ultimately, because the call to mixin.resolve on https://github.com/yargs/yargs-parser/blob/217aa62906d0bbd40e06d675b8a23df8d49a5ad4/lib/yargs-parser.ts#L666 is, by default, a call to path.resolve (owing to https://github.com/yargs/yargs-parser/blob/90f970a6482dd4f5b5eb18d38596dd6f02d73edf/lib/index.ts#L38) and path.resolve balks if not given a string as its second argument.

At the very least the error could be more informative (perhaps, "configuration options may be given at most once"), but because it's possible to have multiple .config() directives applied to yargs, and so the machinery tolerates the notion of multiple config files in general, I would prefer to wrap the entirety of https://github.com/yargs/yargs-parser/blob/217aa62906d0bbd40e06d675b8a23df8d49a5ad4/lib/yargs-parser.ts#L664-L689 in a loop to handle the case that configPath is an array.

Velewele commented 2 years ago

Tt