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

I18n #434

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

I18n #434

wants to merge 3 commits into from

Conversation

jhoobergs
Copy link

@jhoobergs jhoobergs commented Jan 13, 2021

This (draft) PR gives an initial onset for localization (#202) by using fluent-templates to statically load all fluent translations.
It is based on code of @kazimuth

@djc Could you give some quick remarks / questions?

TODO

  • Add with-i18n features
  • Fix the test for invalid text_id's (the test fails, as it should, but we should assert that it fails instead of failing)
  • Look into a better way to check if translations exist (currently catches a panic of fluent-templates, maybe they should use a Result?)
  • Documentation
  • Improve init_translations macro
  • Go through all remaining TODO comments

@djc
Copy link
Owner

djc commented Jan 15, 2021

Looks like a good start! I wonder if we can simplify a bit, though. Can we make a template more like this?

#[derive(Template)]
#[template(path = "i18n.html")]
struct UsesI18n<'a> {
    #[locale]
    loc: unic_langid::LanguageIdentifier,
    name: &'a str,
    hours: f64,
}

Where the type of the field marked as locale must implement a Localize trait. Let's hard-code a directory name for now (what's the usual directory name that projects using Fluent use?), that can be customized through askama.toml. I'm also curious what other template solutions use to call Fluent-like messages. localize(greetingsss, name: name) looks okay to me, though it seems a little different from what we have so far (since we don't do named arguments in general). Does Fluent always require named arguments, or are positional arguments also used?

(Since a language identifier will seemingly always just be parsed from a &str, we may at some point also want to allow &str as #[locale] attribute, taking care to bubble up errors from parsing the language identifier.)

@djc
Copy link
Owner

djc commented Jan 15, 2021

Also, thanks for working on this! I'm very excited to get this in!

@jhoobergs
Copy link
Author

Looks like a good start! I wonder if we can simplify a bit, though. Can we make a template more like this?

#[derive(Template)]
#[template(path = "i18n.html")]
struct UsesI18n<'a> {
    #[locale]
    loc: unic_langid::LanguageIdentifier,
    name: &'a str,
    hours: f64,
}

Where the type of the field marked as locale must implement a Localize trait. Let's hard-code a directory name for now (what's the usual directory name that projects using Fluent use?), that can be customized through askama.toml. I'm also curious what other template solutions use to call Fluent-like messages. localize(greetingsss, name: name) looks okay to me, though it seems a little different from what we have so far (since we don't do named arguments in general). Does Fluent always require named arguments, or are positional arguments also used?

(Since a language identifier will seemingly always just be parsed from a &str, we may at some point also want to allow &str as #[locale] attribute, taking care to bubble up errors from parsing the language identifier.)

How would we know to use MyLocalizer if it is not passed? Do you want to create a struct with a certain name that we can use? This would not allow having different Localizer's for different parts of the code. But this is probably not really a problem / not a valid use case. Allowing only once Localizer, does also allow using the toml file for the fluent-folder path. As far as I can see, there is not really a default folder name. i18n seems good to me.

Fluent does not allow positional arguments.

Allowing a &str for the localizer, is indeed useful.

@djc
Copy link
Owner

djc commented Jan 15, 2021

I guess my question is, what bits of state (other than the locale) does a Localizer need? And will most of these bits of state be per template or globally across a crate/project?

Copy link
Author

@jhoobergs jhoobergs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically: We need to initialize a static loader (with fallback language) and somehow have access to that loader in different parts of the code.

}
$v struct $n {
language: unic_langid::LanguageIdentifier,
loader: &'static fluent_templates::once_cell::sync::Lazy<fluent_templates::StaticLoader>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to be able to access this loader somehow. That's the reason for this struct.

// The directory of localisations and fluent resources.
locales: $locales,
// The language to falback on if something is not present.
fallback_language: $fallback_language,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback language can be added to the toml file.

][..];

// create default localizer
let localizer = super::#loc_ty::default();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we need to be able to somehow access the static loader. Currently an object of the generated struct type is used for that.

self.localized_messages.insert(message.clone());

buf.write(&format!(
"self.{}.translate(\"{}\", &std::iter::FromIterator::from_iter(vec![",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we need to be able to call the translate method. Currently it uses the Localizer object that has to be added to all Template structs.

@djc
Copy link
Owner

djc commented Aug 5, 2021

Huh, sorry I missed your responses on this -- are you still interested in working on this?

Having reviewed your code a bit more, how about having an Askama-provided type Locale, which is instantiated with a locale identifier and a reference to the StaticLoader. We then add a filter i18n(&str) which will expand to referencing the fields of the Locale?

@jhoobergs
Copy link
Author

@djc I would certainly be interested in working on this. I'm not completely sure if I understand your solution with the Askama-provided Locale-type. Could you write a simple example of the code that a user would write to use the localization? I guess this would consist of a rust struct, a template string and maybe some extra setup code?

@djc
Copy link
Owner

djc commented Aug 9, 2021

Something like this:

#[derive(Template)]
#[template(source = "{{ localize(\"hello\", age: 37) }}")]
struct UsesI18n<'a> {
    #[locale]
    loc: askama::Locale::new(lang_id, &TEMPLATES),
}

with this

struct Locale {
   ...
}

impl Locale {
     fn new(lang: unic_langid::LanguageIdentifier, templates: &fluent_templates::StaticLoader) { ... }
}

@89Q12 89Q12 mentioned this pull request Jun 17, 2022
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.

2 participants