-
Notifications
You must be signed in to change notification settings - Fork 424
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
feat: add float_format flag #367
base: master
Are you sure you want to change the base?
Conversation
This commit adds a float_format flag to easyjson which allows users to change the float format used in strconv. The default flag used is `g` by easyjson. If a user now provides a flag such as: bin/easyjson -float_format='f' ./tests/float_format.go It will use that format. The g format converts long floats into the e-notation. Using a flag such as 'f' might be necessary in certain usecases to maintain backward compatibility.
Hey @bulletmys, we internally use this patch and were wondering if it would be possible to upstream it? Since there was not much activity on the repo recently, thought to ping. |
@@ -39,6 +39,7 @@ type Generator struct { | |||
fieldNamer FieldNamer | |||
simpleBytes bool | |||
skipMemberNameUnescaping bool | |||
floatFmt string |
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.
why not byte?
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.
I'd made it string to keep it consistent with the incoming flag which would be string. Also, in case we use byte and pass it as empty, the generator sets it as:
w := jwriter.Writer{FloatFmt: "0"}
I could try figuring this out further if byte seems more relevant as the type of floatFmt
, but I guess it should be ok to keep it as string?
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.
You can check what value is correct on code generation stage and pass default value if cmd line parameter not passed
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.
So currently, since the flag package does not have support for byte/rune flags, it is a bit difficult to do this. I guess it might be okay to keep it as is (?), since the internal method converts the string to byte and sets the default value as well.
Hi @GoWebProd Just wanted to bump this up. |
Apologies for bumping this. We have been using the fork in production since March 2022. Would love to get it upstreamed. If any suggestions/tweaks I can do to align it with the project do let me know, would be happy to work on it. |
+1 Would be great if this PR could be considered. |
This commit adds a float_format flag to easyjson which allows
users to change the float format used in strconv. The default flag
used is
g
by easyjson which is kept as is to avoid a breaking change.If a user wishes, they can now provide a flag such as:
bin/easyjson -float_format='f' ./tests/float_format.go
The g format converts long floats into
e-notation. Using a flag such as 'f' might be necessary for certain
use cases to maintain backward compatibility.