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

Template node structs & whitespace semantics #829

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

couchand
Copy link
Contributor

@couchand couchand commented Jun 24, 2023

This is the first set of changes extracted from #782.

These changes focus on clarifying the semantics of the whitespace of each Node. This PR refrains from abstracting out the common whitespace-handling patterns that emerge here, leaving that for a follow-on PR.

We make the following changes here:

  • Adding Node variant parsing tests
  • (Mostly) consistently using one struct per Node variant
  • Clarifying the semantics of each and every Ws value (though since the whitespace patterns aren't yet abstracted this clarification is still incomplete)

There should be no visible effects of these changes, they are purely distilling towards greater insight. As described in the comments to #782, the general goal is to differentiate between the "inner" whitespace (within nested blocks) and the "outer" whitespace (around every node except lit).

Outer whitespace is the first field of each non-Lit variant of Node. Inner whitespace is on each nested-block-type node, stored alongside the Vec<Node> value that represents the nested block. For instance, for the template

{%- if test +%}FooBar{%+ endif -%}

Would result in the Rust value

Node::Cond(
    Ws(Some(Whitespace::Suppress), Some(Whitespace::Suppress)),
    vec![
        Cond {
            test: CondTest {
                target: None,
                expr: Expr::Var("test"),
            },
            block: vec![Node::Lit("FooBar")],
            ws: Ws(Some(Whitespace::Preserve), Some(Whitespace::Preserve)),
        }
    ]
)

Thanks for your review, your time and effort are appreciated!

@couchand couchand changed the title Template block whitespace semantics Template node whitespace semantics Jun 24, 2023
@couchand couchand changed the title Template node whitespace semantics Template node structs & whitespace semantics Jun 24, 2023
@@ -665,7 +665,14 @@ impl<'a> Generator<'a> {
Node::Include(ws, path) => {
size_hint += self.handle_include(ctx, buf, ws, path)?;
}
Node::Call(ws, scope, name, ref args) => {
Node::Call(
Copy link
Owner

Choose a reason for hiding this comment

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

If we're going to do this, I think we should just pass the &Call directly to write_call()... Not much point in destructing it here. (Same for other node types.)

@djc
Copy link
Owner

djc commented Jul 31, 2023

#834 extracts a parser crate and improves the parser API a bunch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants