Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repeated config arguments results in a confusing error #430

Open
nwf opened this issue Jan 19, 2022 · 1 comment
Open

Repeated config arguments results in a confusing error #430

nwf opened this issue Jan 19, 2022 · 1 comment

Comments

@nwf
Copy link

nwf commented Jan 19, 2022

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

else if (argv[configKey]) error = Error(__('Invalid JSON config file: %s', configPath))
but is, ultimately, because the call to mixin.resolve on
const resolvedConfigPath = mixin.resolve(mixin.cwd(), configPath)
is, by default, a call to path.resolve (owing to
resolve,
) 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

try {
let config = null
const resolvedConfigPath = mixin.resolve(mixin.cwd(), configPath)
const resolveConfig = flags.configs[configKey]
if (typeof resolveConfig === 'function') {
try {
config = resolveConfig(resolvedConfigPath)
} catch (e) {
config = e
}
if (config instanceof Error) {
error = config
return
}
} else {
config = mixin.require(resolvedConfigPath)
}
setConfigObject(config)
} catch (ex: any) {
// Deno will receive a PermissionDenied error if an attempt is
// made to load config without the --allow-read flag:
if (ex.name === 'PermissionDenied') error = ex
else if (argv[configKey]) error = Error(__('Invalid JSON config file: %s', configPath))
}
in a loop to handle the case that configPath is an array.

@Velewele
Copy link

Tt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants