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 boolean literals work in the value of object #17

Merged
merged 1 commit into from Mar 9, 2016

Conversation

Projects
None yet
2 participants
@itchyny
Contributor

itchyny commented Mar 9, 2016

Hi, thank you for your great work! The jo command must be a promising cli tool for generating a JSON value in terminal!

First thing that I thought it inconvenient is the boolean syntax. This pull request enables me to use boolean literals in more readable syntax than @0, @1, or @T.

 $ ./jo foo=true
{"foo":true}

Oh, with this change, we cannot specify a string "true" for values of object. I want to make the following work but I do not know how to change the code.

 $ ./jo foo="true"
{"foo":"true"}
@jpmens

This comment has been minimized.

Show comment
Hide comment
@jpmens

jpmens Mar 9, 2016

Owner

I do not disagree that the current syntax for booleans is a quirk, and I also think =true looks better. However, this patch would break existing usage (which, admittedly can't be large because the project is so young). I have to think about this a bit...

As you've correctly determined, your second example cannot work: the Shell already strips the quotes before jo gets a chance to see the value, so the invocation would have to be something like one of these two:

./jo foo='"true"'    # single and double quotes
{"foo":"true"}

./jo foo=\"true\"
{"foo":"true"}

which is not particularly nice to look at.

I think the question is: which is typically more natural to want? {"switch": true} (as a real boolean) or {"switch" : "true"} as a string?

Owner

jpmens commented Mar 9, 2016

I do not disagree that the current syntax for booleans is a quirk, and I also think =true looks better. However, this patch would break existing usage (which, admittedly can't be large because the project is so young). I have to think about this a bit...

As you've correctly determined, your second example cannot work: the Shell already strips the quotes before jo gets a chance to see the value, so the invocation would have to be something like one of these two:

./jo foo='"true"'    # single and double quotes
{"foo":"true"}

./jo foo=\"true\"
{"foo":"true"}

which is not particularly nice to look at.

I think the question is: which is typically more natural to want? {"switch": true} (as a real boolean) or {"switch" : "true"} as a string?

@jpmens

This comment has been minimized.

Show comment
Hide comment
@jpmens

jpmens Mar 9, 2016

Owner

I've decided to take this and make it default behavior. Thanks for your contribution!

I will add an option to suppress this behavior.

Owner

jpmens commented Mar 9, 2016

I've decided to take this and make it default behavior. Thanks for your contribution!

I will add an option to suppress this behavior.

@jpmens jpmens merged commit 02d3724 into jpmens:master Mar 9, 2016

@itchyny itchyny deleted the itchyny:refine_boolean_syntax branch Mar 9, 2016

@itchyny

This comment has been minimized.

Show comment
Hide comment
@itchyny

itchyny Mar 9, 2016

Contributor

Thank you so much for your quick response and I appreciate your decision. I missed the point that the shell truncates the quotes and ./jo foo='"true"' might be a little complicated. But yet, =true and =false are actually easy to write by hand, human friendly syntax so thanks for merging the pull request. I think I rarely want to set booleans in the string form in my real work; I've never met an API in web applications which make me specify string booleans. BTW, I think the first example in README gets better if you replace with the new boolean syntax.

Contributor

itchyny commented Mar 9, 2016

Thank you so much for your quick response and I appreciate your decision. I missed the point that the shell truncates the quotes and ./jo foo='"true"' might be a little complicated. But yet, =true and =false are actually easy to write by hand, human friendly syntax so thanks for merging the pull request. I think I rarely want to set booleans in the string form in my real work; I've never met an API in web applications which make me specify string booleans. BTW, I think the first example in README gets better if you replace with the new boolean syntax.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment