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

impl FromIterator<char> for ArrayString #142

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

Conversation

oconnor663
Copy link

String includes a number of other impls for e.g. &char and &str, and we
could expand the impls here in a follow-up commit if we want.

String includes a number of other impls for e.g. &char and &str, and we
could expand the impls here in a follow-up commit if we want.
@oconnor663
Copy link
Author

This enables you to .collect() an ArrayString from any iterator of chars. This is a relevant bound for e.g. the hex::ToHex trait.

@clarfonthey
Copy link
Contributor

This is a duplicate of #126, which was ultimately not merged.

@oconnor663
Copy link
Author

Oh thanks for the link. One thing this highlights for me is that I misunderstood how ArrayVec::collect works. I had assumed it panics if it doesn't have enough capacity, but actually it ignores the rest of the iterator, as in #126.

My main reason for preferring to panic was that it fit my use case :) Namely, I know exactly how long a certain string is going to be, and I've allocated exactly enough space to contain it. If it turned out I didn't have enough space, that would be a bug in my code, and I would want to see a panic telling me to fix it.

Do you find the "ignore the rest of the iterator" use case common? If so, then I don't have any better ideas than what you discussed in #126, and I agree that not implementing this at all might be better than implementing something surprising. But is there any chance a future version of ArrayVec might prefer to switch to panicking behavior for extend and collect?

@clarfonthey
Copy link
Contributor

I don't think there's going to be a good sense of what's best until we have a clearer idea of what falliable Extend and FromIterator look like in std. So, for now, this is the behaviour that made the most sense.

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