projectfluent / fluent-rs

Rust implementation of Project Fluent
https://projectfluent.org
Apache License 2.0
1.05k stars 95 forks source link

Potential panic on 32-bit systems #189

Closed Plecra closed 1 year ago

Plecra commented 4 years ago

Anki uses a fork of the library to fix an apparent issue in fluent_bundle::types::FluentNumber::as_string

https://github.com/ankitects/fluent-rs/compare/2068c467a25b1da5d1a0d69540ca67c47bf14064..f61c5e10a53161ef5261f3c87b62047f12e4aa74

https://github.com/projectfluent/fluent-rs/blob/961450bbbfc2b20e9920b5aa1d5384a2dda3922e/fluent-bundle/src/types/number.rs#L133-L149

I don't think it's a good idea to just use the changes made for anki: they seem to defend against the panic on 32-bit systems, rather than properly solving it.

dae commented 4 years ago

Please contact me directly before bothering upstream crate authors in the future. Fluent have already implemented a fix for this panic - the reason Anki is using a forked version is because we also need maximum_fraction_digits, and have not had time to propose a separate patch for it. https://github.com/projectfluent/fluent-rs/pull/162#issuecomment-593247525

Plecra commented 4 years ago

My apologies! I'll be more careful when investigating what's been done from now on.

Would it be unreasonable to pull the float formatting out into a feature request for people watching the conversation?

gregtatum commented 1 year ago

This seems to be resolved.