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

Don't call poll when keyseq_timeout is set to 0 to avoid broken poll implementation for devices on MacOS. #802

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PaulJuliusMartinez
Copy link

The poll system call is broken on MacOS and doesn't work properly for devices, such as /dev/tty.

As a result, reading single escapes is broken when using Behavior::PreferTerm, which causes rustyline to read from /dev/tty. After reading a single escape character, the subsequent call to poll will always return Ok(1), so rustyline will try to parse an escape sequence.

This PR makes a small change to at least improve the behavior when keyseq_timeout is set to Some(0). In this case, we bypass the call to poll altogether, and instead check whether there is any buffered input to determine whether a single escape keypress has been entered or if an escape sequence has been entered, which is a pretty effective heuristic.

I don't know if there's any good workaround for the broken poll implementation when keyseq_timeout is non-zero, but this is at least an improvement.

This should resolve #607.

…if there's any buffered input to determine whether it's a single escape or an escape sequence.
@gwenn
Copy link
Collaborator

gwenn commented Sep 8, 2024

I manage to reproduce the issue.
Only with this patch to debug:

diff --git a/src/tty/unix.rs b/src/tty/unix.rs
index 08d7548..264fc3a 100644
--- a/src/tty/unix.rs
+++ b/src/tty/unix.rs
@@ -707,8 +707,10 @@ impl PosixRawReader {
         if n > 0 {
             return Ok(n as i32);
         }
+        debug!(target: "rustyline", "poll with: {:?}", timeout_ms);
         let mut fds = [poll::PollFd::new(self.as_fd(), PollFlags::POLLIN)];
         let r = poll::poll(&mut fds, timeout_ms);
+        debug!(target: "rustyline", "poll returns: {:?}", r);
         match r {
             Ok(n) => Ok(n),
             Err(Errno::EINTR) => {

And this example (timeout <> 0 + Behavior::PreferTerm):

use rustyline::{Config, DefaultEditor, Result};

fn main() -> Result<()> {
    env_logger::init();
    let config = Config::builder()
        .behavior(rustyline::Behavior::PreferTerm)
        .keyseq_timeout(Some(50))
        .build();
    let mut rl = DefaultEditor::with_config(config)?;

    let _ = rl.readline("> ")?;
    Ok(())
}
RUST_LOG=rustyline=debug cargo run --example issue 2> debug.log

gives

[DEBUG rustyline] poll with: PollTimeout(50)
[DEBUG rustyline] poll returns: Ok(1)

on MacOS as you say.
But we cannot rely on timeout.

No issue with Behavior::Stdio on MacOs or with Behavior::PreferTerm on Linux:

[DEBUG rustyline] poll with: PollTimeout(50)
[DEBUG rustyline] poll returns: Ok(0)
[DEBUG rustyline] c: '\u{1b}' => key: KeyEvent(Esc, Modifiers(0x0))
[DEBUG rustyline] Emacs command: Abort

@gwenn gwenn added the bug label Sep 8, 2024
gwenn added a commit to gwenn/rustyline that referenced this pull request Sep 8, 2024
@gwenn
Copy link
Collaborator

gwenn commented Sep 8, 2024

See #803

@gwenn gwenn added invalid and removed bug labels Sep 8, 2024
@gwenn
Copy link
Collaborator

gwenn commented Oct 27, 2024

Would you mind reviewing / testing #803 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for cancelling input with Escape
2 participants