Skip to content
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

async LineWriter fix #2131 #2477

Merged
merged 9 commits into from Oct 8, 2021

Conversation

FelipeLema
Copy link
Contributor

@FelipeLema FelipeLema commented Aug 20, 2021

fix #2131

  • finish code
  • document
  • add tests

This is based on std::io::LineWriter and its LineWriterShim sidekick

@FelipeLema
Copy link
Contributor Author

FelipeLema commented Aug 20, 2021

I'm having trouble on handling BufWriter pinned type between LineWriter and LineWriterShim right now. Any pointers are appreciated

futures-util/src/io/line_writer.rs Outdated Show resolved Hide resolved
futures-util/src/io/line_writer.rs Outdated Show resolved Hide resolved
futures-util/src/io/line_writer.rs Outdated Show resolved Hide resolved
futures-util/src/io/line_writer.rs Outdated Show resolved Hide resolved
futures-util/src/io/line_writer.rs Outdated Show resolved Hide resolved
futures-util/src/io/line_writer.rs Outdated Show resolved Hide resolved
futures-util/src/io/line_writer.rs Outdated Show resolved Hide resolved
futures-util/src/io/buf_writer.rs Outdated Show resolved Hide resolved
futures-util/src/io/buf_writer.rs Outdated Show resolved Hide resolved
futures-util/src/io/buf_writer.rs Outdated Show resolved Hide resolved
FelipeLema added 4 commits Aug 21, 2021
doing so saves me thinking on how to deal with pin and fields
- translate `std::io::LineWriter`'s unit test
- rename `inner` field to `buf_writer` to avoid confusion with
`BufWriter::inner`
- the above lead me to re-write the logic accordingly
- add methods to access `buf_writer`'s fields
- sorting out types, cannot compile for now
FelipeLema added 4 commits Aug 24, 2021
- default `poll_write_vectored` behaves differently of
`std::io::LineWriter::write_vectored`
- ported from
https://github.com/rust-lang/rust/blob/673d0db5e393e9c64897005b470bfeb6d5aec61b/library/std/src/io/buffered/tests.rs
- that code has lots of comments, you may wanna check it out
- translated tests for `std` 's `line_vectored`
- added `inner_poll_write_vectored` to keep code explicit and verbose
@FelipeLema
Copy link
Contributor Author

FelipeLema commented Sep 2, 2021

I didn't finish porting the last unit test, but it should be good as it is right now. I rather not keep this going on for too long

Maybe someone else will add it in the near future.

@FelipeLema FelipeLema marked this pull request as ready for review Sep 2, 2021
taiki-e
taiki-e approved these changes Oct 8, 2021
Copy link
Member

@taiki-e taiki-e left a comment

LGTM. Thanks @FelipeLema!

@taiki-e taiki-e merged commit 3601bb7 into rust-lang:master Oct 8, 2021
22 checks passed
@FelipeLema FelipeLema deleted the like_std_io_LineWriter-2131 branch Oct 14, 2021
@taiki-e taiki-e added 0.3-backport: pending A-io labels Nov 22, 2021
taiki-e pushed a commit that referenced this issue Nov 23, 2021
@taiki-e taiki-e mentioned this pull request Nov 23, 2021
@taiki-e taiki-e added 0.3-backport: completed and removed 0.3-backport: pending labels Nov 23, 2021
taiki-e pushed a commit that referenced this issue Nov 23, 2021
taiki-e pushed a commit that referenced this issue Nov 23, 2021
@taiki-e taiki-e mentioned this pull request Nov 23, 2021
@taiki-e
Copy link
Member

taiki-e commented Nov 23, 2021

Published in 0.3.18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants