-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Implement additional split_inclusive variants for slices
#91546
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
Implement additional split_inclusive variants for slices
#91546
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @scottmcm (or someone else) soon. Please see the contribution instructions for more information. |
This comment has been minimized.
This comment has been minimized.
75008e2 to
a457a17
Compare
This comment has been minimized.
This comment has been minimized.
a457a17 to
74acc2c
Compare
This comment has been minimized.
This comment has been minimized.
74acc2c to
8c64adf
Compare
rsplit_inclusive for slicessplit_inclusive variants for slices
0d3a25c to
f28ed5c
Compare
This comment has been minimized.
This comment has been minimized.
f28ed5c to
2b461cd
Compare
2b461cd to
69870da
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@scottmcm This is complete and ready for review! |
96411c9 to
e107705
Compare
|
☔ The latest upstream changes (presumably #92556) made this pull request unmergeable. Please resolve the merge conflicts. |
e107705 to
30cb49e
Compare
|
(Sorry for not responding over winter break) Hmm, have you had any libs team feedback on all these? This is a great many new iterator types, for example, as you mentioned. If not, maybe start a zulip thread to collect some thoughts? |
|
@scottmcm The final form of this PR is a lot more extensive than I initially intended; I set out to add a single "obviously" missing iterator, but then realized there were many other "obvious" additions to be made. I'll make a thread on Zulip |
0c842c6 to
663ae21
Compare
|
☔ The latest upstream changes (presumably #92598) made this pull request unmergeable. Please resolve the merge conflicts. |
663ae21 to
ebd85b7
Compare
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #95274) made this pull request unmergeable. Please resolve the merge conflicts. |
|
ping from triage: FYI: when a PR is ready for review, send a message containing |
|
This PR isn't ready as is, I need to integrate some libs team comments. |
|
@Jules-Bertholet any updates on this |
|
@Dylan-DPC Not at present. I'm planning to make an API change proposal before continuing to work on the implementation, that should be ready soon |
afd1844 to
8a321c8
Compare
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
This comment has been minimized.
This comment has been minimized.
…for string slices
8a321c8 to
bfc9df0
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☔ The latest upstream changes (presumably #99223) made this pull request unmergeable. Please resolve the merge conflicts. |
|
I've implemented an alternate API using combinators. It's available in the |
|
Closing this as it is a draft and is inactive for some time. Feel free to reöpen this or create a new pr if you want to continue with this. Thanks |
Implements additional variants of the
splititerator methods for strings and slices.Methods implemented:
slice::{r}split{n}{left}_inclusive{_mut}(),str::{r}split{n}{left}_inclusive(), andstr::split_once{_left}_inclusive: Theinclusivevariants include the separator at the end of the returned elements, while theleft_inclusivevariants include it at the beginning.str::{r}split_initiator, andstr::{r}split_ends: Variants ofsplit_terminatorthat skip empty subslices at the beginning of the string, and at both ends of the string respectively.In addition, this PR implements
Clonewhen appropriate for thesplitniterators, and changes the return value ofsize_hintin a few cases.Unresolved questions
This adds a lot of new methods because of all the possible permutations. The implementations are unified via macros, but it's a lot of choices to sort through in the documentation. Maybe some variants should be omitted, or a more generic interface should be provided?
One possibility to limit the combinatorial explosion, which I'm currently implementing, is to not have the additional "
n" variant methods be defined directly onstr/slice, but on their base iterator.Interacts with #77998 (I haven't yet implemented
as_strfor all the new iterators)