Async reader doesn't produce MouseEvents

[zrzka]

• Platform: macOS
• It works when I replace input.read_async(); with input.read_sync();
• Run, click with the mouse and ...

examples/foo.rs:

use crossterm_input::{InputEvent, KeyEvent, MouseEvent, RawScreen, Result, TerminalInput};

fn main() -> Result<()> {
    let input = TerminalInput::new();
    let _raw = RawScreen::into_raw_mode()?;

    let mut reader = input.read_async();
    input.enable_mouse_mode()?;

    loop {
        if let Some(event) = reader.next() {
            match event {
                InputEvent::Keyboard(KeyEvent::Esc) => break,
                InputEvent::Keyboard(KeyEvent::Enter) => println!("Enter"),
                InputEvent::Mouse(mouse) => {
                    match mouse {
                        MouseEvent::Press(_, x, y) => println!("Press: {}x{}", x, y),
                        MouseEvent::Hold(x, y) => println!("Move: {}x{}", x, y),
                        MouseEvent::Release(x, y) => println!("Release: {}x{}", x, y),
                        _ => {}
                    };
                }
                _ => {}
            };
        }
    }

    input.disable_mouse_mode()?;
    Ok(())
}

[TimonPost]

It looks like it doesn't enable raw mode.... Because it print's the ANSI code to the screen.

[TimonPost]

I am testing on linux now

[TimonPost]

I can validate the same for Linux, not windows. This has to be due to some changes made in the latest PR's. I am sure this worked before. It looks like it breaks out of the loop when Escape his hit. Esc is returned in a fiew scenario's. We should checkout why it returns escape while we press any mouse button.

[zrzka]

I don't think that we did this in any of the last PRs. It looks like a duplicate of #199 (or is somehow related aka swallowed Esc).

[TimonPost]

I might have a clue why this is caused.

[zrzka]

Mind to share the clue? Also it enables the raw mode ... When I add this line ...

                InputEvent::Keyboard(KeyEvent::Char(c)) => println!("Char: {}", c),

... the output looks like ...

... see that asdf wasn't echoed, it's my println!.

Another weird think is that I've got panic in 30%. Run, hit Enter two times, click with mouse button and boom. Have to enable backtrace output and try to simulate this again.

[zrzka]

And it isn't deterministic at all ...

[zrzka]

Also this one is interesting, position reported via KeyEvent::Char.

[zrzka]

This line ...

https://github.com/crossterm-rs/crossterm-input/blob/34d8e783a8a9b444eb6f3df8a122e06adf4011db/src/input/unix.rs#L348

... returns None and then this one ...

https://github.com/crossterm-rs/crossterm-input/blob/34d8e783a8a9b444eb6f3df8a122e06adf4011db/src/input/unix.rs#L369

... returns KeyEvent::Esc. Not really sure that the KeyEvent::Esc is a good strategy here, because Esc is used to quit/cancel/close/...

[TimonPost]

It looks like this should be KeyEvent::UnKnown, can you validate whether the code returns the Escape on that line?

[zrzka]

Also the rare (30%) panic happens here ...

https://github.com/crossterm-rs/crossterm-input/blob/master/src/input/unix.rs#L452

[zrzka]

I'll check which line returns the right Esc, not sure even about Unknown here. First, I have to wrap my head around how the whole async & parsing is done.

[TimonPost]

With read async, we loop trough the bytes from TTY. We sent them over a channel, then we read the bytes one by one here, and when there is a byte on the channel we pass the iterator into the parse_event function so that it can read the next bytes. My expectation is that we are reading so fast that, it might occur that there is only one byte (char) on the iterator - which is in the case of ANSI codes '\x1B' (ESCAPE) - and thus we return that.

• We could change try_iter to iter which would make sure we wait before directly returning after 1 byte. Although I don't think that is going to work out because an escape could be on its own or followed by more characters.
• Instead of iterating very fast we could add a delay of a view MS, this should give the terminal more time to write the full ANSI input code to the TTY.

The SyncReader solves this by reading 2 bytes, then it checks if it contains only the escape key or continues parsing.

[TimonPost]

I am currently researching how we can do input reading better. I noticed that it is very difficult to do well and that there are very few examples whom have a similar implementation like we have. The current input system for Linux is mostly copied from termion (sometimes you have to steal like an artist), but I like to design something my self now because it has some limitations.

I looked at how the new windows terminal did this, and how other libraries were doing this. I might have an nice idea, but I am not sure how to implement that. I designed something in UML and tried to realize a bit of it. But I came to a difficult part where I need to know the whole ANSI input code before I want to do the parsing. How we do it now is that we parse the ANSI code as we iterate. Because the ANSI codes defer in lenght we can't just read into a fixed-size buffer. But we have to assume when we see certain ANSI codes we can read other bytes, which is not wrong, but things would become way easier if we could read the whole ANSI code directly, without having to iterate everything manually.

But that is something not related to this issue. Just something I liked to share

[TimonPost]

The idea of try_recv for async reader is that the user eighter receives None or Some since it is async it shouldn't block, and thus we can't use recv. Nor we can use File::read because that would block as well.

[zrzka]

I read the whole code and now I see where the problem is. I'll reimplement the internals, already have an idea how to do it without breaking changes.

[TimonPost]

Good, do you want to share how you are going about this?

[zrzka]

• I do not want to break API much, so, can't change everything, the goal is to fix it
• We're sending bytes over channel and then doing magic with them, but I think we should send InputEvent and do the magic on the reading thread
• The background thread won't use File & Bytes, but it will use libc::read & libc::select, the libc::select can be used to check if there's anything new or not and then you can easily decide between the Escape key and possible Escape sequences
• Would like to remove these unwrap() calls
• Would like to replace AtomicBool with one shot channel to inform the thread to quit
• This will also mean that I'll have to break API in AsyncReader::new, but that's fine, because it shouldn't be public. Already marked with TODO in the master branch. This change will break the API if we're looking at it strictly, but it won't break normal user code like input.read_async().next().

This is rough idea, my observations, ... and not saying the final one. My plan is to work on this tomorrow.