search

projectfluent / fluent-rs

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

Switch to use ICU4X #335

Open zbraniecki opened 6 months ago

zbraniecki commented 6 months ago

Fixes #329

zbraniecki commented 6 months ago

No performance impact from the switch on any of the fluent-bundle perf tests.

zbraniecki commented 6 months ago

@dminor @aethanyc @makotokato - I'd like to ask for your architectural advice.

Historically, fluent-bundle was using intl_pluralrules which contained all CLDR locales baked in. With this switch we now use ICU4X, and in this PR so far I'm just using PluralRules::try_new which uses compiled data.

We will also want to add DateTimeFormat and NumberFormat, and all three are transitive. They are created based on FluentBundle and memoized using intl-memoizer.

My understanding is that for Mozilla use case, you'd prefer to create each component instance using custom DataProvider, so the shift here is that we'd like to have an option to pass such DataProvider instance to the FluentBundle constructor, and have it pass it to PluralRules/DateTimeFormat/NumberFormat etc. constructor. Is that accurate?

If so, this will require also changes to `L10nRegistry as this is where FluentBundle is generated.

waywardmonkeys commented 6 months ago

I have a pile of pending PRs here in this repo, one of which, #332, fixes the CI failure that you see here.

alerque commented 1 month ago

I'm going to rebase this on main to resolve the merge conflicts so it's that much easier to work on and build on. That way if somebody branches from this to keep working on it it should be able to integrate once things start coming together. In the process I'm running rustfmt and clippy --fix on the commits before rebasing as this pre-empts most of the merge conflicts that have cropped up. I'll manually take care of the ones that remain.

If by chance you have newer work on this branch than what is pushed here do feel free to force push and clobber my rebase. I can always redo it, but it would likely be easier to redo from scratch with my git rerere cache than for you to try to rebase on what I'm about to force push.

alerque commented 1 month ago

@zbraniecki One thing I noticed during the rebase is that the refactoring of intl-memoizer just blew away the inline documentation. I presume that was on purpose and because you think the docs would need to be overhauled, but there were a few documentation fixes in the main branch that could not be re-applied because there were no docs to apply them to. If and when you think docs are ready to be brought back I can maybe help with fitting some of the old pieces/changes into the new locations. For reference git diff a2cef6b..c904ac3 intl-memoizer is the relevant range of changes to the default branch since this was branched and of course a2cef6b..0e1b2e5 is the original range of commits you had here before I rebased them just now.

alerque commented 1 month ago

It looks like I goofed up some of the workspace related changes a little in the rebase, straightening that out now.