futures-rs
futures-rs copied to clipboard
Add `IntoAsyncRead::into_inner()` method and test
This is useful when you don't necessarily want to consume the entire stream as a reader, but still want to retain the stream for other uses.
~The nested tuple/Option/tuple is a bit awkward, but minimal. Do you think it'd be worth making a single-purpose enum to return here instead?~ For readability of code that calls this method, I added an enum.
This build failure is pretty puzzling to me, as I cannot duplicate it even with the exact same nightly compiler on Ubuntu 18.04: https://travis-ci.com/rust-lang/futures-rs/jobs/264248283
Hey, sorry for the delay! I don't quite have the time to properly review this right now, but I'll be sure to take a look in the next few days.
All existing into_inner methods discard the combinator state and return only the underlying stream. Any reason why this returns an own enum containing the state instead of returning the Option<St>? (Methods that return combinator state can cause problems when the internal implementation changes.)
@taiki-e sorry for the delay on this reply. The reason it returns the combinator state is to not lose the bytes that have been buffered up to that point. If we just return the stream, those buffered bytes are lost.
@acfoltzer Thanks for your response. I think the motive makes sense, but I would prefer not to expose the internal representation of these combinators.
So, I think it might be preferable to add a method like BufWriter::buffer separate from into_inner.
And it would probably have the following implementation:
pub fn into_inner(self) -> St {
self.stream
// or:
// if let ReadState::Eof = self.state { None } else { Some(self.stream) }
}
/// Returns a reference to the internally buffered data.
pub fn buffer(&self) -> &[u8] {
match self.state {
ReadState::Ready { chunk, chunk_start } => &chunk.as_ref()[chunk_start..],
ReadState::PendingChunk | ReadState::Eof => &[],
}
}
@acfoltzer What do you think?