search

winnow-rs / winnow

Making parsing a breeze
https://docs.rs/winnow
Other
525 stars 40 forks source link

Context for built-in combinators #421

Open judemille opened 8 months ago

judemille commented 8 months ago

Please complete the following tasks

winnow version

0.5.34

Describe your use case

Using Winnow without adding .context(...) calls to every combinator.

Describe the solution you'd like

Context should be added to built-in combinators, so that when they produce an error, the Display impl indicates what went wrong and why, without having to trace the parsing.

I can help with implementation, once details are worked out.

Alternatives, if applicable

Forcing users to add context calls to each use of every combinator, as current, but that is really not ideal.

Additional Context

This may be difficult, due to the complexities of AddContext. I'm not sure how best to implement this.

epage commented 8 months ago

This was an intentional choice. toml_edit originally used combine which would automatically determine what was expected. The problem was that it was slow (because you couldn't opt-out of what we're getting in #415) and the quality was low. I found that hand-specifying the values gives much better results.

This comes at the cost of not getting great errors in more trivial / prototype cases.

The other problem with baking them in is that it would tie us to one specific type context type. Currently, we offer a default choice but allow people to customize it (which the author of #415 seems to do).

judemille commented 8 months ago

Yeah, I'm not sure how to solve that issue. I figure there must be a good way to go about it, but I'm at a loss.