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

map with both Output and Stream #231

Open
2 tasks done
martinohmann opened this issue Apr 14, 2023 · 7 comments
Open
2 tasks done

map with both Output and Stream #231

martinohmann opened this issue Apr 14, 2023 · 7 comments
Labels
A-combinator Area: combinators C-enhancement Category: Raise on the bar on expectations

Comments

@martinohmann
Copy link
Contributor

Please complete the following tasks

winnow version

v0.4.1

Describe your use case

See the discussion thread #230 (comment)

The general idea is to get access to the state of a winnow::stream::Stateful input in a map-like function so that it can be updated while mapping a parser's output.

Right now, only simple use cases are supported, like the one shown in the Stateful type docs.

Describe the solution you'd like

A method on the Parser trait that allows to modify the state in a map-like operation.

Ideally something like map_with_state that would both provide the output of the previous parser, and a reference to the state inside the Stateful so that it could be updated.

There was another idea in #230 (comment) to generalize this to not only support the Stateful input type, but any input type by having something like map_with_input to expose a reference to the whole input, not just the state in a Stateful.

Alternatives, if applicable

No response

Additional Context

No response

@martinohmann martinohmann added the C-enhancement Category: Raise on the bar on expectations label Apr 14, 2023
@martinohmann
Copy link
Contributor Author

@epage feel free to rename the issue to something better, I was struggling to find a good title.

@epage epage changed the title Mechanisms to manipulate parser state in a map-like fashion map with both Output and Stream Apr 15, 2023
@epage epage added the A-combinator Area: combinators label Apr 15, 2023
@martinohmann
Copy link
Contributor Author

I was thinking about it a bit more and checked the existing API of the Parser trait. We could design this in a way that better integrates with what already exist. Taking recognize/with_recognized and span/with_span as examples, we could:

  • add a stream method which yields the a Stream containing the input that produced the output of the previous parser
  • add with_stream, which yields a tuple of parser output and Stream

Then you could simply access this via a map call. This might be enough to both support the Stateful use case and the use case mentioned in #230 (comment).

I can try to compile a draft PR for this approach in the next few days and play around with it a bit to see if this would actually work in the way I envision.

@martinohmann
Copy link
Contributor Author

I'm also not too nailed down on the naming yet. Maybe output_stream/with_output_stream would be a better fit to make it more obvious what's contained in the returned Stream.

martinohmann added a commit to martinohmann/winnow that referenced this issue Apr 17, 2023
@martinohmann
Copy link
Contributor Author

Another name for this could be recognize_stream/with_recognized_stream since its behaviour is identical to recognize with the difference that it returns a Stream instead of Stream::Slice.

@epage
Copy link
Collaborator

epage commented Dec 18, 2023

chumsky consolidated a of their various map calls by having a type that carries all the information, see zesterer/chumsky#537

I don't think we should do that for the common cases but it provides a general form for complex cases.

The main difference with the proposal here is we'd want to also include Checkpoint from before the start of the parse operation. Naming will be difficult. Likely some minor API considerations that'd need to be taken into account.

@epage epage mentioned this issue Jun 4, 2024
2 tasks
@epage
Copy link
Collaborator

epage commented Aug 30, 2024

I wonder if we could use "tacit trait specialization", like axum and bevy, to allow a broad selection of function signatures to all work with one combinator.

https://willcrichton.net/notes/defeating-coherence-rust/

@martinohmann
Copy link
Contributor Author

Do you envision something like map that could work as map(|output| ...) and map(|(input, output)| ...) using this TTS pattern?

I'm a little bit torn about this because on one hand it improves flexibility, but on the other hand it'll make it harder to reason about what certain APIs expect from the user.

I'm always a bit scared of APIs that involve a lot of generic type parameters -- especially when these are imposed by one or more traits -- because it sometimes makes it really hard to understand which input types satisfy the contract and which don't.

I'd probably take the stance of having more methods that are less generic but serve a distinct purpose rather than cramming too much into a single API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-combinator Area: combinators C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants