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

Names of take_until and take_till don't distinguish them #487

Open
epage opened this issue Feb 28, 2024 · 9 comments
Open

Names of take_until and take_till don't distinguish them #487

epage opened this issue Feb 28, 2024 · 9 comments
Labels
A-combinator Area: combinators C-bug Category: Things not working as expected

Comments

@epage
Copy link
Collaborator

epage commented Feb 28, 2024

From #485

One example is take_till vs take_until. Semantically that means the same thing, and their descriptions on the doc's list of combinators, seem to say the same thing just with different words.

See also #95

@epage epage added A-combinator Area: combinators C-bug Category: Things not working as expected labels Feb 28, 2024
@epage
Copy link
Collaborator Author

epage commented Feb 28, 2024

See also rust-bakery/nom#968

@yyy33
Copy link

yyy33 commented Aug 1, 2024

Why take_till and take_untill are not the same as repeat_till, which receives a Parser as a termination condition, I think take_till and take_untill could be the same way

@epage
Copy link
Collaborator Author

epage commented Aug 1, 2024

repeat_till().take() can fill that same role while these functions fill a more specialized, optimized role.

@yyy33
Copy link

yyy33 commented Aug 7, 2024

repeat_till().take() can fill that same role while these functions fill a more specialized, optimized role.

repeat_till(any, terminator).take(),Am I understanding you correctly?

@epage
Copy link
Collaborator Author

epage commented Aug 7, 2024

Yes

@yyy33
Copy link

yyy33 commented Aug 7, 2024

I've been thinking about your reply for a long time and have been reading the winnow code lately. And I've been comparing winnow's api to regular expressions. I found repeat_till to be the functional equivalent of a superset, t similar to the repeat syntax in regular expressions. And take_while,take_till and so on is a subset of it, some operations repeat_till can do, but it might be faster to use take,take_while? So I'm wondering if it's necessary to just keep repeat_till, and then use specializations for take,take_while and such?

@epage
Copy link
Collaborator Author

epage commented Aug 7, 2024

If you know of a way to to get the performance benefits of take_while with repeat and take_until / take_till with repeat_till,. then I would be open to considering it. For myself, I don't see a good way of doing that. I also suspect users might prefer specialized methods for easier discovery, better examples for specializing on their own, and compile-time guarantees that they are getting better performance.

@yyy33
Copy link

yyy33 commented Aug 8, 2024

What I'm wondering is why winnow::combinator::Repeat parses to an Accumulate instead of <I as Stream>::Slice, and if winnow::combinator::Repeat returns a Slice, wouldn't that be as fast as take_while? while. Define a collect method for winnow::combinator::Repeat that returns Accumulate

@epage
Copy link
Collaborator Author

epage commented Aug 8, 2024

That depends on how expensive your error type is and how much the optimizer can do as looping over parsers has more overhead.

That also leaves out one of the take_*s as it applies simd optimizations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-combinator Area: combinators C-bug Category: Things not working as expected
Projects
None yet
Development

No branches or pull requests

2 participants