I18n

[89Q12]

This PR is based on the work of jhoobergs and PR #434. I mainly just updated the code to work with the changes that have been made since the initial PR, and I split it into its own feature "localization" which can be renamed to maybe "with-localization". Also I added askama::Locale as suggested in #434.

Pending:

• Docs
• Improve the feature, mainly the use of #[cfg(feature = "localization")] since I'm new to rust/cargo

All tests pass. Looking at the old PR the TODO: "Fix the test for invalid text_id's" isn't really needed since we make use of a function from fluent that checks for a fallback language which is why the test doesn't panic, in fact it will never panic as seen here

[Kijewski]

Thank you for your work! I created a PR against your PR with a few suggestions: https://github.com/11Tuvork28/askama/pull/1

[djc]

I'm excited about this PR and also very busy for the next few days, so please have some patience with me!

[89Q12]

I'm excited about this PR and also very busy for the next few days, so please have some patience with me!

All good, no hurry :)

[Kijewski]

Sorry, https://github.com/11Tuvork28/askama/pull/2 is a bit huge again, but I guess it's worth it:

Check if the addressed messages exists, and if the right arguments were used, at compile time.

[89Q12]

Thanks!!, looks great although I haven't understood everything fully.

[89Q12]

So CI should be happier now but clippy will still complain about askama_derive/src/i18n.rs:298:17 and the test parser::tests::test_parse_localizestill fails. I will look into these things tomorrow oh and seems I made a typo in the commit message, its supposed to mean Vec<(PathBuf, Resource<String>)>

[Kijewski]

Sorry, I hope https://github.com/11Tuvork28/askama/pull/3 is the last PR :/

[89Q12]

Sorry, I hope 11Tuvork28#3 is the last PR :/

All good, thanks again

[pashinin]

So what about this PR?

[djc]

Might be good to put a high-level description of all the moving parts (that is, what we need from Fluent and how we connect that into Askama) in the PR here.

[89Q12]

Will do soon but please be patient with me, I'm currently moving to a new place so my time is limited but will do my best to comeback to this asap

[djc]

Sure, no real hurry. Good luck with the move!

[89Q12]

So the idea is that we have an abstraction layer on top of the fluent-templates crate, which in turn provides an easy of interacting with fluentitself. We do so by having a struct:

pub struct Locale<'a> {
    loader: &'a StaticLoader,
    language: LanguageIdentifier,
}

and we have a 2 functions on it

impl Locale<'_> {
    pub fn new(language: LanguageIdentifier, loader: &'static StaticLoader) -> Self {
        Self { loader, language }
    }

    pub fn translate<'a>(
        &self,
        text_id: &str,
        args: impl IntoIterator<Item = (&'a str, FluentValue<'a>)>,
    ) -> String {
        let args = HashMap::<&str, FluentValue<'_>>::from_iter(args);
        let args = match args.is_empty() {
            true => None,
            false => Some(&args),
        };
        self.loader.lookup_complete(&self.language, text_id, args)
    }
}

The translate function is just an abstraction on top on the fluent-templates, this makes it easier to deal with arguments and provide a stable interace that we can use.

Now the other idea is to give the user the ability to configure the loader via a config file and provide good defaults:

# Defaults to "en":
fallback_language = "en"

[fluent]
# Defaults to true:
use_isolating = true

# Defaults to "i18n":
assets_dir = "i18n"

# Default to None:
# core_locales = "…"

Now coming to the macros, the localization!() is used to create the static loader object from the config file,that is used in the Localestruct. Its supposed to be easier for the user and delivering a more full package for i18n like most other libs out there do. But the downside is of course that its one more thing to maintain, so I'm to suggestions.

Hope that I hope I could clear it up a bit more

[LeoniePhiline]

What needs to be done to move this PR forward, hoping for inclusion in 0.13?

I noticed that several open review questions might be easier answered by @Kijewski than by the original PR author, as much of the code in question (such as Unlazy) had been introduced by their PRs towards the forked branch.

I figured Kijewski might not have gotten notified of the review questions.

[89Q12]

Yeah, I don't know a bit of feedback wouldn't hurt about whats unclear. So I could explain it or refere to Kijewski. I have been occupied with personal things, hence why I haven't fixed CI yet.

[LeoniePhiline]

CI fails due to a newly introduced match at Add Expr::is_cachable(). The i18n branch is currently outdated and must be rebased.

@11Tuvork28 Could you rebase your branch on top of the current main? I'll then mark Expr::Localize(_, _) as cacheable via recursive arguments check.

[LeoniePhiline]

I implemented missing recursive is_cachable() for Expr::Localize after the fork was updated.

Would a maintainer please be so kind and approve the workflow? Thank you!

[LeoniePhiline]

Please excuse, my mistake. I had missed to check compilation with default features (without i18n).

This is fixed now in https://github.com/11Tuvork28/askama/pull/7, waiting to be merged into this PR branch.

Once https://github.com/11Tuvork28/askama/pull/7/commits/2bbba70d27a213cdaaa2645f5bc354cb2c871a9a has landed in this PR, the checks should succeed.

Edit: Landed - all checks pass now.

[djc]

@LeoniePhiline and @11Tuvork28 thanks for your patience with me and for the hard work on getting this ready. I'll try to make another pass soon, but feel obliged to warn you that I have a very busy week and generally a lot on my plate.

[TTWNO]

I'd like to pop in on this issue and ask if anything needs to be done to get this merged?

I'm writing a website for my hockey league that needs pretty advanced i18n that fluent provides; I have some time, so please let me know what needs to be done.

[89Q12]

I'd like to pop in on this issue and ask if anything needs to be done to get this merged?

I'm writing a website for my hockey league that needs pretty advanced i18n that fluent provides; I have some time, so please let me know what needs to be done.

Rebase for one would be good, then I'm not sure that this is even wanted anymore given that this hasn't gotten any anttention.

[djc]

Again, sorry to everyone following this PR for my lack of feedback here. Please be assured that I think about this PR at least once a week, but in its current state it's hard for me to justify taking out the time to review the large amount of changes here, compared to a bunch of other stuff (roughly across my family, my day job and other open source projects that I help maintain).

I guess the main thing that could help resolving this deadlock would be to represent the changes here as a series of a smaller PR that have one logical change per PR, so that I can review each of them quickly and thus we can make progress. I should probably have given this feedback sooner, but I figured I'd be able to get to it sooner.

I hope that someone is willing to take that one -- I really would like to get this work merged -- and thanks to everyone who has worked on it so far.

[89Q12]

Thanks for the feedback!! Much appreciated, I will try to take some time and break down the changes into logical blocks. And no worries about not getting to review stuff, its understandable we all have only 24 hours per day.

[TTWNO]

If you want to see a recently-rebased version you can take a look at my fork.

[89Q12]

If you want to see a recently-rebased version you can take a look at my fork.

Can you PR that to my fork? Would be sweet :) But I will take a look regardless, maybe we could work together on splitting it up? Would make it fast and more doable for me

[djc]

You can't really PR a rebase. However, as the PR author you can reset to the rebased commit/branch.

[mgmuesuu]

How is that/

[89Q12]

Closing in favor of #841 and its related PRs. Thanks again to everyone involved.