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.

Sign up for GitHub

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

Jump to bottom

Support &str, char, etc as parsers when using Streaming #79

Closed
2 tasks done
epage opened this issue Jan 26, 2023 · 2 comments · Fixed by #171
Closed
2 tasks done

Support &str, char, etc as parsers when using Streaming #79

epage opened this issue Jan 26, 2023 · 2 comments · Fixed by #171
Labels
A-input Area: input / parsing state C-question Uncertainty is involved M-breaking-change Meta: Implementing or merging this will introduce a breaking change.

Comments

@epage
Copy link
Collaborator

epage commented Jan 26, 2023

Please complete the following tasks

winnow version

0.2.0

Describe your use case

In #78, when porting the json example to streaming, I had to update the code with one_of / tag. I also did it wrong and used one_of when I should have used tag which took some debugging to resolve.

The problem is that whether we are streaming or not is a const STREAMING: bool generic parameter and char can't constrain it.

Describe the solution you'd like

We could switch back to the original proposal in rust-bakery/nom#1535 of calling a function on InputIsStreaming, rather than taking a type parameter

Alternatives, if applicable

Consider streaming a less common case that doesn't get all of the ease of use features

Additional Context

I had originally switch to the const STREAMING: bool parameter to move error reporting to compile time, rather than runtime. For example, the old finish function would panic if Incomplete is returned. Panicing on is_streaming() == true is better but having the compiler tell you this information is even better.
@epage epage added C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change. C-question Uncertainty is involved A-input Area: input / parsing state and removed C-enhancement Category: Raise on the bar on expectations labels Jan 26, 2023
@epage
Copy link
Collaborator Author

epage commented Jan 27, 2023

See also rust-bakery/nom#1614

@epage
Copy link
Collaborator Author

epage commented Feb 15, 2023 •
edited
Loading

#166 allows runtime selection of "partial" parsing state. We could remove the cosnt PARTIAL: bool parameter. It isn't providing as much value as I originally expected. I wanted things to be type safe but only really FInishIResult leverages it.

Or we could use associated types #163

epage added a commit to epage/winnow that referenced this issue Feb 16, 2023
@epage
fix(parser)!: Allow literals to be parsers of partial inputs
7878863 
If you used `Partial`, you couldn't use `"hello"` as a tag but had to do
`tag("hello")` because we needed to constrain the generic parameter.
Now that we've removed it, literals can be used as desired.

The downside is that for `FinishIResult`, this shifts an error from
compile time to runtime.  At least with `is_partial_support`, the user
is more likely to see it than they would have with the original `nom`
code.

BREAKING CHANGE: `StreamIsPartial` no longer takes a `const PARTIAL:
bool` generic parameter.  This was replaced with `is_partial_support`
function.

Fixes winnow-rs#79
@epage epage mentioned this issue Feb 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-input Area: input / parsing state C-question Uncertainty is involved M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(parser)!: Allow literals to be parsers of partial inputs epage/winnow
1 participant
@epage