Skip to content
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

Add number truncation before back-end translation #3162

Merged

Conversation

lucasscharenbroch
Copy link
Contributor

Fixes #3102

When the front-end translate is called, all numbers are truncated to 2 decimal places.
The same was not true for the back-end translate, which seems to be the cause of this corner-case.

It appears that the Rust implementation of Fluent doesn't automatically do the truncation (before the comparison to [one] in FTL) according to the FluentNumberOptions (maximum_fraction_digits), while the JS version does. This is why my code explicitly rounds the number, where the TS only sets maximumFractionDigits.

@dae
Copy link
Member

dae commented Apr 23, 2024

Thanks for looking into this Lucas!

There are a couple of things that would be nice to have:

  • Instead of rebuilding the args into a new container, we could apply the transformation directly in the generated code. The strings.rs file we generate creates functions like this:
    /// {$count} cards added.
    #[inline]
    pub fn importing_cards_added<'a>(&'a self, count: impl Number) -> Cow<'a, str> {

        let mut args = FluentArgs::new();
        args.set("count", count.into());

        self.translate("importing-cards-added", Some(args))
    }

For numbers, we could call a function to modify them before calling .set(), and avoid the extra work.

  • A small unit test that verifies things work correctly (i.e. if the fix is removed, the unit test starts failing) would be good. You could pick a random string from the generated code to test with.

@lucasscharenbroch
Copy link
Contributor Author

Thanks, Dae!

I've made changes accordingly.

@dae
Copy link
Member

dae commented Apr 24, 2024

Nice and clean - thanks again!

@dae dae merged commit ee86835 into ankitects:main Apr 24, 2024
1 check passed
@lucasscharenbroch lucasscharenbroch deleted the add-number-truncation-before-translation branch April 24, 2024 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

“1 hours today”
2 participants