add stream::peekable #366
add stream::peekable #366
Conversation
peek() implementation is still missing
starsheriff
force-pushed
the
stream_peekable
branch
from
ecf962e to
a2dc753
Compare
src/stream/stream/peekable.rs
Outdated
| } | ||
| None => { | ||
| let next = futures_core::ready!(self.as_mut().stream().poll_next(cx)); | ||
| match next { |
| Some(_) => { | ||
| let v = Poll::Ready(self.as_mut().peeked().take().unwrap()); | ||
| *self.as_mut().peeked() = None; | ||
| v |
There was a problem hiding this comment.
In my opinion, the peeked just should hold the next. And every poll_next, it will be changed to new next. Why do *self.as_mut().peeked() = None;
src/stream/stream/peekable.rs
Outdated
| S: Stream, | ||
| { | ||
| pin_utils::unsafe_pinned!(stream: S); | ||
| pin_utils::unsafe_unpinned!(peeked: Option<Option<S::Item>>); |
There was a problem hiding this comment.
It'd be nice if this could be updated to use pin-project-lite the way we've done for other combinators (we merged this in the last week; in general it should be nicer!)
|
@starsheriff ping; just checking in if there's anything we can do to help unblock you. I'd very much would like to see this PR land! |
The comments I got helped a lot already. Haven't had the time the last days to work on it but will pick it up again now. I will ask if I need more help. |
| S: Sized, | ||
| { | ||
| stream: S, | ||
| peeked: Option<Poll<Option<S::Item>>>, |
There was a problem hiding this comment.
this is what I landed on as the value for peeked
| match &self.peeked { | ||
| Some(peeked) => &peeked, | ||
| None => { | ||
| // how to get the next `next` value? What about `Context` |
There was a problem hiding this comment.
this is where I currently have troubles @yoshuawuyts , I have no idea how to get the next next value from here. Given that I store a Option<Poll<Option<S::Item>>> in the struct.
|
@starsheriff The |
Currently everything is breaking. I am still trying to figure out the type. I guess that
peekshould return a Futurepub fn peak(&mut self) -> &PeekFuture<Option<Self::Item>>ref #129