Improve error when whitespace control is used on extends

[GuillaumeGomez]

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.

[GuillaumeGomez]

I just realized:

#[template(source = r#"{% extends "bla.html" %}

hello"#, ext = "html")]

What if I wanted to keep the whitespaces before "hello"? I'd need to add an empty comment right after extends, which isn't great either. With this, shouldn't it be better to actually handle whitespace control in extends?

[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.

[GuillaumeGomez]

Yep, seems like a weird way to handle it. Well, what do you want me to do in this case?

[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.

[djc]

Nit: I'd prefer to either write this with an early return (probably for the Ok case) or just as a match:

match (pws, nws) {
     (None, None) => Ok((i, Self { path })),
     (_, _) => Err(..),
}

[GuillaumeGomez]

👍

[djc]

Should this mention something about why this is the case?

[GuillaumeGomez]

Hum... I think it'd better to mention this in the book instead.

[djc]

I can live with that.

[GuillaumeGomez]

Done.

[djc]

Sorry for all the nits!

[GuillaumeGomez]

Sorry for all the nits!

Nah it's fine. I don't mind the review rounds.