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

Do not unquote integers with a leading zero #391

Merged
merged 3 commits into from
Dec 5, 2017

Conversation

phaer
Copy link
Contributor

@phaer phaer commented Dec 4, 2017

Integers with leading zeros are invalid in JSON, so they should probably stay quoted in. (They would be valid Javascript, but JSON does not support octal notation as stated on http://json.org.

Fixes #389

Integers with leading zeros are invalid in JSON, so they should probably stay quoted in. (They would be valid Javascript, but JSON does not support octal notation as stated on http://json.org.
@solarkennedy
Copy link
Contributor

Please add a test case for this. If we don't, then this particular functionality will be lost as consul_sorted_json changes.

@solarkennedy
Copy link
Contributor

@phaer does this also fix #283 ?

@phaer
Copy link
Contributor Author

phaer commented Dec 4, 2017

Just added tests and fixed the regex to still unquote for the case where the number is just '0'. As far as I understand, it does not fix #283 which looks for a way to keep valid integers quoted depending on key name. This PR is only about (invalid) integers with a leading zero. Sadly, it does not work the other way around either, a fix for #283 would not fix #389 .

@solarkennedy solarkennedy merged commit afe64be into voxpupuli:master Dec 5, 2017
@phaer phaer deleted the patch-1 branch December 5, 2017 18:19
@phaer phaer changed the title Do not unquote integers with a starting zero Do not unquote integers with a leading zero Dec 5, 2017
spuder pushed a commit to spuder/puppet-consul that referenced this pull request Feb 25, 2020
Do not unquote integers with a starting zero
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