[nextest-filtering] Replace nom parser with winnow
#1214
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1214 +/- ##
==========================================
+ Coverage 77.40% 77.49% +0.09%
==========================================
Files 72 72
Lines 17859 17884 +25
==========================================
+ Hits 13823 13859 +36
+ Misses 4036 4025 -11 ☔ View full report in Codecov by Sentry. |
The explicit `complete` version is deprecated, so switching straight to `impl Parser for &str`, a shortcut for the auto-`complete` version.
`char` was consolidated into `one_of` but this jumped straight to the `impl Parser for char` which is easier to read.
`pair` is deprecated in favor of `impl Parser for (...)`
In winnow 0.5, we can replace this with `tag` and speed it up
Free-standing `recognize` is deprecated in favor of trait methods but it wasn't even needed here.
This added back in a `tag` that a later release (0.4?) can remove due to better type inference.
This added a lot of `tag` / `one_of`s that can be removed with a later release (0.4?)
This was possible due to improvements in type inference (when you have a generics problem, throw more `PhantomData` at it...).
In 0.5, `parse_next` modifies input so the old behavior is under the
name `parse_peek` (as it doesn't commit to the parse but returns what it
would commit).
So `unpeek` takes a combinator that doesn't modify input ("peek parses")
and makes it commit to the result modifying the input.
This is added only to ease the transition. Future commits will work to
remove `unpeek` calls.
Unsure why it was originally needed. One difference with `nom` is that we return `Stream::Slice` from `token` parsers, rather than `Stream`, simplifying the more common cases.
Because parsers now take `&mut I` and all cloning is gone, we don't need `RefCell` anymore.
|
Wow, thanks! Will look very soon. (you want to run cargo xfmt to fix the lint issue) |
|
This is awesome! Thank you. I'll go ahead and land this. |
|
As you use it, I welcome any feedback to help make it better! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@sunshowers expressed interest in
winnowand I figured porting an existing project they were familiar with would make it easier when they try it out.When i first switched from
noms pureFn(I) -> (I, O)parser model toFn(&mut I) -> O, I was concerned about it making things worse. It does have the downside of requiring more lifetime annotations to disambiguate things but I feel like it has been proven time and again to simplify code. In this case, it removed a lot of bookkeeping from the functions, making the intentional bookkeeping more obvious, and it allowed the removal ofRefCellfor error recovery.I broke this up into a lot of commits
I did put a little too much in the
unpeek-ectomy commit towards the end. The main downside is that we can'tgit bisectto one specific bookkeeping change.I did this as 1 PR though because the decision to commit to this is all-or-nothing.