Repository: djc/askama
Pull request #954 (2024-01-22 14:38:16)

Improve error when whitespace control is used on extends
Took me a while to understand why `askama` was complaining about an `extends`. Realized that it was because whitespace control is not allowed on it. Even though it does nothing, I think it's not great user experience to that this opaque error with valid jinja code so I updated the parser to allow it.

djc: I think I'd prefer to improve the error message instead of allowing it in the parser as it could be confusing to have it doing nothing.
djc: I think the whitespace outside `extends` is always ignored because the top-level flow comes from the base template, and the child template only picks up things inside blocks. So it is actually ignored AFAIK.
djc: How hard would it be to emit a good error message for this? Maybe just parse it like it's allowed but then emit an error from the parser if any of the whitespace controls are non-default for `extends`?
GuillaumeGomez: Pretty easy. Updating.
GuillaumeGomez: Done,
GuillaumeGomez: Done.
GuillaumeGomez: > Sorry for all the nits!

Nah it's fine. I don't mind the review rounds.

Patch:
diff --git a/askama_parser/src/node.rs b/askama_parser/src/node.rs
index 3f5198c..16c5382 100644
--- a/askama_parser/src/node.rs
+++ b/askama_parser/src/node.rs
@@ -928,8 +928,22 @@ pub struct Extends<'a> {
 
 impl<'a> Extends<'a> {
     fn parse(i: &'a str) -> ParseResult<'a, Self> {
-        let (i, path) = preceded(ws(keyword("extends")), cut(ws(str_lit)))(i)?;
-        Ok((i, Self { path }))
+        let start = i;
+
+        let (i, (pws, _, (path, nws))) = tuple((
+            opt(Whitespace::parse),
+            ws(keyword("extends")),
+            cut(pair(ws(str_lit), opt(Whitespace::parse))),
+        ))(i)?;
+        match (pws, nws) {
+            (None, None) => Ok((i, Self { path })),
+            (_, _) => Err(nom::Err::Failure(ErrorContext {
+                input: start,
+                message: Some(Cow::Borrowed(
+                    "whitespace control is not allowed on `extends`",
+                )),
+            })),
+        }
     }
 }
 
diff --git a/book/src/template_syntax.md b/book/src/template_syntax.md
index 4bd9894..31b1eaa 100644
--- a/book/src/template_syntax.md
+++ b/book/src/template_syntax.md
@@ -328,6 +328,16 @@ blocks from the base template with those from the child template. Inside
 a block in a child template, the `super()` macro can be called to render
 the parent block's contents.
 
+Because top-level content from the child template is thus ignored, the `extends`
+tag doesn't support whitespace control:
+
+```html
+{%- extends "base.html" +%}
+```
+
+The above code is rejected because we used `-` and `+`. For more information
+about whitespace control, take a look [here](#whitespace-control).
+
 ## HTML escaping
 
 Askama by default escapes variables if it thinks it is rendering HTML


Test patch:
diff --git a/testing/tests/ui/extends.rs b/testing/tests/ui/extends.rs
new file mode 100644
index 0000000..55c8b56
--- /dev/null
+++ b/testing/tests/ui/extends.rs
@@ -0,0 +1,18 @@
+use askama::Template;
+
+#[derive(Template)]
+#[template(
+    source = r#"{%- extends "whatever.html" %}"#,
+    ext = "html"
+)]
+struct ExtendsPre;
+
+#[derive(Template)]
+#[template(
+    source = r#"{% extends "whatever.html" -%}"#,
+    ext = "html"
+)]
+struct ExtendsPost;
+
+fn main() {
+}
diff --git a/testing/tests/ui/extends.stderr b/testing/tests/ui/extends.stderr
new file mode 100644
index 0000000..346246d
--- /dev/null
+++ b/testing/tests/ui/extends.stderr
@@ -0,0 +1,19 @@
+error: whitespace control is not allowed on `extends`
+       problems parsing template source at row 1, column 2 near:
+       "- extends \"whatever.html\" %}"
+ --> tests/ui/extends.rs:3:10
+  |
+3 | #[derive(Template)]
+  |          ^^^^^^^^
+  |
+  = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: whitespace control is not allowed on `extends`
+       problems parsing template source at row 1, column 2 near:
+       " extends \"whatever.html\" -%}"
+  --> tests/ui/extends.rs:10:10
+   |
+10 | #[derive(Template)]
+   |          ^^^^^^^^
+   |
+   = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)