-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix: Large numbers lose precision in localized messages #9300
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
base: main
Are you sure you want to change the base?
Conversation
|
I think the approach is still bugged, because you are leaving out the case of having a I had created a (rough) PR which I think functionally is more correct, but whose code is very ugly (yours is much more polished). You can see the relevant commit here: temp-mfontanaar@4bb0e01 That being said, I'm puzzled about how does your approach works. In my tests, it seemed like FluentArgs was always converting to a |
|
hmm |
|
not super happy that it will be added this in every call of translate!() |
|
Extracted logic to |
306880a to
a484d4d
Compare
CodSpeed Performance ReportMerging #9300 will improve performances by 8.22%Comparing Summary
Benchmarks breakdown
Footnotes
|
|
GNU testsuite comparison: |
fc18e0d to
9828d57
Compare
|
GNU testsuite comparison: |
|
i think it has a bit too much complexity for a corner case :/ |
The translate! macro lost precision for u64 > i64::MAX due to f64 fallback. Now parses u64 before f64 to maintain accuracy. Fixes uutils#9294
9828d57 to
52f4c2a
Compare
|
GNU testsuite comparison: |
Big numbers like
i64::MAXwere showing up wrong in error messages because fluent-rs converts everything to f64 internally. This breaks numbers beyond ±2^53.Quick fix: check the range first. If it's too big for f64 to handle safely, we pass it as a string instead. Now
9223372036854775807shows correctly instead of9223372036854776000.This is a temporary workaround until fluent-rs fixes their precision issue upstream.
Closes #9294
Related: projectfluent/fluent-rs#337