Add config file support #16
Conversation
adamreisnz
left a comment
adamreisnz
left a comment
There was a problem hiding this comment.
Hey @RyanZim, thanks for the PR. I'll have a closer look at it when I'm back from my trip next week.
With this approach, having a config file means you can't pass CLI arguments to "override" the config file if you will, or to only use the config file for common parameters while feeding from and/or to parameters via the CLI. I'd like to support this though, as it seems like a logical use case. Could you see if you can add that in? Otherwise I can take it from here when I'm back.
|
@adamreisnz Updated. Hope this is what you wanted, let me know if I misunderstood. |
2 similar comments
|
I think so, will test this on Monday when back from holiday. |
adamreisnz
left a comment
adamreisnz
left a comment
There was a problem hiding this comment.
A few small changes, rest looks good! Thanks!
bin/cli.js
Outdated
| const to = argv._.shift(); | ||
| let from; | ||
| let to; | ||
| let files; |
There was a problem hiding this comment.
Can we simplify this to a one liner let from, to, files;?
| config = require(path.join(process.cwd(), argv.config)); | ||
| } | ||
| catch (e) { | ||
| console.error(chalk.red('Cannot load config file')); |
There was a problem hiding this comment.
Probably need to add a console.error(chalk.red(e)); to notify the user of the error
bin/cli.js
Outdated
| } | ||
| from = config.from; | ||
| to = config.to; | ||
| if (typeof config.files === 'string') config.files = [config.files]; |
There was a problem hiding this comment.
This won't pass the linter, need to wrap if statements with curly brackets
bin/cli.js
Outdated
|
|
||
| if (!from) from = argv._.shift(); | ||
| if (!to) to = argv._.shift(); | ||
| if (!files) files = argv._; |
There was a problem hiding this comment.
Same here, needs curly brackets for the if statements. In addition, the checks need to explicitly check for undefined, otherwise you won't be able to replace with an empty string. E.g. if (typeof to === 'undefined')
There was a problem hiding this comment.
Good catch! Didn't think about undefined.
bin/cli.js
Outdated
| if (!to) to = argv._.shift(); | ||
| if (!files) files = argv._; | ||
|
|
||
| if (!from || !to) { |
|
Updated. |
2 similar comments
|
Thanks @adamreisnz! |
3 participants
Fixes #15.