Skip to content
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

Use less specific type in custom combinator example #1662

Conversation

smheidrich
Copy link
Contributor

The custom combinator from the "Wrapper combinators that eat whitespace before and after a parser" example uses the needlessly specific trait bound FnMut(...) -> IResult for its parser parameter type, which limits its applicability as a combinator, because not all parsers (e.g. Map) implement the FnMut(...) -> IResult trait. But conversely, FnMut(...) -> IResult implements the Parser trait, so IMHO using Parser as the trait bound instead would make perfect sense.

This PR makes that change and also removes a pointless import of nom::combinator::value.

The example in question was the only documentation I could find on how to write a combinator of your own (but to be honest I haven't looked very hard), so that's why I think it's important to get it right.

@smheidrich smheidrich requested a review from Geal as a code owner May 8, 2023 00:30
@Geal
Copy link
Collaborator

Geal commented Jun 10, 2023

that part of the doc had not been modified in a while, it's good to update to to use Parser, especially with upcoming changes

@Geal Geal merged commit 71ba24f into rust-bakery:main Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants