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

take and into_inner_unchecked functions #187

Merged
merged 4 commits into from
May 31, 2021

Conversation

conradludgate
Copy link
Contributor

@conradludgate conradludgate commented May 25, 2021

Adds two new functions

  1. into_inner_unchecked, unsafe and unchecked variant of into_inner
  2. take, which returns the ArrayVec while only borrowing, replacing the original with a new empty ArrayVec

@conradludgate
Copy link
Contributor Author

I'm impartial to the take naming, so if you know any better names, I'll gladly update

@bluss
Copy link
Owner

bluss commented May 26, 2021

IMO .take() should be reserved for something that takes self, i.e ".take() -> ArrayVec" just like how Option::take works. That way, it can be take_array here instead.

Would it make sense to write ".take() -> ArrayVec" here and let the user compose it themselves with .into_inner()?

@conradludgate
Copy link
Contributor Author

Would it make sense to write ".take() -> ArrayVec" here and let the user compose it themselves with .into_inner()?

Not a terrible idea. I'll implement that

@bluss
Copy link
Owner

bluss commented May 29, 2021

Thanks, this makes sense to include once docs are written and PR description is updated. 🙂

@conradludgate
Copy link
Contributor Author

I'll update the PR with docs and tests later today then :)

@conradludgate conradludgate changed the title take, take_unchecked, into_inner_unchecked take and into_inner_unchecked functions May 29, 2021
src/arrayvec.rs Show resolved Hide resolved
@bluss bluss merged commit 515a18c into bluss:master May 31, 2021
@bluss
Copy link
Owner

bluss commented May 31, 2021

Thanks!

@bluss bluss added this to the 0.7.1 milestone May 31, 2021
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.

2 participants