-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
settings_ui: Fix IEEE 754 floating point error when serializing value to JSON #40677
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
settings_ui: Fix IEEE 754 floating point error when serializing value to JSON #40677
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @dongdong867 on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
nice work @dongdong867 ! |
probably-neb
left a comment
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.
Awesome work. Thanks! I've pushed a couple commits, The first one is to make sure all f32 settings get serialized with rounding for consistency and to ensure that regardless of what a user has in their settings, changing it in the settings UI doesn't result in floating error in the settings file. The second is to add some basic tests.
I'm going to merge this once CI passes. Thanks again!
|
Thanks for the follow-up commits and the review! |
Closes #40556
Release Notes:
settings.jsonfileNote
Seems like there's another pull request fixing the centered layout in #40661 by normalizing the whole
settings.jsonfile when updating it. Not sure which is the better way to handle this issue.Description
This pull request solves the IEEE 754 floating point error when serializing values from settings-ui into
settings.jsonby creating a twoserde_helperto convert the problematic f32 into a formatted two decimal placed f32.Fields currently exists the IEEE 754 error:
How to verify
Unnecessary Code Fade
as_is_unnecessary_code.mov
to_be_unnecessary_code.mov
Drop Size Target
as_is_drop_size_target.mov
to_be_drop_size_target.mov
Centered Layout Left/Right Padding
as_is_centered_layout.mov
to_be_centered_layout.mov
Inactive Opacity
as_is_inactive_opacity.mov
to_be_inactive_opacity.mov