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

Add a subslice function for Bytes (#198) #208

Merged
merged 2 commits into from Sep 2, 2018

Conversation

federicomenaquintero
Copy link
Contributor

No description provided.

@carllerche
Copy link
Member

Sorry for letting this hang. I will try to up prioritize it.

The implementation is good. The name is bugging me. I would like it to match the pattern of the others. So, I would like a name like:

slice_???

Except, I can't think of anything good :(

Best I could think of is slice_subset... but I Don't like that either. Thoughts?

@carllerche
Copy link
Member

carllerche commented Aug 31, 2018

Or, what about slice_for?

let sub_bytes = bytes.slice_for(&sub);

@carllerche
Copy link
Member

carllerche commented Aug 31, 2018

Another (via Ralith in gitter): slice_ref

let sub_bytes = bytes.slice_ref(&sub);

@carllerche
Copy link
Member

I'm like slice_ref as it follows the slice_ pattern and implies that you are slicing using a reference to self.

bytes.slice_ref(self.as_ref()) == bytes

@federicomenaquintero
Copy link
Contributor Author

Cool, I'll rebase this and use slice_ref.

@federicomenaquintero
Copy link
Contributor Author

I've renamed the function to slice_ref() and the sub argument to subset. Also changed the test names to make it more clear what they are doing.

@carllerche
Copy link
Member

Thanks for sticking with it 👍

@carllerche carllerche changed the base branch from master to v0.4.x August 31, 2018 16:09
@carllerche carllerche changed the base branch from v0.4.x to master August 31, 2018 16:10
@carllerche
Copy link
Member

I'm going to rebase the PR against the 0.4.x branch.

This lets us take Bytes and a &[u8] slice that is contained in it, and
create a new Bytes that corresponds to that subset slice.

tokio-rs#198
@carllerche carllerche changed the base branch from master to v0.4.x August 31, 2018 16:12
@carllerche
Copy link
Member

CI hung for some reason...

@carllerche carllerche merged commit 79f0559 into tokio-rs:v0.4.x Sep 2, 2018
@federicomenaquintero
Copy link
Contributor Author

Yay, thanks!

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.

None yet

2 participants