-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add support for rest pattern #104
Conversation
cb7ac70
to
3d22745
Compare
3d22745
to
c860a22
Compare
Rebased and fixed clippy. CI is finally happy. :) |
rinja_parser/src/target.rs
Outdated
if let Target::Rest(s) = target { | ||
if !allow_named_rest && s.inner.is_some() { | ||
return Err(nom::Err::Failure(ErrorContext::new( | ||
format!("`@ ..` cannot be used in {type_kind}"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rust uses "@ ..
is only allowed in slice patterns". I like that better, because it tells you what you can do, instead of what you can't do.
The code looks fine, so if you like the current phrasing better, then it's not a showstopper for me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No you're right, I prefer your version. Updating!
And done. :) |
rinja_parser/src/target.rs
Outdated
if let Target::Rest(s) = target { | ||
if !allow_named_rest && s.inner.is_some() { | ||
return Err(nom::Err::Failure(ErrorContext::new( | ||
format!("`@ ..` is only allowed in slice patterns"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format!()
is not needed anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even clippy was complaining about it. :3
674c7ff
to
4de78ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I was afraid of a far bigger diff.
As discussed in #101, I added support for it.