-
Notifications
You must be signed in to change notification settings - Fork 140
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
Add support for parsing numbers according to JSON format #220
Conversation
Looks like a good idea. I will review and I expect to merge. |
include/fast_float/float_common.h
Outdated
@@ -16,6 +16,7 @@ enum chars_format { | |||
scientific = 1 << 0, | |||
fixed = 1 << 2, | |||
hex = 1 << 3, | |||
json = 1 << 4 | fixed | scientific, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obviously, this will cause a conflict with #219
but that's not a big issue.
tests/json_fmt.cpp
Outdated
|
||
int main() | ||
{ | ||
const std::vector<double> expected{ -0.2, 0.02, 0.002, 1., 0. }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both 1. and 0. are fine, but my expectation is that you could use 1 and 0. (This is not a request for you to change the code, just a comment.)
@mayawarrier I think that the JSON format does not allow a leading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not clear that this code disallows leading pluses, and I think it should.
Got it, I'll just ignore the FASTFLOAT_ALLOWS_LEADING_PLUS macro when the runtime format is json. |
- Most JSON parsers offer this option too
I disallowed leading pluses as requested. I also added an option to allow inf/nan in JSON mode. This is a common option in JSON parsers despite not being part of the standard. |
I am sure that this could be perfected, but it looks solid to me, merging. |
Details on the format are available here (RFC 8259): https://datatracker.ietf.org/doc/html/rfc8259#section-6
This is compatible with previous JSON RFCs as well as the number format hasn't changed.