Skip to content

add some futures_core::stream::Stream impls for AsyncInputStream #80

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

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

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Aug 8, 2025

WIP: This PR still needs some tests

Closes #77

/// items of `Result<Vec<u8>, std::io::Error>`. The returned byte vectors
/// will be at most 8k. If you want to control chunk size, use
/// `Self::into_stream_of`.
pub fn into_stream(self) -> AsyncInputChunkStream {
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling into_stream() on an AsyncInputStream feels odd. 😆

Could this instead be:

impl AsyncInputChunkStream {
    pub fn new(stream: AsyncInputStream) -> Self {
        Self {
            stream,
            chunk_size: 8 * 1024,
        }
    }

    pub fn with_size(stream: AsyncInputStream, chunk_size: usize) -> Self {
        Self {
            stream,
            chunk_size,
        }
    }
}

which makes it somewhat analogous to Vec::new and Vec::with_capacity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think anyone wants to think about AsyncInputChunkStream as a type itself, I would make it a private type and just return impl Stream<Item = ...> since that is all that matters, except we need to return a concrete type in case the user wants the into_inner method on the ChunkStream/ByteStream. I don't think the analogy to Vec really holds much water.

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.

futures_core::stream::Stream interop
2 participants