[WIP] Apply number formatting when using FluentNumberOptions #377
Conversation
Changes: - Rstest crate has been added to simplify the unit tests definitions. - Add unit tests that cover the use cases that are meant to be fixed.
felipet
changed the title
felipet
changed the title
|
Hi @zbraniecki , sorry for bothering you (I saw you've got assigned as reviewer of the last PR), but no one seems to take over this issue. Could you point me to someone that could review this request? Thanks. |
These should correspond to the JS behaviour: new Intl.NumberFormat('en', { maximumFractionDigits: 3 }).format(12.34567)
// → "12.346"
new Intl.NumberFormat('en', { maximumSignificantDigits: 3 }).format(12.34567)
// → "12.3" |
Great, that makes it crystal clear. Thanks, @eemeli . However, I'd like to hear that someone will be available to review and eventually merge the code before getting into the implementation. |
This pull request aims to fill a gap found in the code regarding number formatting.
According to the code, 5 different format options are allowed regarding a number's length, considering rear and trailing zeroes when needed. These options are listed in the struct
FluentNumberOptions.However, I can't figure out what is the expected difference between
FluentNumberOptions.maximum_significant_digitsandFluentNumberOptions.maximum_fraction_digits; and betweenFluentNumberOptions.minimum_fraction_digitsandFluentNumberOptions.minimum_significant_digits. I'll consider those redundant, and I won't implement any specific formatting unless someone tells me what's the difference.The current implementation:
https://github.com/projectfluent/fluent-rs/blob/f2033ce8340e09000ad9efccd6215b3fa5c23496/fluent-bundle/src/types/number.rs#L148C1-L165C2
Only considers the option
FluentNumberOptions.minimum_fraction_digits, thus all the other options are simply ignored when provided. This was already reported by #368.I'd like to work on a proposal for the implementation of such missing logic. I've added a few unit tests that show what kind of formatting shall be expected when using the options. So the idea is to implement the code that make those test to pass.
Please, let me know if I'm missing something.
Notes for the reviewers: I added Rstest as a dev-dependency, if that is not acceptable, I'll rewrite the tests that I pushed.
Closes #368