Conversation
|
r? @djc |
|
Can you add some high-level context/motivation here? Is it feasible to split this in smaller changes (that still compile/pass tests)? |
1 similar comment
|
Can you add some high-level context/motivation here? Is it feasible to split this in smaller changes (that still compile/pass tests)? |
|
Updated the commit message. |
|
Sorry, didn't address your question. No, I don't think it's feasible to break this into smaller bits. |
Slices already encode their lengths; use that instead. Remove the unused return value while I'm here. Closes console-rs#225.
Rather than manually implementing non-blocking reads using polling, simply configure the underlying file descriptor to be non-blocking where necessary. Replace libc calls with the normal abstractions from the standard library. This makes the code less error prone and properly encodes type system invariants (such as `io::Read::read` requiring its receiver to be mutable) which were previously not preserved because the implementation used raw file descriptors.
|
Sorry, this is a larger change than I feel comfortable reviewing (since I have limited experience with this crate and the underlying Unix APIs). The transformation from select to temporarily changing the socket to be non-blocking seems pretty tricky to get right and I don't think the motivation is sufficient given the risk. |
|
Who would be an appropriate reviewer? @atanunq wrote the original code in c8ea46f and @mitsuhiko modified it in #169. |
|
My impression is that @mitsuhiko doesn't have much capacity for console in the near future. |
|
The original code written by @atanunq borrowed from rustyline, which uses polling, but rustyline also uses a non-zero timeout. I'd love for one of these two original authors to weigh in. Also tagging @pksunkara who approved the original changes. |
2 participants
See commit message.