Terminal no longer requires Read+Write

This allows adding parsed readers without having to worry about
reconciling Read vs. parsed read.

src/render/terminfo.rs

@@ -240,7 +240,7 @@ impl TerminfoRenderer {
 }
 
 impl TerminfoRenderer {
-    pub fn render_to<R: UnixTty + Read, W: UnixTty + Write>(
+    pub fn render_to<R: Read, W: UnixTty + Write>(
         &mut self,
         changes: &[Change],
         _read: &mut R,
@@ -617,22 +617,6 @@ mod test {
         }
     }
 
-    impl Write for FakeTerm {
-        fn write(&mut self, buf: &[u8]) -> IoResult<usize> {
-            self.write.write(buf)
-        }
-
-        fn flush(&mut self) -> IoResult<()> {
-            self.write.flush()
-        }
-    }
-
-    impl Read for FakeTerm {
-        fn read(&mut self, buf: &mut [u8]) -> IoResult<usize> {
-            self.read.read(buf)
-        }
-    }
-
     impl Terminal for FakeTerm {
         fn set_raw_mode(&mut self) -> Result<(), Error> {
             bail!("not implemented");
@@ -661,6 +645,9 @@ mod test {
 
             self.write.set_size(size)
         }
+        fn flush(&mut self) -> Result<(), Error> {
+            Ok(())
+        }
     }
 
     #[test]

src/terminal/mod.rs

@@ -8,7 +8,6 @@ use caps::Capabilities;
 use failure::Error;
 use num::{self, NumCast};
 use std::fmt::Display;
-use std::io::{Read, Write};
 use surface::Change;
 
 #[cfg(unix)]
@@ -47,7 +46,7 @@ pub struct ScreenSize {
 /// If the `set_raw_mode` or `set_cooked_mode` functions are used in
 /// any combination, the implementation is required to restore the
 /// terminal mode that was in effect when it was created.
-pub trait Terminal: Read + Write {
+pub trait Terminal {
     /// Raw mode disables input line buffering, allowing data to be
     /// read as the user presses keys, disables local echo, so keys
     /// pressed by the user do not implicitly render to the terminal
@@ -63,6 +62,9 @@ pub trait Terminal: Read + Write {
     /// Render a series of changes to the terminal output
     fn render(&mut self, changes: &[Change]) -> Result<(), Error>;
 
+    /// Flush any buffered output
+    fn flush(&mut self) -> Result<(), Error>;
+
     /*
     /// Sets the terminal to cooked mode, which is essentially the opposite
     /// to raw mode: input and output processing are enabled.
@@ -92,8 +94,6 @@ pub fn new_terminal(caps: Capabilities) -> Result<impl Terminal, Error> {
     SystemTerminal::new(caps)
 }
 
-const BUF_SIZE: usize = 128;
-
 pub(crate) fn cast<T: NumCast + Display + Copy, U: NumCast>(n: T) -> Result<U, Error> {
     num::cast(n).ok_or_else(|| format_err!("{} is out of bounds for this system", n))
 }

src/terminal/unix.rs

@@ -2,8 +2,9 @@ use failure::Error;
 use istty::IsTty;
 use libc::{self, winsize};
 use std::fs::OpenOptions;
-use std::io::{stdin, stdout, Error as IOError, ErrorKind, Read, Result as IOResult, Write};
+use std::io::{stdin, stdout, Error as IOError, ErrorKind, Read, Write};
 use std::mem;
+use std::ops::Deref;
 use std::os::unix::io::{AsRawFd, RawFd};
 use termios::{
     cfmakeraw, tcdrain, tcflush, tcsetattr, Termios, TCIFLUSH, TCIOFLUSH, TCOFLUSH, TCSADRAIN,
@@ -13,7 +14,9 @@ use termios::{
 use caps::Capabilities;
 use render::terminfo::TerminfoRenderer;
 use surface::Change;
-use terminal::{cast, ScreenSize, Terminal, BUF_SIZE};
+use terminal::{cast, ScreenSize, Terminal};
+
+const BUF_SIZE: usize = 128;
 
 /// Helper function to duplicate a file descriptor.
 /// The duplicated descriptor will have the close-on-exec flag set.
@@ -51,46 +54,19 @@ pub trait UnixTty {
     fn purge(&mut self, purge: Purge) -> Result<(), Error>;
 }
 
-pub struct TtyHandle {
+struct Fd {
     fd: RawFd,
 }
 
-impl Drop for TtyHandle {
+impl Drop for Fd {
     fn drop(&mut self) {
         unsafe {
             libc::close(self.fd);
         }
     }
 }
-impl Write for TtyHandle {
-    fn write(&mut self, buf: &[u8]) -> Result<usize, IOError> {
-        let size = unsafe { libc::write(self.fd, buf.as_ptr() as *const _, buf.len()) };
-        if size == -1 {
-            Err(IOError::last_os_error())
-        } else {
-            Ok(size as usize)
-        }
-    }
 
-    fn flush(&mut self) -> Result<(), IOError> {
+impl Fd {
-        self.drain()
-            .map_err(|e| IOError::new(ErrorKind::Other, format!("{}", e)))?;
-        Ok(())
-    }
-}
-
-impl Read for TtyHandle {
-    fn read(&mut self, buf: &mut [u8]) -> Result<usize, IOError> {
-        let size = unsafe { libc::read(self.fd, buf.as_mut_ptr() as *mut _, buf.len()) };
-        if size == -1 {
-            Err(IOError::last_os_error())
-        } else {
-            Ok(size as usize)
-        }
-    }
-}
-
-impl TtyHandle {
     pub fn new<S: AsRawFd>(s: &S) -> Result<Self, Error> {
         ensure!(s.is_tty(), "Can only construct a TtyHandle from a tty");
         let fd = dup(s.as_raw_fd())?;
@@ -98,17 +74,96 @@ impl TtyHandle {
     }
 }
 
-impl UnixTty for TtyHandle {
+impl Deref for Fd {
+    type Target = RawFd;
+    fn deref(&self) -> &RawFd {
+        &self.fd
+    }
+}
+
+pub struct TtyReadHandle {
+    fd: Fd,
+}
+
+impl TtyReadHandle {
+    fn new(fd: Fd) -> Self {
+        Self { fd }
+    }
+}
+
+impl Read for TtyReadHandle {
+    fn read(&mut self, buf: &mut [u8]) -> Result<usize, IOError> {
+        let size = unsafe { libc::read(*self.fd, buf.as_mut_ptr() as *mut _, buf.len()) };
+        if size == -1 {
+            Err(IOError::last_os_error())
+        } else {
+            Ok(size as usize)
+        }
+    }
+}
+
+pub struct TtyWriteHandle {
+    fd: Fd,
+    write_buffer: Vec<u8>,
+}
+
+impl TtyWriteHandle {
+    fn new(fd: Fd) -> Self {
+        Self {
+            fd,
+            write_buffer: Vec::with_capacity(BUF_SIZE),
+        }
+    }
+
+    fn flush_local_buffer(&mut self) -> Result<(), IOError> {
+        if self.write_buffer.len() > 0 {
+            do_write(*self.fd, &self.write_buffer)?;
+            self.write_buffer.clear();
+        }
+        Ok(())
+    }
+}
+
+fn do_write(fd: RawFd, buf: &[u8]) -> Result<usize, IOError> {
+    let size = unsafe { libc::write(fd, buf.as_ptr() as *const _, buf.len()) };
+    if size == -1 {
+        Err(IOError::last_os_error())
+    } else {
+        Ok(size as usize)
+    }
+}
+
+impl Write for TtyWriteHandle {
+    fn write(&mut self, buf: &[u8]) -> Result<usize, IOError> {
+        if self.write_buffer.len() + buf.len() > self.write_buffer.capacity() {
+            self.flush()?;
+        }
+        if buf.len() >= self.write_buffer.capacity() {
+            do_write(*self.fd, buf)
+        } else {
+            self.write_buffer.write(buf)
+        }
+    }
+
+    fn flush(&mut self) -> Result<(), IOError> {
+        self.flush_local_buffer()?;
+        self.drain()
+            .map_err(|e| IOError::new(ErrorKind::Other, format!("{}", e)))?;
+        Ok(())
+    }
+}
+
+impl UnixTty for TtyWriteHandle {
     fn get_size(&mut self) -> Result<winsize, Error> {
         let mut size: winsize = unsafe { mem::zeroed() };
-        if unsafe { libc::ioctl(self.fd, libc::TIOCGWINSZ, &mut size) } != 0 {
+        if unsafe { libc::ioctl(*self.fd, libc::TIOCGWINSZ, &mut size) } != 0 {
             bail!("failed to ioctl(TIOCGWINSZ): {}", IOError::last_os_error());
         }
         Ok(size)
     }
 
     fn set_size(&mut self, size: winsize) -> Result<(), Error> {
-        if unsafe { libc::ioctl(self.fd, libc::TIOCSWINSZ, &size as *const _) } != 0 {
+        if unsafe { libc::ioctl(*self.fd, libc::TIOCSWINSZ, &size as *const _) } != 0 {
             bail!(
                 "failed to ioctl(TIOCSWINSZ): {:?}",
                 IOError::last_os_error()
@@ -119,7 +174,7 @@ impl UnixTty for TtyHandle {
     }
 
     fn get_termios(&mut self) -> Result<Termios, Error> {
-        Termios::from_fd(self.fd).map_err(|e| format_err!("get_termios failed: {}", e))
+        Termios::from_fd(*self.fd).map_err(|e| format_err!("get_termios failed: {}", e))
     }
 
     fn set_termios(&mut self, termios: &Termios, when: SetAttributeWhen) -> Result<(), Error> {
@@ -128,11 +183,11 @@ impl UnixTty for TtyHandle {
             SetAttributeWhen::AfterDrainOutputQueue => TCSADRAIN,
             SetAttributeWhen::AfterDrainOutputQueuePurgeInputQueue => TCSAFLUSH,
         };
-        tcsetattr(self.fd, when, termios).map_err(|e| format_err!("set_termios failed: {}", e))
+        tcsetattr(*self.fd, when, termios).map_err(|e| format_err!("set_termios failed: {}", e))
     }
 
     fn drain(&mut self) -> Result<(), Error> {
-        tcdrain(self.fd).map_err(|e| format_err!("tcdrain failed: {}", e))
+        tcdrain(*self.fd).map_err(|e| format_err!("tcdrain failed: {}", e))
     }
 
     fn purge(&mut self, purge: Purge) -> Result<(), Error> {
@@ -141,16 +196,15 @@ impl UnixTty for TtyHandle {
             Purge::OutputQueue => TCOFLUSH,
             Purge::InputAndOutputQueue => TCIOFLUSH,
         };
-        tcflush(self.fd, param).map_err(|e| format_err!("tcflush failed: {}", e))
+        tcflush(*self.fd, param).map_err(|e| format_err!("tcflush failed: {}", e))
     }
 }
 
 /// A unix style terminal
 pub struct UnixTerminal {
-    read: TtyHandle,
+    read: TtyReadHandle,
-    write: TtyHandle,
+    write: TtyWriteHandle,
     saved_termios: Termios,
-    write_buffer: Vec<u8>,
     renderer: TerminfoRenderer,
 }
 
@@ -169,8 +223,8 @@ impl UnixTerminal {
         read: &A,
         write: &B,
     ) -> Result<UnixTerminal, Error> {
-        let read = TtyHandle::new(read)?;
+        let read = TtyReadHandle::new(Fd::new(read)?);
-        let mut write = TtyHandle::new(write)?;
+        let mut write = TtyWriteHandle::new(Fd::new(write)?);
         let saved_termios = write.get_termios()?;
         let renderer = TerminfoRenderer::new(caps);
 
@@ -179,7 +233,6 @@ impl UnixTerminal {
             write,
             saved_termios,
             renderer,
-            write_buffer: Vec::with_capacity(BUF_SIZE),
         })
     }
 
@@ -191,38 +244,6 @@ impl UnixTerminal {
         let file = OpenOptions::new().read(true).write(true).open("/dev/tty")?;
         Self::new_with(caps, &file, &file)
     }
-
-    fn flush_local_buffer_only(&mut self) -> IOResult<()> {
-        if self.write_buffer.len() > 0 {
-            self.write.write(&self.write_buffer)?;
-            self.write_buffer.clear();
-        }
-        Ok(())
-    }
-}
-
-impl Read for UnixTerminal {
-    fn read(&mut self, buf: &mut [u8]) -> IOResult<usize> {
-        self.read.read(buf)
-    }
-}
-
-impl Write for UnixTerminal {
-    fn write(&mut self, buf: &[u8]) -> IOResult<usize> {
-        if self.write_buffer.len() + buf.len() > self.write_buffer.capacity() {
-            self.flush_local_buffer_only()?;
-        }
-        if buf.len() >= self.write_buffer.capacity() {
-            self.write.write(buf)
-        } else {
-            self.write_buffer.write(buf)
-        }
-    }
-
-    fn flush(&mut self) -> IOResult<()> {
-        self.flush_local_buffer_only()?;
-        self.write.flush()
-    }
 }
 
 impl Terminal for UnixTerminal {
@@ -258,6 +279,11 @@ impl Terminal for UnixTerminal {
         self.renderer
             .render_to(changes, &mut self.read, &mut self.write)
     }
+    fn flush(&mut self) -> Result<(), Error> {
+        self.write
+            .flush()
+            .map_err(|e| format_err!("flush failed: {}", e))
+    }
 }
 
 impl Drop for UnixTerminal {

src/terminal/windows.rs

@@ -20,7 +20,9 @@ use winapi::um::winnt::DUPLICATE_SAME_ACCESS;
 use caps::Capabilities;
 use render::windows::WindowsConsoleRenderer;
 use surface::Change;
-use terminal::{cast, ScreenSize, Terminal, BUF_SIZE};
+use terminal::{cast, ScreenSize, Terminal};
+
+const BUF_SIZE: usize = 128;
 
 pub trait ConsoleInputHandle {
     fn set_input_mode(&mut self, mode: u32) -> Result<(), Error>;
@@ -108,20 +110,23 @@ impl ConsoleInputHandle for InputHandle {
 
 struct OutputHandle {
     handle: RawHandle,
+    write_buffer: Vec<u8>,
 }
 
-impl Drop for OutputHandle {
+impl OutputHandle {
-    fn drop(&mut self) {
+    fn new(handle: RawHandle) -> Self {
-        unsafe { CloseHandle(self.handle) };
+        Self {
+            handle,
+            write_buffer: Vec::with_capacity(BUF_SIZE),
+        }
     }
 }
 
-impl Write for OutputHandle {
+fn do_write(handle: RawHandle, buf: &[u8]) -> IOResult<usize> {
-    fn write(&mut self, buf: &[u8]) -> IOResult<usize> {
     let mut num_wrote = 0;
     let ok = unsafe {
         WriteFile(
-                self.handle,
+            handle,
             buf.as_ptr() as *const _,
             buf.len() as u32,
             &mut num_wrote,
@@ -133,9 +138,31 @@ impl Write for OutputHandle {
     } else {
         Ok(num_wrote as usize)
     }
+}
+
+impl Drop for OutputHandle {
+    fn drop(&mut self) {
+        unsafe { CloseHandle(self.handle) };
+    }
+}
+
+impl Write for OutputHandle {
+    fn write(&mut self, buf: &[u8]) -> IOResult<usize> {
+        if self.write_buffer.len() + buf.len() > self.write_buffer.capacity() {
+            self.flush()?;
+        }
+        if buf.len() >= self.write_buffer.capacity() {
+            do_write(self.handle, buf)
+        } else {
+            self.write_buffer.write(buf)
+        }
     }
 
     fn flush(&mut self) -> IOResult<()> {
+        if self.write_buffer.len() > 0 {
+            do_write(self.handle, &self.write_buffer)?;
+            self.write_buffer.clear();
+        }
         Ok(())
     }
 }
@@ -239,7 +266,6 @@ impl ConsoleOutputHandle for OutputHandle {
 pub struct WindowsTerminal {
     input_handle: InputHandle,
     output_handle: OutputHandle,
-    write_buffer: Vec<u8>,
     saved_input_mode: u32,
     saved_output_mode: u32,
     renderer: WindowsConsoleRenderer,
@@ -279,9 +305,7 @@ impl WindowsTerminal {
         }
 
         let mut input_handle = InputHandle { handle: dup(read)? };
-        let mut output_handle = OutputHandle {
+        let mut output_handle = OutputHandle::new(dup(write)?);
-            handle: dup(write)?,
-        };
 
         let saved_input_mode = input_handle.get_input_mode()?;
         let saved_output_mode = output_handle.get_output_mode()?;
@@ -293,7 +317,6 @@ impl WindowsTerminal {
             saved_input_mode,
             saved_output_mode,
             renderer,
-            write_buffer: Vec::with_capacity(BUF_SIZE),
         })
     }
 
@@ -319,33 +342,6 @@ impl WindowsTerminal {
     }
 }
 
-impl Read for WindowsTerminal {
-    fn read(&mut self, buf: &mut [u8]) -> IOResult<usize> {
-        self.input_handle.read(buf)
-    }
-}
-
-impl Write for WindowsTerminal {
-    fn write(&mut self, buf: &[u8]) -> IOResult<usize> {
-        if self.write_buffer.len() + buf.len() > self.write_buffer.capacity() {
-            self.flush()?;
-        }
-        if buf.len() >= self.write_buffer.capacity() {
-            self.output_handle.write(buf)
-        } else {
-            self.write_buffer.write(buf)
-        }
-    }
-
-    fn flush(&mut self) -> IOResult<()> {
-        if self.write_buffer.len() > 0 {
-            self.output_handle.write(&self.write_buffer)?;
-            self.write_buffer.clear();
-        }
-        self.output_handle.flush()
-    }
-}
-
 impl Terminal for WindowsTerminal {
     fn set_raw_mode(&mut self) -> Result<(), Error> {
         let mode = self.input_handle.get_input_mode()?;
@@ -396,4 +392,9 @@ impl Terminal for WindowsTerminal {
         self.renderer
             .render_to(changes, &mut self.input_handle, &mut self.output_handle)
     }
+    fn flush(&mut self) -> Result<(), Error> {
+        self.output_handle
+            .flush()
+            .map_err(|e| format_err!("flush failed: {}", e))
+    }
 }