Make themes more flexible (#1004)

* Site templates can replace theme templates

* Integrate test case within test_site/

* Full backwards-compatibility with testcase in test_site

* Refine test case

* Call parent's block in child template for test case

* Check both templates are applied

* Follow testing advice

* Test for 'include' in themes and shortcodes

* Documentation for themes and how to extend them

Co-authored-by: Vincent Prouillet <balthek@gmail.com>

components/site/src/lib.rs

@@ -91,11 +91,7 @@ impl Site {
             );
             let mut tera_theme = Tera::parse(&theme_tpl_glob)
                 .map_err(|e| Error::chain("Error parsing templates from themes", e))?;
-            rewrite_theme_paths(
+            rewrite_theme_paths(&mut tera_theme, &theme);
-                &mut tera_theme,
-                tera.templates.values().map(|v| v.name.as_ref()).collect(),
-                &theme,
-            );
             // TODO: we do that twice, make it dry?
             if theme_path.join("templates").join("robots.txt").exists() {
                 tera_theme

components/utils/src/templates.rs

@@ -61,52 +61,21 @@ pub fn render_template(
     }
 }
 
-/// Rewrites the path from extend/macros of the theme used to ensure
+/// Rewrites the path of duplicate templates to include the complete theme path
-/// that they will point to the right place (theme/templates/...)
+/// Theme templates  will be injected into site templates, with higher priority for site
-/// Include is NOT supported as it would be a pain to add and using blocks
+/// templates. To keep a copy of the template in case it's being extended from a site template
-/// or macros is always better anyway for themes
+/// of the same name, we reinsert it with the theme path prepended
-/// This will also rename the shortcodes to NOT have the themes in the path
+pub fn rewrite_theme_paths(tera_theme: &mut Tera, theme: &str) {
-/// so themes shortcodes can be used.
+    let theme_basepath = format!("{}/templates/", theme);
-pub fn rewrite_theme_paths(tera_theme: &mut Tera, site_templates: Vec<&str>, theme: &str) {
+    let mut new_templates = HashMap::new();
-    let mut shortcodes_to_move = vec![];
+    for (key, template) in &tera_theme.templates {
-    let mut templates = HashMap::new();
+        let mut tpl = template.clone();
-    let old_templates = ::std::mem::replace(&mut tera_theme.templates, HashMap::new());
+        tpl.name = format!("{}{}", theme_basepath, key);
-
+        new_templates.insert(tpl.name.clone(), tpl);
-    // We want to match the paths in the templates to the new names
-    for (key, mut tpl) in old_templates {
-        tpl.name = format!("{}/templates/{}", theme, tpl.name);
-        // First the parent if there is one
-        // If a template with the same name is also in site, assumes it overrides the theme one
-        // and do not change anything
-        if let Some(ref p) = tpl.parent.clone() {
-            if !site_templates.contains(&p.as_ref()) {
-                tpl.parent = Some(format!("{}/templates/{}", theme, p));
-            }
-        }
-
-        // Next the macros import
-        let mut updated = vec![];
-        for &(ref filename, ref namespace) in &tpl.imported_macro_files {
-            updated.push((format!("{}/templates/{}", theme, filename), namespace.to_string()));
-        }
-        tpl.imported_macro_files = updated;
-
-        if tpl.name.starts_with(&format!("{}/templates/shortcodes", theme)) {
-            let new_name = tpl.name.replace(&format!("{}/templates/", theme), "");
-            shortcodes_to_move.push((key, new_name.clone()));
-            tpl.name = new_name;
-        }
-
-        templates.insert(tpl.name.clone(), tpl);
-    }
-
-    tera_theme.templates = templates;
-
-    // and then replace shortcodes in the Tera instance using the new names
-    for (old_name, new_name) in shortcodes_to_move {
-        let tpl = tera_theme.templates.remove(&old_name).unwrap();
-        tera_theme.templates.insert(new_name, tpl);
     }
+    // Contrary to tera.extend, hashmap.extend does replace existing keys
+    // We can safely extend because there's no conflicting paths anymore
+    tera_theme.templates.extend(new_templates);
 }
 
 #[cfg(test)]
@@ -117,7 +86,7 @@ mod tests {
     #[test]
     fn can_rewrite_all_paths_of_theme() {
         let mut tera = Tera::parse("test-templates/*.html").unwrap();
-        rewrite_theme_paths(&mut tera, vec!["base.html"], "hyde");
+        rewrite_theme_paths(&mut tera, "hyde");
         // special case to make the test work: we also rename the files to
         // match the imports
         for (key, val) in &tera.templates.clone() {
@@ -133,7 +102,7 @@ mod tests {
         );
         assert_eq!(
             tera.templates["hyde/templates/child.html"].parent,
-            Some("hyde/templates/index.html".to_string())
+            Some("index.html".to_string())
         );
     }
 }

docs/content/documentation/themes/creating-a-theme.md

@@ -51,11 +51,6 @@ theme, with live reload working as expected.
 Make sure to commit every directory (including `content`) in order for other people
 to be able to build the theme from your repository.
 
-### Caveat
-
-Please note that [include paths](https://tera.netlify.com/docs#include) can only be used in normal templates.
-Theme templates should use [macros](https://tera.netlify.com/docs#macros) instead.
-
 ## Submitting a theme to the gallery
 
 If you want your theme to be featured in the [themes](@/themes/_index.md) section

docs/content/documentation/themes/extending-a-theme.md

@@ -0,0 +1,21 @@
++++
+title = "Extending a theme"
+weight = 30
++++
+
+When your site uses a theme, you can replace parts of it in your site's templates folder. For any given theme template, you can either override a single block in it, or replace the whole template. If a site template and a theme template collide, the site template will be given priority. Whether a theme template collides or not, theme templates remain accessible from any template within `theme_name/templates/`.
+
+## Replacing a template
+
+When a site template and a theme template have the same path, for example `templates/page.html` and `themes/theme_name/templates/page.html`, the site template is the one that will be used. This is how you can replace a whole template for a theme.
+
+## Overriding a block
+
+If you don't want to replace a whole template, but override parts of it, you can [extend the template](https://tera.netlify.app/docs/#inheritance) and redefine some specific blocks. For example, if you want to override the `title` block in your theme's page.html, you can create a page.html file in your site templates with the following content:
+
+```
+{% extends "theme_name/templates/page.html" %}
+{% block title %}{{ page.title }}{% endblock %}
+```
+
+If you extend `page.html` and not `theme_name/templates/page.html` specifically, it will extend the site's page template if it exists, and the theme's page template otherwise. This makes it possible to override your theme's base template(s) from your site templates, as long as the theme templates do not hardcode the theme name in template paths. For instance, children templates in the theme should use `{% extends 'index.html' %}`, not `{% extends 'theme_name/templates/index.html' %}`.

test_site/templates/index.html

@@ -1,19 +1,6 @@
-<!DOCTYPE html>
-<html lang="en">
-    <head>
-      <meta charset="UTF-8">
-      <meta name="apple-mobile-web-app-capable" content="yes">
-      <meta name="viewport" content="width=device-width, initial-scale=1">
-      <meta name="description" content="{{ config.description }}">
-      <meta name="author" content="{{ config.extra.author.name }}">
-      <link href="https://fonts.googleapis.com/css?family=Fira+Mono|Fira+Sans|Merriweather" rel="stylesheet">
-      <link href="{{ get_url(path="/site.css", cachebust=true) | safe }}" rel="stylesheet">
-      <title>{{ config.title }}</title>
-    </head>
 
-  <body>
+{% extends 'sample/templates/index.html' %}
-    <div class="content">
+{% block content %}
-        {% block content %}
     <div class="list-posts">
         {% for page in section.pages %}
             <article>
@@ -21,9 +8,4 @@
             </article>
         {% endfor %}
     </div>
-        {% endblock content %}
+{% endblock content %}
-    </div>
-    <script src="{{ get_url(path="scripts/hello.js") | safe }}"
-            integrity="sha384-{{ get_file_hash(path="scripts/hello.js") }}"></script>
-  </body>
-</html>

test_site/templates/section.html

@@ -1,13 +1,7 @@
-{% extends "index.html" %}
+{% extends "sample/templates/section-specific-extends.html" %}
-
 {% block content %}
      {% for page in section.pages %}
         {{page.title}}
     {% endfor %}
-    {{ section.relative_path | safe }}
+    {{ super() }}
-    {% for sub in section.subsections %}
-        {% set subsection = get_section(path=sub) %}
-        {{subsection.title}}
-        Sub-pages: {{subsection.pages | length}}
-    {% endfor %}
 {% endblock content %}

test_site/themes/sample/templates/index.html

@@ -1 +1,19 @@
-Hello
+<!DOCTYPE html>
+<html lang="en">
+    <head>
+        <meta charset="UTF-8">
+        <meta name="apple-mobile-web-app-capable" content="yes">
+        <meta name="viewport" content="width=device-width, initial-scale=1">
+        <meta name="description" content="{{ config.description }}">
+        <meta name="author" content="{{ config.extra.author.name }}">
+        <link href="https://fonts.googleapis.com/css?family=Fira+Mono|Fira+Sans|Merriweather" rel="stylesheet">
+        <link href="{{ config.base_url }}/site.css" rel="stylesheet">
+        <title>{{ config.title }}</title>
+    </head>
+
+    <body>
+        <div class="content">
+        {% block content %}Hello{% endblock content %}
+        </div>
+    </body>
+</html>

test_site/themes/sample/templates/section-specific-extends.html

@@ -0,0 +1,9 @@
+{% extends "index.html" %}
+{% block content %}
+    {{ section.relative_path | safe }}
+    {% for sub in section.subsections %}
+        {% set subsection = get_section(path=sub) %}
+        {{subsection.title}}
+        Sub-pages: {{subsection.pages | length}}
+    {% endfor %}
+{% endblock content %}

test_site/themes/sample/templates/section.html

@@ -0,0 +1,4 @@
+{% extends 'index.html' %}
+{% block content %}
+I'm overriden in all cases so seeing me is a bug
+{% endblock %}

test_site/themes/sample/templates/shortcodes/pirate.html

@@ -0,0 +1 @@
+{% include 'shortcodes/pirate_included.html' %}

test_site/themes/sample/templates/shortcodes/pirate_included.html

@@ -0,0 +1 @@
+SHOULDNOTAPPEAR: overriden by site shortcode