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

Make booleans work in config files #223

Closed
wants to merge 4 commits into from

Conversation

sgentle
Copy link
Contributor

@sgentle sgentle commented Aug 4, 2015

Hi there,

Booleans don't seem to be working in config files. I added a test and a potential fix, but I'm not sure if it's the best way to proceed. Happy for feedback!

@sgentle
Copy link
Contributor Author

sgentle commented Aug 4, 2015

I realised my original fix doesn't work when you want to override a boolean in the config from the cli.

@nexdrew
Copy link
Member

nexdrew commented Aug 6, 2015

Thanks @sgentle! I will try to take a look at this today. I'd like to get PR 222 settled before merging others, but hopefully that will be soon.

@nexdrew
Copy link
Member

nexdrew commented Aug 6, 2015

Looks pretty good to me.

The only potentially undesirable side-effect I have noticed is that the help() option and version() option can now be given a truthy value (e.g. true, 1, {}, []) from the config file. As you can imagine, this results in displaying the help content or version number when only specifying the config argument in command line, which is a little odd.

We obviously don't have this problem with the current version of yargs, but that's only because it ignores all boolean arguments defined in a config file, as you've found.

I think this is an acceptable oddity, but I'd rather defer to @bcoe's judgment.

@bcoe Thoughts? Seems to be a case of "you get what you ask for" to me. Otherwise, we'd have to update the parser to have knowledge of the help() and version() options.

@bcoe
Copy link
Member

bcoe commented Aug 7, 2015

I'll review and merge for early tomorrow. Love the fix, and nothing bad jumps out at me, but I'm always especially careful in code that touches the parser.

@bcoe
Copy link
Member

bcoe commented Aug 8, 2015

see #226

@bcoe bcoe closed this Aug 8, 2015
@bcoe
Copy link
Member

bcoe commented Aug 12, 2015

@sgentle I've published this to the next branch of yargs:

npm install yargs@next

Give this a shot, and let me know if it does the trick for you.

georgiyordanov added a commit to georgiyordanov/postcss-cli that referenced this pull request Mar 31, 2016
Due to yargs/yargs#223,
using booleans in the config file doesn't work if
`npm install` picks up a version of yargs lower than 3.18.1
georgiyordanov added a commit to georgiyordanov/postcss-cli that referenced this pull request Mar 31, 2016
Due to yargs/yargs#223, using booleans in the config
file doesn't work if `npm install` picks up a version of yargs lower than 3.18.1
georgiyordanov added a commit to georgiyordanov/postcss-cli that referenced this pull request Mar 31, 2016
Due to yargs/yargs#223, using booleans in the config
file doesn't work if `npm install` picks up a version of yargs lower than 3.18.1
georgiyordanov added a commit to georgiyordanov/postcss-cli that referenced this pull request Mar 31, 2016
Due to yargs/yargs#223, using booleans in the
config file doesn't work if `npm install` picks up a version of yargs
lower than 3.18.1
georgiyordanov added a commit to georgiyordanov/postcss-cli that referenced this pull request Mar 31, 2016
Due to yargs/yargs#223, using booleans in the
config file doesn't work if `npm install` picks up a version of yargs
lower than 3.18.1, so bump yargs to 3.32.0
watilde pushed a commit to watilde/postcss-cli that referenced this pull request Apr 14, 2016
Due to yargs/yargs#223, using booleans in the
config file doesn't work if `npm install` picks up a version of yargs
lower than 3.18.1, so bump yargs to 3.32.0
watilde pushed a commit to watilde/postcss-cli that referenced this pull request Aug 29, 2016
Due to yargs/yargs#223, using booleans in the
config file doesn't work if `npm install` picks up a version of yargs
lower than 3.18.1, so bump yargs to 3.32.0
watilde pushed a commit to watilde/postcss-cli that referenced this pull request Aug 29, 2016
Due to yargs/yargs#223, using booleans in the
config file doesn't work if `npm install` picks up a version of yargs
lower than 3.18.1, so bump yargs to 3.32.0
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

Successfully merging this pull request may close these issues.

None yet

3 participants