Add AsyncBufReadExt::read_line #1556
Conversation
taiki-e
force-pushed
the
io-read_line
branch
3 times, most recently
from
3784773
to
9adc2de
Compare
|
EDIT: Fixed in rust-lang/rust#60501 |
taiki-e
force-pushed
the
io-read_line
branch
2 times, most recently
from
354c38e
to
a4c98c0
Compare
|
This looks good to me, but has conflicts with master-- will merge after rebase. |
|
Rebased. |
|
Nice! |
| let mut v = Vec::new(); | ||
| assert_eq!(run(buf.read_until(b'3', &mut v)).unwrap(), 2); | ||
| assert_eq!(v, b"12"); | ||
|
|
||
| let mut buf = MaybePending::new(b"12333"); | ||
| let mut buf = b"12333".interleave_pending(); |
There was a problem hiding this comment.
Note that this won't add a pending in between reads that actually return data like the old implementation, it will be like Pending, Ready(b"12333"), Pending, Ready(b"").
If you want to test reading data multiple times my plan elsewhere was to use TryStreamExt::into_async_read, so something like stream::iter(vec![b"12", b"33", b"3"]).map(Ok).into_async_read().interleave_pending().
There was a problem hiding this comment.
Thanks for pointing that out. I updated this PR after checking locally that the new test can detect problems like #1566, but the code you suggested is even better. I will open a PR
There was a problem hiding this comment.
Hmm... thread 'maybe_pending' panicked at 'Attempted to consume from IntoAsyncRead without chunk' at
https://github.com/rust-lang-nursery/futures-rs/blob/f4dad6654b65be08bb0b59f2a274ad18d96e436a/futures/tests/io_read_until.rs#L56
This is because read_until runs consume unconditionally even if used is zero.
https://github.com/rust-lang-nursery/futures-rs/blob/f4dad6654b65be08bb0b59f2a274ad18d96e436a/futures-util/src/io/read_until.rs#L33-L43
https://github.com/rust-lang-nursery/futures-rs/blob/f4dad6654b65be08bb0b59f2a274ad18d96e436a/futures-util/src/try_stream/into_async_read.rs#L149
This can be solved by consuming it only when used is non-zero.
However, the current implementation is the same behavior as std.
https://github.com/rust-lang/rust/blob/55c48b4e823ad3518bb8e6d3fd7da4f2fc0c5fc2/src/libstd/io/mod.rs#L1475-L1492
There was a problem hiding this comment.
Sounds like IntoAsyncRead should special-case consume(0) to be a noop then.
There was a problem hiding this comment.
Sounds good. Opened #1584.
Also,
AsyncBufReadExt::linescan be implemented using this.