-
-
Notifications
You must be signed in to change notification settings - Fork 3
build: upgrade winnow to 0.7 #26
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
Conversation
Signed-off-by: tison <wander4096@gmail.com>
| F: ModalParser<I, O, E>, | ||
| G: FnMut(O) -> Result<O2, E2>, | ||
| I: Stream, | ||
| E: FromExternalError<I, E2>, | ||
| { | ||
| #[inline] | ||
| fn parse_next(&mut self, input: &mut I) -> PResult<O2, E> { | ||
| fn parse_next(&mut self, input: &mut I) -> ModalResult<O2, E> { | ||
| let start = input.checkpoint(); | ||
| let o = self.parser.parse_next(input)?; | ||
|
|
||
| (self.map)(o).map_err(|err| { | ||
| input.reset(&start); | ||
| ErrMode::from_external_error(input, ErrorKind::Verify, err).cut() | ||
| ErrMode::from_external_error(input, err).cut() |
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.
@epage FYI how this crate upgrades to winnow 0.7 and retain the try_map_cut combinator.
Not sure if we can achieve the same behavior without Cut. From the changelog, it seems cut is secondary to use?
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.
Making ErrMode optional was about paying for what you use. toml_edit still uses ModalResult and cut_err.
To not use cut_err, you need to find ways to restructure a parser to reduce the use of opt/alt to where you don't need it (e.g. using dispatch! instead).
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.
Thanks for your information! Then I'd merge this patch as is.
I'd investigate if dispatch! can be used here 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.
winnow-rs/winnow#692 is an example of that. I only did that so I wouldn't force ErrMode on users.
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.
Thanks for the reference.
After a closer look it seems cronexpr wants to terminate immediately if a part match is success but the value is an unexpected one (mainly for better error reporting), thus try_map_cut. No backtrace is expected here.
So I'd continue the current approach unless I find a way to express the semantic better.
No description provided.