Skip to content
This repository was archived by the owner on Mar 11, 2025. It is now read-only.

Fix nested templates block #1029

Merged
merged 4 commits into from
May 16, 2024
Merged

Fix nested templates block #1029

merged 4 commits into from
May 16, 2024

Conversation

GuillaumeGomez
Copy link
Collaborator

@GuillaumeGomez GuillaumeGomez commented May 2, 2024

Fixes #1022.

Problem was that we were still rendering the content into the WritableBuffer.

@GuillaumeGomez
Copy link
Collaborator Author

And fixed clippy lints.

@GuillaumeGomez GuillaumeGomez changed the title Nested templates block Fix nested templates block May 2, 2024
@@ -11,7 +11,7 @@ struct FragmentSimple<'a> {
fn test_fragment_simple() {
let simple = FragmentSimple { name: "world" };

assert_eq!(simple.render().unwrap(), "\n\n<p>Hello world!</p>\n");
assert_eq!(simple.render().unwrap(), "\n<p>Hello world!</p>");
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all lines removal, I'm not sure whether it was bad before and this PR fixed it as a side effect or the opposite...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this particular case, IMO the previous state was good and this PR breaks it.

{% block body %}
<p>Hello {{ name }}!</p>
{% endblock %}

Arguably this should render newlines before and after the p node?

@@ -1813,6 +1817,7 @@ struct Buffer {
indent: u8,
// Whether the output buffer is currently at the start of a line
start: bool,
discard: bool,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the previous PR added discard in WritableBuffer. Remind me why we have both and why we need to discard in both (instead of discarding in one of them only)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One is the actual string where we push content and the other is pending AST that wasn't pushed yet into the string.

So in this case, if we only want to render a specific named block, which is then followed by another one (which we're not interested into), both will get rendered. However, the block we want to render could also be in other blocks, so we still need to go through the whole tree of each present block, but only render the block that was specified.

#[derive(Template)]
#[template(path = "blocks.txt", block = "section")]
struct Section<'a> {
values: Vec<&'a str>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: why couldn't this just be a slice?


#[test]
fn test_specific_block() {
let values: Vec<&str> = vec!["a", "b", "c"];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: pretty sure this doesn't need the type annotation.

@@ -11,7 +11,7 @@ struct FragmentSimple<'a> {
fn test_fragment_simple() {
let simple = FragmentSimple { name: "world" };

assert_eq!(simple.render().unwrap(), "\n\n<p>Hello world!</p>\n");
assert_eq!(simple.render().unwrap(), "\n<p>Hello world!</p>");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this particular case, IMO the previous state was good and this PR breaks it.

{% block body %}
<p>Hello {{ name }}!</p>
{% endblock %}

Arguably this should render newlines before and after the p node?

@GuillaumeGomez
Copy link
Collaborator Author

Mystery solved (for the missing trailing backline characters): I accidentaly removed child.flush_ws(def.ws2);. Much less diff in the tests now. \o/

@rellfy
Copy link

rellfy commented May 3, 2024

I can confirm this resolved an issue I had where contents from the template leaked into the block-fragmented child template. I added another test case here: #1032

@djc djc mentioned this pull request May 7, 2024
7 tasks
@djc djc merged commit 4d237ab into askama-rs:main May 16, 2024
@GuillaumeGomez GuillaumeGomez deleted the nested-templates-block branch May 16, 2024 12:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compilation error when using nested template blocks with in a single template file
4 participants