Unify handling of calls #614
Conversation
Instead of having `Expr::VarCall`, `Expr::PathCall` and `Expr::MethodCall`, this PR unifies the handling of calls by removing the former three variants, and introducing `Expr::Call`. This makes parsing postfix operators easier.
askama_shared/src/generator.rs
Outdated
| @@ -1329,16 +1319,27 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { | |||
| _ => return Err("loop.cycle(…) expects exactly one argument".into()), | |||
| }, | |||
| s => return Err(format!("unknown loop method: {:?}", s).into()), | |||
| }, | |||
| left => { | |||
| match *left { | |||
There was a problem hiding this comment.
This seems like something that could easily be forgotten, to be updated, when changing the parser. Could it maybe be "simplified" or handled in the parser, by having some enum CallKind, which is a third value of Expr::Call()? :)
There was a problem hiding this comment.
Please see my comment in #590: https://github.com/djc/askama/pull/590#discussion_r779486870. I think the list of special cases is exhaustive. I think not even the Expr::Unary(..) | Expr::BinOp(..) | Expr::Range(..) case is actually need, because the parser won't ever generate an Expr::Call(Expr::Unary(..), ..), only Call(Group(Unary(..)), ..).
askama_shared/src/parser.rs
Outdated
| } | ||
|
|
||
| Ok((i, res)) | ||
| fn expr_attr<'a>(i: &'a str) -> IResult<&'a str, Box<dyn 'a + FnOnce(Expr<'a>) -> Expr<'a>>> { |
There was a problem hiding this comment.
Why do we need this Box<dyn 'a + FnOnce(Expr<'a>) -> Expr<'a>> contraption?
There was a problem hiding this comment.
We don't. I guess I wanted to make it hyper-extensible for no proper reason. :D
Instead of having
Expr::VarCall,Expr::PathCallandExpr::MethodCall, this PR unifies the handling of calls by removingthe former three variants, and introducing
Expr::Call.This makes parsing postfix operators easier.
Extracted from #590.