Repository: djc/askama (rust)

Issue:
Proper file extension for templates for plugin integration?
First of all, let me congratulate you for your work. I'm developing an application with Askama and Rocket, and I'm very happy with the results. However, I recently hit an interesting bump. This was using VS Code as my development environment:

- As the templates are all HTML, I'm just using `.html` extension.
- The HTML formatting plugins don't recognize Askama templates. That means no syntax highlighting, no error detection, and no proper formatting. In fact one plugin completely mangled a few templates and caused my website to crash.
- As Askama is based on Jinja2, I tried to rename my files as `.html.j2` and use a Jinja2 plugin (no Askama plugin seems to exist), and lo and behold, it worked perfectly. Syntax highlighting and proper formatting. The code compiles and runs flawlessly, but...
- Askama's Rocket responder implementation guesses the content type from the file extension, but `.html.j2` is not recognized, so it just returns an empty response! No error message, no nothing, just a totally blank page.

So my question is, how can we fix this? I have a couple ideas:

- Write a proper Askama plugin with its own file extension, or reuse an existing Jinja2 plugin, even if the two syntaxes don't match perfectly?
- Fix the responders implementations to strip the last part of the extension from the file name, in case it matches a predefined list.

Pull request description:
Fixed extension to ignore common jinja extensions
Fixes #457

- Fixed so `extension()` for `foo.html.{jinja,jinja2,j2}` resolves to `html` instead of `{jinja,jinja2,j2}`.
- Changed to allow both `path` and `ext`, i.e. so it would allow `#[template(path = "template.foo.bar.baz", ext = "html")]`

_This was just a quick fix, so I hope I didn't overlook anything_


Patch:
diff --git a/askama_shared/src/generator.rs b/askama_shared/src/generator.rs
index b4363a8..f8e2d8f 100644
--- a/askama_shared/src/generator.rs
+++ b/askama_shared/src/generator.rs
@@ -156,10 +156,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
         buf.writeln("}")?;
 
         buf.writeln("fn extension(&self) -> Option<&'static str> {")?;
-        buf.writeln(&format!(
-            "{:?}",
-            self.input.path.extension().map(|s| s.to_str().unwrap())
-        ))?;
+        buf.writeln(&format!("{:?}", self.input.extension()))?;
         buf.writeln("}")?;
 
         buf.writeln("fn size_hint(&self) -> usize {")?;
@@ -175,10 +172,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
         buf.writeln("}")?;
 
         buf.writeln("fn extension() -> Option<&'static str> {")?;
-        buf.writeln(&format!(
-            "{:?}",
-            self.input.path.extension().map(|s| s.to_str().unwrap())
-        ))?;
+        buf.writeln(&format!("{:?}", self.input.extension()))?;
         buf.writeln("}")?;
 
         buf.writeln("}")?;
@@ -235,10 +229,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
             "fn into_response(self, _state: &::askama_gotham::State)\
              -> ::askama_gotham::Response<::askama_gotham::Body> {",
         )?;
-        let ext = match self.input.path.extension() {
-            Some(s) => s.to_str().unwrap(),
-            None => "txt",
-        };
+        let ext = self.input.extension().unwrap_or("txt");
         buf.writeln(&format!("::askama_gotham::respond(&self, {:?})", ext))?;
         buf.writeln("}")?;
         buf.writeln("}")
@@ -256,12 +247,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
             "res.body = Some(Box::new(::askama_iron::Template::render(&self).unwrap().into_bytes()));",
         )?;
 
-        let ext = self
-            .input
-            .path
-            .extension()
-            .map_or("", |s| s.to_str().unwrap_or(""));
-        match ext {
+        match self.input.extension().unwrap_or("") {
             "html" | "htm" => {
                 buf.writeln("::askama_iron::ContentType::html().0.modify(res);")?;
             }
@@ -314,7 +300,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
 
         buf.writeln(&format!(
             "::mendes::askama::into_response(app, req, &self, {:?})",
-            self.input.path.extension()
+            self.input.extension()
         ))?;
         buf.writeln("}")?;
         buf.writeln("}")?;
@@ -335,10 +321,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
              -> ::askama_rocket::Result<'askama> {",
         )?;
 
-        let ext = match self.input.path.extension() {
-            Some(s) => s.to_str().unwrap(),
-            None => "txt",
-        };
+        let ext = self.input.extension().unwrap_or("txt");
         buf.writeln(&format!("::askama_rocket::respond(&self, {:?})", ext))?;
 
         buf.writeln("}")?;
@@ -347,12 +330,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
     }
 
     fn impl_tide_integrations(&mut self, buf: &mut Buffer) -> Result<(), CompileError> {
-        let ext = self
-            .input
-            .path
-            .extension()
-            .and_then(|s| s.to_str())
-            .unwrap_or("txt");
+        let ext = self.input.extension().unwrap_or("txt");
 
         self.write_header(
             buf,
@@ -375,12 +353,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
     fn impl_warp_reply(&mut self, buf: &mut Buffer) -> Result<(), CompileError> {
         self.write_header(buf, "::askama_warp::warp::reply::Reply", None)?;
         buf.writeln("fn into_response(self) -> ::askama_warp::warp::reply::Response {")?;
-        let ext = self
-            .input
-            .path
-            .extension()
-            .and_then(|s| s.to_str())
-            .unwrap_or("txt");
+        let ext = self.input.extension().unwrap_or("txt");
         buf.writeln(&format!("::askama_warp::reply(&self, {:?})", ext))?;
         buf.writeln("}")?;
         buf.writeln("}")
diff --git a/askama_shared/src/input.rs b/askama_shared/src/input.rs
index 59f1303..8986fea 100644
--- a/askama_shared/src/input.rs
+++ b/askama_shared/src/input.rs
@@ -1,6 +1,6 @@
 use crate::{CompileError, Config, Syntax};
 
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
 use quote::ToTokens;
@@ -124,11 +124,8 @@ impl<'a> TemplateInput<'a> {
         // of `ext` is merged into a synthetic `path` value here.
         let source = source.expect("template path or source not found in attributes");
         let path = match (&source, &ext) {
-            (&Source::Path(ref path), None) => config.find_template(path, None)?,
+            (&Source::Path(ref path), _) => config.find_template(path, None)?,
             (&Source::Source(_), Some(ext)) => PathBuf::from(format!("{}.{}", ast.ident, ext)),
-            (&Source::Path(_), Some(_)) => {
-                return Err("'ext' attribute cannot be used with 'path' attribute".into())
-            }
             (&Source::Source(_), None) => {
                 return Err("must include 'ext' attribute when using 'source' attribute".into())
             }
@@ -198,6 +195,24 @@ impl<'a> TemplateInput<'a> {
             path,
         })
     }
+
+    pub fn extension(&self) -> Option<&str> {
+        self.ext.as_deref().or_else(|| extension(&self.path))
+    }
+}
+
+fn extension(path: &Path) -> Option<&str> {
+    let ext = path.extension().map(|s| s.to_str().unwrap())?;
+
+    const JINJA_EXTENSIONS: [&str; 3] = ["j2", "jinja", "jinja2"];
+    if JINJA_EXTENSIONS.contains(&ext) {
+        Path::new(path.file_stem().unwrap())
+            .extension()
+            .map(|s| s.to_str().unwrap())
+            .or(Some(ext))
+    } else {
+        Some(ext)
+    }
 }
 
 pub enum Source {
@@ -227,3 +242,51 @@ impl FromStr for Print {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_ext() {
+        assert_eq!(extension(Path::new("foo-bar.txt")), Some("txt"));
+        assert_eq!(extension(Path::new("foo-bar.html")), Some("html"));
+        assert_eq!(extension(Path::new("foo-bar.unknown")), Some("unknown"));
+
+        assert_eq!(extension(Path::new("foo/bar/baz.txt")), Some("txt"));
+        assert_eq!(extension(Path::new("foo/bar/baz.html")), Some("html"));
+        assert_eq!(extension(Path::new("foo/bar/baz.unknown")), Some("unknown"));
+    }
+
+    #[test]
+    fn test_double_ext() {
+        assert_eq!(extension(Path::new("foo-bar.html.txt")), Some("txt"));
+        assert_eq!(extension(Path::new("foo-bar.txt.html")), Some("html"));
+        assert_eq!(extension(Path::new("foo-bar.txt.unknown")), Some("unknown"));
+
+        assert_eq!(extension(Path::new("foo/bar/baz.html.txt")), Some("txt"));
+        assert_eq!(extension(Path::new("foo/bar/baz.txt.html")), Some("html"));
+        assert_eq!(
+            extension(Path::new("foo/bar/baz.txt.unknown")),
+            Some("unknown")
+        );
+    }
+
+    #[test]
+    fn test_skip_jinja_ext() {
+        assert_eq!(extension(Path::new("foo-bar.html.j2")), Some("html"));
+        assert_eq!(extension(Path::new("foo-bar.html.jinja")), Some("html"));
+        assert_eq!(extension(Path::new("foo-bar.html.jinja2")), Some("html"));
+
+        assert_eq!(extension(Path::new("foo/bar/baz.txt.j2")), Some("txt"));
+        assert_eq!(extension(Path::new("foo/bar/baz.txt.jinja")), Some("txt"));
+        assert_eq!(extension(Path::new("foo/bar/baz.txt.jinja2")), Some("txt"));
+    }
+
+    #[test]
+    fn test_only_jinja_ext() {
+        assert_eq!(extension(Path::new("foo-bar.j2")), Some("j2"));
+        assert_eq!(extension(Path::new("foo-bar.jinja")), Some("jinja"));
+        assert_eq!(extension(Path::new("foo-bar.jinja2")), Some("jinja2"));
+    }
+}


Test patch:
diff --git a/testing/templates/foo.html b/testing/templates/foo.html
new file mode 100644
index 0000000..85fa64c
--- /dev/null
+++ b/testing/templates/foo.html
@@ -0,0 +1,1 @@
+foo.html
\ No newline at end of file
diff --git a/testing/templates/foo.html.jinja b/testing/templates/foo.html.jinja
new file mode 100644
index 0000000..dc645f3
--- /dev/null
+++ b/testing/templates/foo.html.jinja
@@ -0,0 +1,1 @@
+foo.html.jinja
\ No newline at end of file
diff --git a/testing/templates/foo.jinja b/testing/templates/foo.jinja
new file mode 100644
index 0000000..f47c4d5
--- /dev/null
+++ b/testing/templates/foo.jinja
@@ -0,0 +1,1 @@
+foo.jinja
\ No newline at end of file
diff --git a/testing/tests/ext.rs b/testing/tests/ext.rs
new file mode 100644
index 0000000..5ed4e72
--- /dev/null
+++ b/testing/tests/ext.rs
@@ -0,0 +1,67 @@
+use askama::Template;
+
+#[derive(Template)]
+#[template(path = "foo.html")]
+struct PathHtml;
+
+#[test]
+fn test_path_ext_html() {
+    let t = PathHtml;
+    assert_eq!(t.render().unwrap(), "foo.html");
+    assert_eq!(t.extension(), Some("html"));
+}
+
+#[derive(Template)]
+#[template(path = "foo.jinja")]
+struct PathJinja;
+
+#[test]
+fn test_path_ext_jinja() {
+    let t = PathJinja;
+    assert_eq!(t.render().unwrap(), "foo.jinja");
+    assert_eq!(t.extension(), Some("jinja"));
+}
+
+#[derive(Template)]
+#[template(path = "foo.html.jinja")]
+struct PathHtmlJinja;
+
+#[test]
+fn test_path_ext_html_jinja() {
+    let t = PathHtmlJinja;
+    assert_eq!(t.render().unwrap(), "foo.html.jinja");
+    assert_eq!(t.extension(), Some("html"));
+}
+
+#[derive(Template)]
+#[template(path = "foo.html", ext = "txt")]
+struct PathHtmlAndExtTxt;
+
+#[test]
+fn test_path_ext_html_and_ext_txt() {
+    let t = PathHtmlAndExtTxt;
+    assert_eq!(t.render().unwrap(), "foo.html");
+    assert_eq!(t.extension(), Some("txt"));
+}
+
+#[derive(Template)]
+#[template(path = "foo.jinja", ext = "txt")]
+struct PathJinjaAndExtTxt;
+
+#[test]
+fn test_path_ext_jinja_and_ext_txt() {
+    let t = PathJinjaAndExtTxt;
+    assert_eq!(t.render().unwrap(), "foo.jinja");
+    assert_eq!(t.extension(), Some("txt"));
+}
+
+#[derive(Template)]
+#[template(path = "foo.html.jinja", ext = "txt")]
+struct PathHtmlJinjaAndExtTxt;
+
+#[test]
+fn test_path_ext_html_jinja_and_ext_txt() {
+    let t = PathHtmlJinjaAndExtTxt;
+    assert_eq!(t.render().unwrap(), "foo.html.jinja");
+    assert_eq!(t.extension(), Some("txt"));
+}