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

[issues-1223] Add take_from combinator #1566

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ikrivosheev
Copy link

Close: #1223

@Xiretza
Copy link
Contributor

Xiretza commented Oct 28, 2022

I think this might be better implemented as a combinator void(p) with the semantics of value((), p) (which would be used like void(take(n))), rather than a specialized combinator for skipping a fixed number of elements.

@ikrivosheev
Copy link
Author

I think this might be better implemented as a combinator void(p) with the semantics of value((), p) (which would be used like void(take(n))), rather than a specialized combinator for skipping a fixed number of elements.

take(n) - also create new slice. The idea was not to create unnecessary slice and immediately skip a piece

@Xiretza
Copy link
Contributor

Xiretza commented Oct 28, 2022

Apart from inlining probably removing it outright, "creating a slice" is an incredibly cheap operation - it's really just a single addition. I don't think there's much to optimize here; if you have benchmarks suggesting otherwise, those would of course be valuable to see.

@ikrivosheev
Copy link
Author

Not everyone works only with slices, some one can work with Read + Write or something else. And it looks very strange that in order to skip a piece of data, you need to call take. It's logical to have some way to skip (real skip, not take then skip) piece of data.

@epage
Copy link
Contributor

epage commented Jan 5, 2023

Not everyone works only with slices, some one can work with Read + Write or something else. And it looks very strange that in order to skip a piece of data, you need to call take. It's logical to have some way to skip (real skip, not take then skip) piece of data.

To clarify, are you working with Read + Write and needing ti for that?

I'm also concerned that this would need a proliferation of different combinators to fully flesh out the idea of "needing" skip support (e.g. skip_while, skip_until) which would bloat nom. I feel like more concrete justification would be needed to justify that.

@ikrivosheev ikrivosheev changed the title [issues-1223] Add InputSkip trait [issues-1223] Add take_from combinator Jan 26, 2023
@ikrivosheev
Copy link
Author

@Geal hello! I have rebased PR, can you review?

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.

is there a "skip" or "drop", like "take"?
3 participants