Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nested templates block #1029

Merged
merged 4 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 24 additions & 22 deletions askama_derive/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ impl<'a> Generator<'a> {
break;
}
}
let current_buf = mem::take(&mut self.buf_writable);
let current_buf = mem::take(&mut self.buf_writable.buf);

self.prepare_ws(filter.ws1);
let mut size_hint = self.handle(ctx, &filter.nodes, buf, AstLevel::Nested)?;
Expand Down Expand Up @@ -734,7 +734,7 @@ impl<'a> Generator<'a> {
}
};

mem::drop(mem::replace(&mut self.buf_writable, current_buf));
self.buf_writable.buf = current_buf;

let mut filter_buf = Buffer::new(buf.indent);
let Filter {
Expand Down Expand Up @@ -892,13 +892,6 @@ impl<'a> Generator<'a> {
name: Option<&'a str>,
outer: Ws,
) -> Result<usize, CompileError> {
let block_fragment_write = self.input.block == name && self.buf_writable.discard;

// Allow writing to the buffer if we're in the block fragment
if block_fragment_write {
self.buf_writable.discard = false;
}

// Flush preceding whitespace according to the outer WS spec
self.flush_ws(outer);

Expand All @@ -917,6 +910,15 @@ impl<'a> Generator<'a> {
(None, None) => return Err("cannot call 'super()' outside block".into()),
};

self.write_buf_writable(buf)?;

let block_fragment_write = self.input.block == name && self.buf_writable.discard;
// Allow writing to the buffer if we're in the block fragment
if block_fragment_write {
self.buf_writable.discard = false;
}
let prev_buf_discard = mem::replace(&mut buf.discard, self.buf_writable.discard);

// Get the block definition from the heritage chain
let heritage = self
.heritage
Expand Down Expand Up @@ -961,9 +963,9 @@ impl<'a> Generator<'a> {
// Need to flush the buffer before popping the variable stack
child.write_buf_writable(buf)?;
}

child.flush_ws(def.ws2);
let buf_writable = mem::take(&mut child.buf_writable);
self.buf_writable = buf_writable;
self.buf_writable = child.buf_writable;

// Restore original block context and set whitespace suppression for
// succeeding whitespace according to the outer WS spec
Expand All @@ -973,6 +975,7 @@ impl<'a> Generator<'a> {
if block_fragment_write {
self.buf_writable.discard = true;
}
buf.discard = prev_buf_discard;

Ok(size_hint)
}
Expand Down Expand Up @@ -1024,7 +1027,7 @@ impl<'a> Generator<'a> {
.all(|w| matches!(w, Writable::Lit(_)))
{
let mut buf_lit = Buffer::new(0);
for s in mem::take(&mut self.buf_writable) {
for s in mem::take(&mut self.buf_writable.buf) {
if let Writable::Lit(s) = s {
buf_lit.write(s);
};
Expand All @@ -1044,7 +1047,7 @@ impl<'a> Generator<'a> {
let mut buf_format = Buffer::new(0);
let mut buf_expr = Buffer::new(indent + 1);

for s in mem::take(&mut self.buf_writable) {
for s in mem::take(&mut self.buf_writable.buf) {
match s {
Writable::Lit(s) => {
buf_format.write(&s.replace('{', "{{").replace('}', "}}"));
Expand Down Expand Up @@ -1814,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
Owner

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

}

impl Buffer {
Expand All @@ -1822,10 +1826,14 @@ impl Buffer {
buf: String::new(),
indent,
start: true,
discard: false,
}
}

fn writeln(&mut self, s: &str) -> Result<(), CompileError> {
if self.discard {
return Ok(());
}
if s == "}" {
self.dedent()?;
}
Expand All @@ -1841,6 +1849,9 @@ impl Buffer {
}

fn write(&mut self, s: &str) {
if self.discard {
return;
}
if self.start {
for _ in 0..(self.indent * 4) {
self.buf.push(' ');
Expand Down Expand Up @@ -2119,15 +2130,6 @@ impl<'a> WritableBuffer<'a> {
}
}

impl<'a> IntoIterator for WritableBuffer<'a> {
type Item = Writable<'a>;
type IntoIter = <Vec<Writable<'a>> as IntoIterator>::IntoIter;

fn into_iter(self) -> Self::IntoIter {
self.buf.into_iter()
}
}

impl<'a> Deref for WritableBuffer<'a> {
type Target = [Writable<'a>];

Expand Down
11 changes: 11 additions & 0 deletions testing/templates/blocks.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{% block index %}
Section: {{ s1 }}
{% endblock %}

{% block section -%}
[
{%- for value in values -%}
{{ value }}
{%- endfor -%}
]
{%- endblock %}
32 changes: 27 additions & 5 deletions testing/tests/block_fragments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>\n");
}

#[derive(Template)]
Expand All @@ -28,7 +28,7 @@ fn test_fragment_super() {

assert_eq!(
sup.render().unwrap(),
"\n\n<p>Hello world!</p>\n\n<p>Parent body content</p>\n\n"
"\n<p>Hello world!</p>\n\n<p>Parent body content</p>\n\n"
);
}

Expand All @@ -43,7 +43,7 @@ fn test_fragment_nested_block() {

assert_eq!(
nested_block.render().unwrap(),
"\n\n<p>I should be here.</p>\n"
"\n<p>I should be here.</p>\n"
);
}

Expand All @@ -61,7 +61,7 @@ fn test_fragment_nested_super() {

assert_eq!(
nested_sup.render().unwrap(),
"\n\n<p>Hello world!</p>\n\n[\n<p>Parent body content</p>\n]\n\n"
"\n<p>Hello world!</p>\n\n[\n<p>Parent body content</p>\n]\n\n"
);
}

Expand All @@ -79,5 +79,27 @@ fn test_fragment_unused_expression() {
required: "Required",
};

assert_eq!(unused_expr.render().unwrap(), "\n\n<p>Required</p>\n");
assert_eq!(unused_expr.render().unwrap(), "\n<p>Required</p>\n");
}

#[derive(Template)]
#[template(path = "blocks.txt", block = "index")]
struct RenderInPlace<'a> {
s1: Section<'a>,
}

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

#[test]
fn test_specific_block() {
let s1 = Section {
values: &["a", "b", "c"],
};
assert_eq!(s1.render().unwrap(), "[abc]");
let t = RenderInPlace { s1 };
assert_eq!(t.render().unwrap(), "\nSection: [abc]\n");
}
7 changes: 1 addition & 6 deletions testing/tests/inheritance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,14 +329,9 @@ fn test_flat_deep() {

#[derive(Template)]
#[template(path = "let-base.html")]
#[allow(dead_code)]
djc marked this conversation as resolved.
Show resolved Hide resolved
struct LetBase {}

#[test]
fn test_let_base() {
let t = LetBase {};
assert_eq!(t.render().unwrap(), "");
}

#[derive(Template)]
#[template(path = "let-child.html")]
struct LetChild {}
Expand Down
1 change: 1 addition & 0 deletions testing/tests/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ struct FunctionTemplate;

impl FunctionTemplate {
#[allow(clippy::trivially_copy_pass_by_ref)]
#[allow(dead_code)]
fn world3(&self, s: &str, v: u8) -> String {
format!("world{s}{v}")
}
Expand Down