Skip to content

Conversation

@blasrodri
Copy link
Contributor

Allows extending a Vec with a Stream<Item=T>.

Ref: #2842

Motivation

Solution

Allows extending a Vec<T> with a Stream<Item=T>.

Ref: tokio-rs#2842
@blasrodri blasrodri force-pushed the stream-add-extend-function branch from 2c50bb6 to 36157b3 Compare October 20, 2020 11:54
@Darksonn Darksonn added A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-stream Module: tokio/stream labels Oct 20, 2020
@MikailBag
Copy link
Contributor

Shouldn't it be generic over C: std::iter::Extend (and then using coll.extend(std::iter::once(...))?
That way HashSet, String and other types will also be supported.

@blasrodri
Copy link
Contributor Author

Shouldn't it be generic over C: std::iter::Extend (and then using coll.extend(std::iter::once(...))?
That way HashSet, String and other types will also be supported.

It makes sense. However, in order to call .await() I need extend_one() to be stable. Or do you have another idea?

@MikailBag
Copy link
Contributor

coll.extend_one(x) can be replaced with coll.extend(std::iter::once(x)) IIUC

Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't decided if we want this yet, but I noticed that it could be simplified.

@Darksonn
Copy link
Contributor

It's unfortunate that we are not able to reserve space before-hand using the Stream::size_hint method when using the Extend trait.

@blasrodri
Copy link
Contributor Author

@Darksonn shall we close this PR?

@Darksonn Darksonn changed the title stream: add stream utility function stream: add stream extend function Dec 5, 2020
@Darksonn
Copy link
Contributor

Darksonn commented Dec 5, 2020

I've decided to close this because I feel that this is not the best possible API for this function. I want an API that:

  1. Is not restricted to Vec.
  2. Is able to use the size_hint of the stream.

This PR only satisfies the first. You should feel free to open an issue proposing a different API if you have an implementation idea that satisfies these.

@Darksonn Darksonn closed this Dec 5, 2020
@blasrodri
Copy link
Contributor Author

blasrodri commented Dec 5, 2020

@taiki-e based on your comment do you think we should wait until rust-lang/rust#72631 in order to implement this properly?

@Darksonn
Copy link
Contributor

Darksonn commented Dec 5, 2020

That would be one solution, but it's unclear when and if it will be merged, which is why I didn't just merge this now to wait for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-stream Module: tokio/stream

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants