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

fix: support --set boolean and numberic params #20

Merged
merged 2 commits into from
Sep 23, 2023

Conversation

jbedard
Copy link
Contributor

@jbedard jbedard commented Sep 22, 2023

Didn't think of this one at first. Note that we'll have the same issue with numbers if there are any numeric options.

@songkeys
Copy link
Owner

Note that we'll have the same issue with numbers if there are any numeric options.

Are there any numeric options available currently? Can we implement the logic as a precautionary measure?

@jbedard
Copy link
Contributor Author

jbedard commented Sep 22, 2023

Yeah, I could do that if you'd like?

Note that just like boolean it will be an assumption that anything that "looks like" a number will be parsed as a number, just like "true" and "false" strings "look like" booleans are parsed as booleans. I think it's a safe assumption for now, especially since I don't think there are any variable string options?

@songkeys
Copy link
Owner

Yes, I agree. Please do it. Also, I noticed that this parameter is missing in the readme document. Could you also include it there?

@jbedard
Copy link
Contributor Author

jbedard commented Sep 23, 2023

Updated 👍

@songkeys songkeys merged commit ed8dea2 into songkeys:main Sep 23, 2023
3 of 4 checks passed
@songkeys songkeys changed the title fix: support --set boolean params fix: support --set boolean and numberic params Sep 23, 2023
@songkeys
Copy link
Owner

Thank you for the effort!

@jbedard jbedard deleted the extends-options-bool branch September 23, 2023 00:35
@jbedard
Copy link
Contributor Author

jbedard commented Sep 23, 2023

Thanks for the quick review! 👍

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.

2 participants