From 464efa69709464dc331616f12292e2ccdc56f658 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sun, 8 Sep 2024 11:51:42 +0200 Subject: [PATCH 1/6] Use select instead of poll on MacOS for /dev/tty See #802 --- src/tty/unix.rs | 84 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 61 insertions(+), 23 deletions(-) diff --git a/src/tty/unix.rs b/src/tty/unix.rs index 08d754810..e3e378d64 100644 --- a/src/tty/unix.rs +++ b/src/tty/unix.rs @@ -1,6 +1,4 @@ //! Unix specific definitions -#[cfg(feature = "buffer-redux")] -use buffer_redux::BufReader; use std::cmp; use std::collections::HashMap; use std::fs::{File, OpenOptions}; @@ -13,12 +11,16 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::{self, SyncSender}; use std::sync::{Arc, Mutex}; +#[cfg(feature = "buffer-redux")] +use buffer_redux::BufReader; use log::{debug, warn}; use nix::errno::Errno; use nix::poll::{self, PollFlags, PollTimeout}; use nix::sys::select::{self, FdSet}; #[cfg(not(feature = "termios"))] use nix::sys::termios::Termios; +#[cfg(target_os = "macos")] +use nix::sys::time::TimeValLike; use nix::unistd::{close, isatty, read, write}; #[cfg(feature = "termios")] use termios::Termios; @@ -195,6 +197,8 @@ pub struct PosixRawReader { key_map: PosixKeyMap, // external print reader pipe_reader: Option, + #[cfg(target_os = "macos")] + is_dev_tty: bool, } impl AsFd for PosixRawReader { @@ -243,6 +247,7 @@ impl PosixRawReader { config: &Config, key_map: PosixKeyMap, pipe_reader: Option, + #[cfg(target_os = "macos")] is_dev_tty: bool, ) -> Self { let inner = TtyIn { fd, sigwinch_pipe }; #[cfg(any(not(feature = "buffer-redux"), test))] @@ -259,6 +264,8 @@ impl PosixRawReader { parser: Parser::new(), key_map, pipe_reader, + #[cfg(target_os = "macos")] + is_dev_tty, } } @@ -306,9 +313,8 @@ impl PosixRawReader { match self.poll(timeout) { // Ignore poll errors, it's very likely we'll pick them up on // the next read anyway. - Ok(0) | Err(_) => Ok(E::ESC), - Ok(n) => { - debug_assert!(n > 0, "{}", n); + Ok(false) | Err(_) => Ok(E::ESC), + Ok(true) => { // recurse, and add the alt modifier. let E(k, m) = self._do_escape_sequence(false)?; Ok(E(k, m | M::ALT)) @@ -702,38 +708,53 @@ impl PosixRawReader { }) } - fn poll(&mut self, timeout_ms: PollTimeout) -> Result { + fn poll(&mut self, timeout: PollTimeout) -> Result { let n = self.tty_in.buffer().len(); if n > 0 { - return Ok(n as i32); + return Ok(true); } + #[cfg(target_os = "macos")] + if self.is_dev_tty { + // poll doesn't work for /dev/tty on MacOS but select does + return Ok(match self.select(Some(timeout), false /* ignored */)? { + // ugly but ESC means timeout... + Event::KeyPress(KeyEvent::ESC) => false, + _ => true, + }); + } + debug!(target: "rustyline", "poll with: {:?}", timeout); let mut fds = [poll::PollFd::new(self.as_fd(), PollFlags::POLLIN)]; - let r = poll::poll(&mut fds, timeout_ms); + let r = poll::poll(&mut fds, timeout); + debug!(target: "rustyline", "poll returns: {:?}", r); match r { - Ok(n) => Ok(n), + Ok(n) => Ok(n != 0), Err(Errno::EINTR) => { if self.tty_in.get_ref().sigwinch()? { Err(ReadlineError::WindowResized) } else { - Ok(0) // Ignore EINTR while polling + Ok(false) // Ignore EINTR while polling } } Err(e) => Err(e.into()), } } - fn select(&mut self, single_esc_abort: bool) -> Result { + // timeout is used only with /dev/tty on MacOs + fn select(&mut self, timeout: Option, single_esc_abort: bool) -> Result { let tty_in = self.as_fd(); let sigwinch_pipe = self .tty_in .get_ref() .sigwinch_pipe .map(|fd| unsafe { BorrowedFd::borrow_raw(fd) }); - let pipe_reader = self - .pipe_reader - .as_ref() - .map(|pr| pr.lock().unwrap().0.as_raw_fd()) - .map(|fd| unsafe { BorrowedFd::borrow_raw(fd) }); + let pipe_reader = if timeout.is_some() { + None + } else { + self.pipe_reader + .as_ref() + .map(|pr| pr.lock().unwrap().0.as_raw_fd()) + .map(|fd| unsafe { BorrowedFd::borrow_raw(fd) }) + }; loop { let mut readfds = FdSet::new(); if let Some(sigwinch_pipe) = sigwinch_pipe { @@ -743,7 +764,14 @@ impl PosixRawReader { if let Some(pipe_reader) = pipe_reader { readfds.insert(pipe_reader); } - if let Err(err) = select::select(None, Some(&mut readfds), None, None, None) { + let mut timeout = match timeout { + Some(pt) => pt + .as_millis() + .map(|ms| nix::sys::time::TimeVal::milliseconds(ms as i64)), + None => None, + }; + if let Err(err) = select::select(None, Some(&mut readfds), None, None, timeout.as_mut()) + { if err == Errno::EINTR && self.tty_in.get_ref().sigwinch()? { return Err(ReadlineError::WindowResized); } else if err != Errno::EINTR { @@ -756,8 +784,16 @@ impl PosixRawReader { self.tty_in.get_ref().sigwinch()?; return Err(ReadlineError::WindowResized); } else if readfds.contains(tty_in) { - // prefer user input over external print - return self.next_key(single_esc_abort).map(Event::KeyPress); + if timeout.is_some() { + // ugly but ENTER means success (no timeout)... + return Ok(Event::KeyPress(KeyEvent::ENTER)); + } else { + // prefer user input over external print + return self.next_key(single_esc_abort).map(Event::KeyPress); + } + } else if timeout.is_some() { + // ugly but ESC means timeout... + return Ok(Event::KeyPress(KeyEvent::ESC)); } else if let Some(ref pipe_reader) = self.pipe_reader { let mut guard = pipe_reader.lock().unwrap(); let mut buf = [0; 1]; @@ -776,7 +812,7 @@ impl RawReader for PosixRawReader { #[cfg(not(feature = "signal-hook"))] fn wait_for_input(&mut self, single_esc_abort: bool) -> Result { match self.pipe_reader { - Some(_) => self.select(single_esc_abort), + Some(_) => self.select(None, single_esc_abort), None => self.next_key(single_esc_abort).map(Event::KeyPress), } } @@ -800,7 +836,7 @@ impl RawReader for PosixRawReader { self.timeout_ms }; match self.poll(timeout_ms) { - Ok(0) => { + Ok(false) => { // single escape } Ok(_) => { @@ -1117,14 +1153,14 @@ impl Renderer for PosixRenderer { } fn move_cursor_at_leftmost(&mut self, rdr: &mut PosixRawReader) -> Result<()> { - if rdr.poll(PollTimeout::ZERO)? != 0 { + if rdr.poll(PollTimeout::ZERO)? { debug!(target: "rustyline", "cannot request cursor location"); return Ok(()); } /* Report cursor location */ self.write_and_flush("\x1b[6n")?; /* Read the response: ESC [ rows ; cols R */ - if rdr.poll(PollTimeout::from(100u8))? == 0 + if !rdr.poll(PollTimeout::from(100u8))? || rdr.next_char()? != '\x1b' || rdr.next_char()? != '[' || read_digits_until(rdr, ';')?.is_none() @@ -1420,6 +1456,8 @@ impl Term for PosixTerminal { config, key_map, self.pipe_reader.clone(), + #[cfg(target_os = "macos")] + self.close_on_drop, ) } From 33302a898af792ca415b892fdc41b8125e41d108 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sun, 8 Sep 2024 13:12:47 +0200 Subject: [PATCH 2/6] Oops --- src/tty/unix.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tty/unix.rs b/src/tty/unix.rs index e3e378d64..47e5e8d74 100644 --- a/src/tty/unix.rs +++ b/src/tty/unix.rs @@ -19,7 +19,6 @@ use nix::poll::{self, PollFlags, PollTimeout}; use nix::sys::select::{self, FdSet}; #[cfg(not(feature = "termios"))] use nix::sys::termios::Termios; -#[cfg(target_os = "macos")] use nix::sys::time::TimeValLike; use nix::unistd::{close, isatty, read, write}; #[cfg(feature = "termios")] From 792f9a743a9e6a4bab180364e1f0523e603b20f1 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sun, 8 Sep 2024 13:16:12 +0200 Subject: [PATCH 3/6] Oops --- src/tty/unix.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tty/unix.rs b/src/tty/unix.rs index 47e5e8d74..3ec5a74b1 100644 --- a/src/tty/unix.rs +++ b/src/tty/unix.rs @@ -818,7 +818,7 @@ impl RawReader for PosixRawReader { #[cfg(feature = "signal-hook")] fn wait_for_input(&mut self, single_esc_abort: bool) -> Result { - self.select(single_esc_abort) + self.select(None, single_esc_abort) } fn next_key(&mut self, single_esc_abort: bool) -> Result { From 1807fe0ea64bf6c06c11f1bc1ed0c08c1f3e0b93 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 14 Dec 2024 15:50:07 +0100 Subject: [PATCH 4/6] Introduce Event::Timeout --- src/keymap.rs | 1 + src/tty/mod.rs | 2 ++ src/tty/unix.rs | 9 +++------ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/keymap.rs b/src/keymap.rs index 784c91016..5be616001 100644 --- a/src/keymap.rs +++ b/src/keymap.rs @@ -441,6 +441,7 @@ impl<'b> InputState<'b> { tty::Event::ExternalPrint(msg) => { wrt.external_print(msg)?; } + _ => {} } } } diff --git a/src/tty/mod.rs b/src/tty/mod.rs index 62484f042..37f80936e 100644 --- a/src/tty/mod.rs +++ b/src/tty/mod.rs @@ -19,6 +19,8 @@ pub trait RawMode: Sized { pub enum Event { KeyPress(KeyEvent), ExternalPrint(String), + #[cfg(unix)] + Timeout(bool), } /// Translate bytes read from stdin to keys. diff --git a/src/tty/unix.rs b/src/tty/unix.rs index 3ec5a74b1..c8b82ec9a 100644 --- a/src/tty/unix.rs +++ b/src/tty/unix.rs @@ -716,8 +716,7 @@ impl PosixRawReader { if self.is_dev_tty { // poll doesn't work for /dev/tty on MacOS but select does return Ok(match self.select(Some(timeout), false /* ignored */)? { - // ugly but ESC means timeout... - Event::KeyPress(KeyEvent::ESC) => false, + Event::Timeout(true) => false, _ => true, }); } @@ -784,15 +783,13 @@ impl PosixRawReader { return Err(ReadlineError::WindowResized); } else if readfds.contains(tty_in) { if timeout.is_some() { - // ugly but ENTER means success (no timeout)... - return Ok(Event::KeyPress(KeyEvent::ENTER)); + return Ok(Event::Timeout(false)); } else { // prefer user input over external print return self.next_key(single_esc_abort).map(Event::KeyPress); } } else if timeout.is_some() { - // ugly but ESC means timeout... - return Ok(Event::KeyPress(KeyEvent::ESC)); + return Ok(Event::Timeout(true)); } else if let Some(ref pipe_reader) = self.pipe_reader { let mut guard = pipe_reader.lock().unwrap(); let mut buf = [0; 1]; From ea504426911679b9e9487734d3d4efa4acc60e5e Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 14 Dec 2024 16:03:24 +0100 Subject: [PATCH 5/6] Try to fix clippy warning --- src/tty/mod.rs | 2 +- src/tty/unix.rs | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/tty/mod.rs b/src/tty/mod.rs index 37f80936e..491315482 100644 --- a/src/tty/mod.rs +++ b/src/tty/mod.rs @@ -19,7 +19,7 @@ pub trait RawMode: Sized { pub enum Event { KeyPress(KeyEvent), ExternalPrint(String), - #[cfg(unix)] + #[cfg(target_os = "macos")] Timeout(bool), } diff --git a/src/tty/unix.rs b/src/tty/unix.rs index c8b82ec9a..2f5a4d5c5 100644 --- a/src/tty/unix.rs +++ b/src/tty/unix.rs @@ -782,14 +782,17 @@ impl PosixRawReader { self.tty_in.get_ref().sigwinch()?; return Err(ReadlineError::WindowResized); } else if readfds.contains(tty_in) { + #[cfg(target_os = "macos")] if timeout.is_some() { return Ok(Event::Timeout(false)); - } else { - // prefer user input over external print - return self.next_key(single_esc_abort).map(Event::KeyPress); } + // prefer user input over external print + return self.next_key(single_esc_abort).map(Event::KeyPress); } else if timeout.is_some() { + #[cfg(target_os = "macos")] return Ok(Event::Timeout(true)); + #[cfg(not(target_os = "macos"))] + unreachable!() } else if let Some(ref pipe_reader) = self.pipe_reader { let mut guard = pipe_reader.lock().unwrap(); let mut buf = [0; 1]; From 2085d0c018f6ae0b1716dc09db6659c8ad87e813 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 14 Dec 2024 16:18:04 +0100 Subject: [PATCH 6/6] Fix build on unix --- src/keymap.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/keymap.rs b/src/keymap.rs index 5be616001..ac32156a9 100644 --- a/src/keymap.rs +++ b/src/keymap.rs @@ -441,6 +441,7 @@ impl<'b> InputState<'b> { tty::Event::ExternalPrint(msg) => { wrt.external_print(msg)?; } + #[cfg(target_os = "macos")] _ => {} } }