Add AllowSyncIo wrapper #76
Conversation
|
Thanks. This type would be more generally useful bedsides just wrapping sync IO. For example, you could take an async IO handle, wrap it with some type that maintains the Async properties but doesn’t impl the Async traits. In this case, the type is still Async but just doesn’t impl the traits. Given that, I wonder if there could be a more fitting name. |
|
Yeah, I've got no problem changing the name-- I'm sure there's something better we could come up with. |
|
@carllerche @alexcrichton Any ideas for a better name here? |
|
Nah sorry, no ideas here :( |
|
|
|
@Ralith Yeah, originally I called it |
|
Also @carllerche if you're trying to use it in that fashion, it should use |
|
@cramertj is there a reason not to always use Some naming thoughts: |
|
@carllerche If we see |
|
@carllerche I've updated this PR to reflect the discussion. |
|
Rebased. |
|
@carllerche Are there any more changes you're looking for here? |
src/allow_std.rs
Outdated
| fn shutdown(&mut self) -> Poll<(), io::Error> { | ||
| Ok(Async::Ready(())) | ||
| } | ||
| fn write_buf<B: Buf>(&mut self, buf: &mut B) -> Poll<usize, io::Error> { |
There was a problem hiding this comment.
This looks like the same impl as the default impl. Is there a reason why you included it?
There was a problem hiding this comment.
It was different when I used try! instead of try_nb!, but that's no longer the case, so I can remove it now.
src/allow_std.rs
Outdated
| impl<T> AsyncRead for AllowStdIo<T> where T: io::Read { | ||
| // TODO: override prepare_unitialized_buffer once `Read::initializer` is stable. | ||
| // See rust-lang/rust #42788 | ||
| fn read_buf<B: BufMut>(&mut self, buf: &mut B) -> Poll<usize, io::Error> { |
There was a problem hiding this comment.
This looks like the same impl as the default trait impl. Is there a reason you included it?
src/allow_std.rs
Outdated
| /// `AllowStdIo` will cause the event loop to block, so they should be used | ||
| /// with care. | ||
| #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] | ||
| pub struct AllowStdIo<T>(pub T); |
There was a problem hiding this comment.
I think it would be better to make the inner representation of the struct private and add AllowStdIo::new. This allows the internals to be changed if needed.
Also, could you addget_ref / get_mut / into_inner implementations for this type?
This is nice to have when you're being lazy and combining synchronous file IO or stdin/out with the various tokio-io combinators. In general, I think it's good to discourage this kind of behavior, but I think the name in the API (
AllowSyncIo) makes it pretty obvious and explicit what's going on.