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

draft Buf writer #92

Merged
merged 6 commits into from
Sep 24, 2019
Merged

draft Buf writer #92

merged 6 commits into from
Sep 24, 2019

Conversation

Drevoed
Copy link
Contributor

@Drevoed Drevoed commented Aug 20, 2019

This pull request will resolve #83 when merged.

It includes asynchronous BufWriter and LineWriter with tests.
Currently I'm having problems with implementing poll_write for BufWriter, because trying to call
self.buf().write(buf).poll(cx) results in

error[E0599]: no method named `poll` found for type `io::write::WriteFuture<'_, std::vec::Vec<u8>>` in the current scope
   --> src/io/buf_writer.rs:239:33
    |
239 |             self.buf().write(buf).poll(cx)
    |                                 ^^^^
    |
   ::: src/io/write.rs:149:1
    |
149 | pub struct WriteFuture<'a, T: Unpin + ?Sized> {
    | --------------------------------------------- method `poll` not found for this
    |
    = help: items from traits can only be used if the trait is implemented and in scope
    = note: the following traits define an item `poll`, perhaps you need to implement one of them:
            candidate #1: `core::future::future::Future`
            candidate #2: `futures::future::Future`
            candidate #3: `futures::stream::Stream`

I assume that's because WriteFuture has trait bounds on ?Sized but I'm not sure how to get around that.

The above error was fixed by wrapping the buf in Pin explicitly.

Previous PR was closed because I forgot to mark it as draft.

@Drevoed Drevoed marked this pull request as ready for review August 20, 2019 19:14
@skade skade requested a review from a user August 26, 2019 21:42
@skade
Copy link
Collaborator

skade commented Aug 26, 2019

I think this PR could use some tests, especially around how BufWriter is used.

ghost
ghost previously requested changes Aug 27, 2019
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Can we perhaps move LineWriter into a new pull request to keep this one simple?

@yoshuawuyts
Copy link
Contributor

Ref #131 also (tracking issue for async_std::io)

@yoshuawuyts
Copy link
Contributor

@Drevoed o/ wanted to check in how things are. Is there anything we can do to help you get unblocked?

@Drevoed
Copy link
Contributor Author

Drevoed commented Sep 17, 2019

@yoshuawuyts I'm currently a bit busy with day-time job but will try to implement requested changes ASAP, I don't think I will have any problems, but if I will - I am going to ask for assistance surely :)

@yoshuawuyts yoshuawuyts added the enhancement New feature or request label Sep 18, 2019
@yoshuawuyts yoshuawuyts requested a review from a user September 24, 2019 20:01
@yoshuawuyts yoshuawuyts dismissed ghost ’s stale review September 24, 2019 20:09

changes resolved

Copy link
Contributor

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

This patch looks excellent to me! -- I think we should go ahead and merge this in. Thanks so much for all the work you've put into this; this is really good!

@yoshuawuyts yoshuawuyts merged commit 90872dd into async-rs:master Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add BufWriter
3 participants