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

WIP: Multiple templates per struct #600

Closed
wants to merge 6 commits into from
Closed

WIP: Multiple templates per struct #600

wants to merge 6 commits into from

Conversation

Kijewski
Copy link
Contributor

No description provided.

@djc
Copy link
Owner

djc commented Jan 12, 2022

Have you looked at #434? I think that's probably a better approach.

@Kijewski
Copy link
Contributor Author

Kijewski commented Jan 12, 2022

I don't think it's an either-or, both approaches could be used together.

I called my implementation "l10n", but it is also usable e.g. if you have different templates for desktop and mobile users:

#[template(path = "desktop/index.html", localizer = "is_mobile")]
#[l10n("true", "mobile/index.html")]
struct Index {}

Or to present an "eat our cookies!" template

#[template(path = "desktop/index.html", localizer = "cookie")]
#[l10n("None", "accept_cookie_form.html")]
struct Index {}

And for the standard i18n case, I guess I'd use my approach if you have two languages, and fluent if you have a lot of languages.

@gpmake
Copy link

gpmake commented Jan 12, 2022

I refactored my test code with WIP: I18n #600.

{https://github.com/gpmake/basic-actix-askama-multilingual-site.git, branch = "pr-multi-paths}

It works well and I find the proposed solution very clean.

I don't quite understand why you need to add the index.html file. What is the purpose?
In my code I had to insert these (empty) files otherwise it would not compile.

#[derive(Template)]
#[template(path = "localization/index.html", localizer = "language")]
#[l10n(r#""de""#, "localization/index.de.html")]
#[l10n(r#""en""#, "localization/index.en.html")]
#[l10n(r#""es""#, "localization/index.es.html")]
#[l10n(r#""fr""#, "localization/index.fr.html")]
struct LetDestructoringTuple<'a> {
    language: &'a str,
    user: &'a str,
}

@Kijewski
Copy link
Contributor Author

In my code I had to insert these (empty) files otherwise it would not compile.

The basic case is meant as a fallback. Probably I should make this optional if the other cases are exhaustive.

@djc
Copy link
Owner

djc commented Jan 12, 2022

I don't think we should call this i18n, it's more like multi-template support or something (my use case would be to implement templates for plaintext and HTML email). So can you paste some generated code here that shows wat this ends up generating?

@Kijewski
Copy link
Contributor Author

impl<'a> ::askama::Template for LetDestructoringTuple<'a> {
    fn render_into(&self, writer: &mut (impl ::std::fmt::Write + ?Sized)) -> ::askama::Result<()> {
        include_bytes!("…/askama/testing/templates/localization/index.es.html");
        include_bytes!("…/askama/testing/templates/localization/index.fr.html");
        include_bytes!("…/askama/testing/templates/localization/index.de.html");
        include_bytes!("…/askama/testing/templates/localization/index.html");
        include_bytes!("…/askama/testing/templates/localization/index.en.html");
        include_bytes!("…/askama/testing/templates/localization/base.html");
        match &self.language {
            &"de" => {
                write!(
                    writer,
                    "Hallo, {expr0}!",
                    expr0 = &::askama::MarkupDisplay::new_unsafe(&(self.user), ::askama::Html),
                )?;
            }
            &"en" => {
                write!(
                    writer,
                    "Hello, {expr1}!",
                    expr1 = &::askama::MarkupDisplay::new_unsafe(&(self.user), ::askama::Html),
                )?;
            }
            &"es" => {
                write!(
                    writer,
                    "¡Hola, {expr2}!",
                    expr2 = &::askama::MarkupDisplay::new_unsafe(&(self.user), ::askama::Html),
                )?;
            }
            &"fr" => {
                write!(
                    writer,
                    "Localization test:\nBonjour, {expr3} !",
                    expr3 = &::askama::MarkupDisplay::new_unsafe(&(self.user), ::askama::Html),
                )?;
            }
            _ => {
                write!(
                    writer,
                    "Not implemented: {expr4}",
                    expr4 = &::askama::MarkupDisplay::new_unsafe(&(self.language), ::askama::Html),
                )?;
            }
        }
        Ok(())
    }
    const EXTENSION: ::std::option::Option<&'static ::std::primitive::str> = Some("html");
    const SIZE_HINT: ::std::primitive::usize = 17;
    const MIME_TYPE: &'static ::std::primitive::str = "text/html; charset=utf-8";
}
impl<'a> ::std::fmt::Display for LetDestructoringTuple<'a> {
    #[inline]
    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        ::askama::Template::render_into(self, f).map_err(|_| ::std::fmt::Error {})
    }
}

@djc
Copy link
Owner

djc commented Jan 12, 2022

Okay, that seems pretty neat! I'll need to review this in more detail...

@Kijewski Kijewski changed the title WIP: I18n WIP: Multiple templates per struct Jan 12, 2022
@gpmake
Copy link

gpmake commented Jan 12, 2022

It is a neat improvement for my case, too! The PR title "Multiple templates per struct" also seems fitting fine.

@djc
Copy link
Owner

djc commented Jan 25, 2022

So one thing that this approach does not solve is having templates with different MIME types. I feel that is a fairly important use case here, and we should not commit to this approach if we think we might need another approach in order to have templates with different MIME types (but the same context type).

@Kijewski
Copy link
Contributor Author

Hm, maybe something like:

fn get_alternative_index(&self) -> usize {
    …the generated match block…
}

/// The template's extension, if provided
const EXTENSION: &'static [Option<&'static str>];

/// Provides a conservative estimate of the expanded length of the rendered template
const SIZE_HINT: &'static [usize];

/// The MIME type (Content-Type) of the data that gets rendered by this Template
const MIME_TYPE: &'static [&'static str];

?

@djc
Copy link
Owner

djc commented Feb 14, 2022

I think it ends up being a crappy fit for what the Template trait is supposed to mean. I think maybe this should be a different trait, like MultiTemplate or TemplateGroup?

@Kijewski
Copy link
Contributor Author

Closed in favor of #666 (comment). Unless someone wants to adopt this PR, but I don't think that it's much more useful than EnumTemplate.

@Kijewski Kijewski closed this Apr 21, 2022
@Kijewski Kijewski deleted the pr-multi-paths branch September 26, 2022 13:00
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.

3 participants