Implement match functionality

[anowell]

This is a first stab at basic match functionality discussed in #46. Several things are incomplete:

• Whitespace handling isn't right. It's been tripping me up a bunch, and I'm not even sure I've really nailed down the "right" behavior of whitespace control.
• [punted] variants with a single parameter currently need to be enclosed in parentheses, e.g. Some with (val)
• [punted] compound match arms aren't supported yet, e.g., when Foo | Bar
• else isn't supported yet (though when _ probably works - unsure if that should be prevented)
• doesn't properly support both owned and ref types. I'm increasingly convinced that waiting for match modes to land is a good idea.
• I'm not sure if it can match variants with literal values currently, e.g., when Some with "foo"
• I used endmatch. Previous discussions just used end. Not sure what's preferred.
• Custom enums don't work (:: can't be in an identifier. needs a path parser)
• more tests (still planning to commit custom enum tests once they pass)
• docs

I'm not sure when I'll get to the remainder - I don't expect I'll be able to jump on this too much more in the coming week or so. I'll try to pick it back up when I can, but I certainly don't oppose anyone stepping in to help land any of this functionality in the meantime.

[anowell]

re: whitespace. Aside from really struggling with the whitespace handling, I'm not super clear on the right behavior. But mostly, I think that any whitespace between the match and first arm should be ignored, otherwise you will end up with the first arm having an extra newline in the common cases.

The examples below should probably be test cases. I've attempted to define the assertions for each of these when rendered with both Some("boat") and None (but not fully convinced I have the right assertions)

A) everything on its own line

item = {% match item %}
{% when Some with val %}
{{val}}
{% when None %}
none
{% endmatch %}

assert: "item =\nboat\n" or "item =\nnone\n"

B) one-line for each arm

item = {% match item %}
{% when Some with val %}{{val}}
{% when None %}none
{% endmatch %}

assert: "item = boat\n" or "item = none\n"

C) compact one-liner

item = {% match item %}{% when Some with val %}{{val}}{% when None %}none{% endmatch %}

assert: "item = boat" or "item = none"

D) strange one-liner

item = {% match item %}
{% when Some with val %}{{val}}{% when None %}none{% endmatch %}

assert: "item = boat" or "item = none" (same as C?)

E) Endmatch trailing without a newline

item = {% match item %}
{% when Some with val %}{{val}}
{% when None %}none{% endmatch %}

assert: "item = boat" or "item = none" (same as C? or should the Some variant have an extra newline?)

F) all arms share one line

item = {% match item %}
{% when Some with val %}{{val}}{% when None %}none
{% endmatch %}

assert: "item = boat\n" or "item = none\n" (same as B?)

G) first arm on the match line

item = {% match item %}{% when Some with val %}
{{val}}
{% when None %}
none
{% endmatch %}

assert: "item = \nboat\n" or "item = \nnone\n" (Same as A?)

[djc]

Looks great! Feedback on your open items:

• Whitespace should be fully explicit. So, preserve all the whitespace you see in the template, then allow the placement of - in all the usual places to suppress it. This makes it predictable. So I don't think we should implicitly suppress whitespace between match and the first when.

• Let's postpone compound arms to a future PR?

• Let's also postpone paren-less single-expression variants? Or at least put it in a separate commit.

• I think we should probably allow when _ as an alternative to else.

• Waiting for match modes will mean that we either need to wait for them to hit stable or we need to add some stuff to deal with the appropriate feature flag.

• Should add a test to check that literal-as-variant works.

• Let's use endmatch.

• Feel free to leave docs to me.

I'd also prefer to have more tests in separate commits, so keep this easy to review.

[djc]

I don't think we need to push the locals here?

[djc]

For readability of the generated code, I'd like to only emit the comma where necessary. I think the easiest way to do that is to enumerate over the loop iterator and emit a prefix comma for index > 0.

[djc]

Please add a trailing newline here.

[anowell]

I neglected to chime in, but I'm 100% in agreement with all your feedback/suggesions. I just need to find a bit more time to push this across the finish line. ;-)

And I still hope to put more thought into what options there are with or without waiting for match modes to land - but I figure I'll worry about that after all the other details mentioned.

[djc]

Sounds good! I'll probably focus on other things in the mean time, on the assumption that binding modes are a prerequisite. Unless you need more help/feedback, of course!

[anowell]

A little bit of progress:

• whitespace is still kicking my butt. I haven't figured out how to get the whitespace between the match and first when line to actually get handled at all (this line never prints anything). And other when arms end up inheritating whitespace from the end of the body of the previous when arm which seems wrong (you can see the extra newline in the test cases). But I want to compare with if/else rendering because I feel like I'm doing roughly the same thing, so I'm actually wondering if regular else conditions also end up starting with any trailing whitespace from the if body.

• else works

• match literals work and have a few tests. There are two places where literals can live:

  1. when Some with ("foo") -> hence parameters can be str_lit, num_lit, or identifier
  2. when "foo" -> hence the variant can be str_lit, num_lit, or name

I could (should?) have used 2 different enum types for those, but since both identifier and name are just &str, and I'm aready shamelessly duplicating parsing code, I shared a MatchParameter type for now.

There is a big caveat I discovered: when my_var might be surprising. It will parse, but my_var will be parsed as a variant name, but the generated code will treat it like a match arm that catches everything else and binds it to my_var, but it won't insert my_var as a local. I don't see a reliable way to disambiguate variable binding (when foo) from variant name (when None) without some new syntax. I really didn't want to support the the variable binding case at all (it's really only useful if the match expression called a function). But it seems too easy to mistakenly do {% when foo %}{{foo}} and then get an error that variable foo doesn't exist in your template.

[djc]

Whitespace: I have to admit it's a bit tricky. Each piece of whitespace (which will be previous whitespace to a particular Node or trailing whitespace to particular node) can be suppressed from both sides if surrounded by blocks or expressions. Literal nodes cannot suppress, so they always flush. So what happens is, preceding whitespace needs to be flushed by a particular block, while trailing whitespace needs to be sent into the cache, so that the next block can decide whether to suppress (discard) or allow (flush) it. If the function is handling a single-node block (an expression, for example), you can just use handle_ws() to do both. But in other cases, you have to make somewhat-judicious use of prepare_ws() and flush_ws().

How does Rust handle the my_var => case? I would guess it has similar behavior; and if the failure modes are similar to real Rust, I wouldn't be too worried. A possible alternative is to key off the quite strong convention that variants start with uppercase to help distinguish between variants and variables.

[djc]

(Of course if you're done with trying to get the whitespace to line up, feel free to just say so and I'll dive in myself.)

[anowell]

I'm not sure how Rust handles my_var =>, but anectodally, they seem to be type checking to see if my_var is an enum variant. See this playground example. I mostly lean on the side of leaving it broken/unsupported, and if it's ever truly needed, you could add syntax like {% else my_var %} or {% else with my_var %} (or parse with case-sensitivity).

I haven't given up on whitespace (yet), but the trial-and-error battling with it has been almost half of my total time working on this PR, so if you see the obvious problem, go ahead and fix it; I get no joy out of mindlessly battling a problem that you could fix in 30s. But I'm hoping that walking the code again sometime with the context you just added might help make something click.

But mostly I'm thinking about how to solve the ref/match-mode stuff.

[anowell]

I stumbled onto a trick that seems to work pretty well for auto-ref/deref. This playground does the best job of showing how it works, using (&expr).deref(). I do have to prefix all the enum and number arms with &, because supporting string literals implies a reference type. And unfortunately it can only auto-deref one level, so you can have Option<T> or &Option<T> in your Template, but not &&Option<T>. I'm doubtful we'll find much better until the match mode stuff lands.

Here's a break down of what the rendered code ends up looking like:

Option<&str> and &Option<&str> render

    fn render_into(&self, writer: &mut ::std::fmt::Write) -> ::askama::Result<()> {
        #[allow(unused_imports)] use ::std::ops::Deref as HiddenDerefTrait;
        match (&self.item).deref() {
            &Some("foo") => {
                writer.write_str("\n")?;
                writer.write_str("Found literal foo")?;
            }
            &Some(ref val) => {
                writer.write_str("\n")?;
                writer.write_str("\n")?;
                writer.write_str("Found")?;
                writer.write_str(" ")?;
                let askama_expr = &val;
                writer.write_fmt(format_args!("{}", &::askama::MarkupDisplay::from(askama_expr)))?;
            }
            &None => {
                writer.write_str("\n")?;
                writer.write_str("\n")?;
                writer.write_str("Not Found")?;
            }
        }
        writer.write_str("\n")?;
        Ok(())
    }

&str literal renders

    fn render_into(&self, writer: &mut ::std::fmt::Write) -> ::askama::Result<()> {
        #[allow(unused_imports)] use ::std::ops::Deref as HiddenDerefTrait;
        match (&self.item).deref() {
            "foo" => {
                writer.write_str("\n")?;
                writer.write_str("Found literal foo")?;
            }
            "bar" => {
                writer.write_str("\n")?;
                writer.write_str("\n")?;
                writer.write_str("Found literal bar")?;
            }
            _ => {
                writer.write_str("\n")?;
                writer.write_str("\n")?;
                writer.write_str("Else found")?;
                writer.write_str(" ")?;
                let askama_expr = &self.item;
                writer.write_fmt(format_args!("{}", &::askama::MarkupDisplay::from(askama_expr)))?;
            }
        }
        writer.write_str("\n")?;
        Ok(())
    }

u32 literal renders

    fn render_into(&self, writer: &mut ::std::fmt::Write) -> ::askama::Result<()> {
        #[allow(unused_imports)] use ::std::ops::Deref as HiddenDerefTrait;
        match (&self.item).deref() {
            &42 => {
                writer.write_str("\n")?;
                writer.write_str("Found answer to everything")?;
            }
            _ => {
                writer.write_str("\n")?;
                writer.write_str("\n")?;
                writer.write_str("Else found")?;
                writer.write_str(" ")?;
                let askama_expr = &self.item;
                writer.write_fmt(format_args!("{}", &::askama::MarkupDisplay::from(askama_expr)))?;
            }
        }
        writer.write_str("\n")?;
        Ok(())
    }

---

Also, while adding a few more tests, I discovered that it doesn't currently support custom enums because when Color::Rgb contains :: which isn't valid in an identifier, so I'll look into a helper to parse paths, and then probably split out a MatchVariant type separate from MatchParameter, since the refs inside the enum should remain identifiers. It's added to the checklist in the first comment.

[djc]

Wow, that's good news! If you rebase your branch onto master, you get d497a31, which should help.

[djc]

@anowell ping?

[anowell]

I've had the custom enum stuff mostly implemented for weeks, just wasn't ready to take on the whitespace issues. Anyhow, I just rebased everything I had. I feel like I created some redundant code for parsing the MatchVariant and MatchParameter - certainly open to better ideas - but it should all be functional.

I'm a bit pressed on a mid-Nov deadline, so it'll probably be a couple weeks before I can dig into the whitespace issue or anything else that comes up on this, so feel free to resolve that yourself if you want to see it land sooner. ;-)

[djc]

I fixed the whitespace issue by allowing the parser to see the whitespace between match and the first when, and reflecting that whitespace in the generated code (pending the usual whitespace suppression mechanics). I squashed your commits a little bit, including my whitespace fix, and pushed them to master as 14beb21..2257afd. Thanks for working on this, very cool stuff!

I'll write up some documentation before I release a new version.