Add support for generics syntax in parser #317
Conversation
Instead of making it part of the identifier. Otherwise `normalize_identifier` won't work.
rinja_parser/src/expr.rs
Outdated
| fn parse(i: &mut &'i str, level: Level<'_>) -> ParseResult<'i, WithSpan<'i, Self>> { | ||
| let start = *i; | ||
| ( | ||
| ws(identifier_with_refs), |
There was a problem hiding this comment.
This allows e.g. &&u32, but would fail for &&core::primitive::u32. I think we need to support paths in here. What do you think.
There was a problem hiding this comment.
Didn't even think about it but I agree.
|
I made minor changes to keep track of the nesting level, so |
|
Funnily enough, I was thinking about this case the other day but it was late and I forgot afterwards. ^^' |
|
Anyway, gonna allow to have paths as generics tomorrow. |
|
Oops, I didn't see that you answered immediately. I'll give the PR back to you, now. :) |
rinja_derive/src/generator/expr.rs
Outdated
| node: Span<'_>, | ||
| ) -> Result<DisplayWrap, CompileError> { | ||
| ensure_filter_has_no_generics(ctx, name, generics, node)?; |
There was a problem hiding this comment.
I'm not a big fan because this call can (and will be) forgotten when we add new filters. I voluntarily moved this check outside of the function to special-case filters which actually were making use of generics so by default, all the others would error.
There was a problem hiding this comment.
Actually I don't think that the line get be forgotten. Without the line, the argument generics is unused, and rust will print a warning.
For functions like visit_filter() I like to use a clear dispatch pattern. The brains should be in the dispatch target functions, the dispatcher should be as stupid and as simple as possible. And because an unused argument would render a warning, I don't see any drawbacks in here.
But if you dislike this implementation very much, then I could live is an implementation with more logic inside the dispatcher, too.
There was a problem hiding this comment.
It's a valid argument although I think centralizing checks when possible is a better approach. And also, sometimes you can just prepend with _ to do your things and forget about it later on (happened to me 😅).
There was a problem hiding this comment.
Yeay. :D
But another day, tonight it's video game time. :)
Oh no, less work for me. 😛 Just waiting for you to tell me what you think about my logic and if that sounds ok to you, I'll change the code and move the check in the "common filter path" instead. |
|
Sorry, it was meant for a fix I wrote on docs.rs. ^^' |
|
Applied the changes we talked about. I think we're good to go for this part? |
Extracted the parser generics support add from #311.