projectfluent / fluent-rs

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

Switch FluentBundle to own its locale fallback chain #141

Closed zbraniecki closed 4 years ago

zbraniecki commented 4 years ago

This is an attempt to use the new LangId/LanguageNegotiation/Intl/Fluent combination based on the current state of all the underlying crates.

I'm currently using LanguageIdentifier because it's more mature, but once it settles, we will want to store a fallback chain of Locale because the data from it will be used to inform intl formatters such as datetimeformat/numberformat and pluralrules which may use unicode extension keys.

Now, is that what we want? Is it a good design? Would it make sense to implement some type like LocaleFallbackChain that would be based on Vec<Locale>?

@cmyr - can you confirm that this looks how you'd expect based on https://github.com/projectfluent/fluent-locale-rs/pull/15 ?

Especially I'm curious about the simple-app example where we collect request locales from the user, available from the system, negotiate, and the push to localization.

My one concern is that we force FluentBundle to own, while it doesn't need anything but references, but forcing references would require lifetimes... Maybe Vec<Cow<LanguageIdentifier>>? This would be particularly useful once we get to higher-level APIs like Localization in fluent-fallback where we get one list of locales, and we generate and re-generate FluentBundle's inside it - I think I'd prefer not to have to create a new Languageidentifier for each FluentBundle...

Pike commented 4 years ago

From a Localization POV, the Bundle could hold a slice of the Locale/LanguageIdentifier container owned by the Localization? Ask me next week if that makes any sense life-time-wise.

(Taking Rust training the second half of this week)

zbraniecki commented 4 years ago

Right, so yet another approach would be to allow FluentBundle to hold owned vec or a slice...

zbraniecki commented 4 years ago

Notice, that doing Cow<[LanguageIdentifier]> would require us to re-add a lifetime to FluentBundle.

I'm not complaining... ok, I am complaining because it's a lot of tedious work to mark it everywhere, but I'll do it if that's the right call :)

zbraniecki commented 4 years ago

My two cents on the conversation here: unless you are creating FluentBundles very frequently, it is probably not worth jumping through hoops in order to avoid cloneing a few LanguageIdentifiers. It just shouldn't be expensive enough to matter, and API ergonomics are important.

Good point. I'm not creating them very often. Once we mature the higher level crates, we may want to revisit this, because, as Axel pointed out, eventually we'll have sth like:

struct Localization {
  locales: Vec<Locale>,
  bundles: Vec<FluentBundle>
}

and each bundle will use a shrinking subset of the locales:

let locales = vec!["es-CL", "es-AR", "es", "en"];

Localization {
    locales: locales,
    bundles: vec![
        FluentBundle::new(vec!["es-CL", "es-AR", "es", "en"]),
        FluentBundle::new(vec!["es-AR", "es", "en"]),
        FluentBundle::new(vec!["es", "en"]),
        FluentBundle::new(vec!["en"]),
    ]
}

The bundles are lazy generated, but it would still be nice to only hand them the ref to the slice of the Localization::locales.

But we can address that later. For now, I'm closing this issue.