-
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
Improve Level API a bit #31
Conversation
rinja_parser/src/lib.rs
Outdated
fn nest<'b, T, F: FnMut(&'b str) -> ParseResult<'b, T>>( | ||
&self, | ||
i: &'b str, | ||
mut callback: F, |
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.
F
can be FnOnce
. Currently that does not matter, but maybe we refactor the parser code later.
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.
Good point!
rinja_parser/src/lib.rs
Outdated
i: &'b str, | ||
mut callback: F, | ||
) -> ParseResult<'b, T> { | ||
let prev_level = self.level.get(); | ||
let (_, level) = self.level.get().nest(i)?; |
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.
You could directly call prev_level.nest(i)
.
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.
Indeed!
let (i, _) = s.nest(j)?; | ||
let result = func(i, s); | ||
s.leave(); | ||
let (i, node) = result?; |
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.
It is much clearer what is going on with your change, and the code does look much more nom
-like. Clear advancement!
892d968
to
3c9e85a
Compare
There is still the original Something for later on. :) |
3c9e85a
to
4b847a2
Compare
Fixed CI. :) |
Part of #30.
It already improves the situation a bit. Not sure if the rest is worth improving or not though.