Refactor unix (a)sync readers #9

zrzka · 2019-10-09T15:59:51Z

Changes

One background/reading thread for unlimited number of Sync & Async readers
- Reading & parsing is done in one place
- Readers are informed via std::sync::mpsc channels
TerminalInput::read_line functionality moved to the Input::read_line with default implementation
UnixInput::read_char reimplemented with SyncReader & waiting for the InputEvent::Keyboard(KeyEvent::Char(..)) event
Introduced InternalEvent
Introduced InputEvent::CursorPosition (UNIX only)
Fixed rxvt mouse event parsing
- Top/left is 0, 0
Split of the parse_event function to multiple/smaller ones
Basic set of tests for all the parse_event functions

Fixes crossterm-rs/crossterm#199
Fixes crossterm-rs/crossterm#271
Fixes crossterm-rs/crossterm#253

No longer valid

Keeping for history, comments when this PR was started.

It's a proof of concept for discussion! Nothing to merge, ...

It fixes my problems with (a)sync readers and Esc swallowing
You can play with it via cargo run --example foo (part of this PR)
It's ugly, still lot of room for improvement, but that's because I tried not to break API much, except
- AsyncReader::new
- InputEvent::Internal (see comments inside)

Signed-off-by: Robert Vojta <rvojta@me.com>

src/input/unix.rs

src/sys/unix.rs

examples/foo.rs

src/lib.rs

src/sys/unix.rs

zrzka · 2019-10-10T11:15:30Z

@TimonPost replied to some comments and ignored docstring comments. This is not something to merge right away, it's something to talk about if it's fine or not, etc. When we agree that it's a way to go for now, then I'll write docs, etc.

TimonPost · 2019-10-10T11:27:29Z

okay, I will play around it when I am at home.

zrzka · 2019-10-10T11:33:14Z

Okay. I already have something on my mind to improve it, but going offline today. Tooths out, I'll be back tomorrow.

Signed-off-by: Robert Vojta <rvojta@me.com>

TimonPost · 2019-10-11T15:02:11Z

maybe we can put the UNIX mouse encoding into its own trait and implement it for various encodings. Because the parse function is shared logic and can be unified into a single trait.

trait MouseEncoding {
    fn parse_csi(buffer: &[u8]) -> Result<Option<InternalEvent>>;
}

struct RXVTMouseEncoding;

impl MouseEncoding for RXVTMouseEncoding {
    // Buffer does NOT contain: ESC [
    fn parse_csi(buffer: &[u8]) -> Result<Option<InternalEvent>> {
        // ESC [ Cb ; Cx ; Cy ; M
        let s = std::str::from_utf8(&buffer[..buffer.len() - 1])
            .map_err(|_| could_not_parse_event_error())?;
        let mut split = s.split(';');

        let cb = next_parsed::<u16>(&mut split)?;
        let cx = next_parsed::<u16>(&mut split)?;
        let cy = next_parsed::<u16>(&mut split)?;

        let mouse_input_event = match cb {
            32 => MouseEvent::Press(MouseButton::Left, cx, cy),
            33 => MouseEvent::Press(MouseButton::Middle, cx, cy),
            34 => MouseEvent::Press(MouseButton::Right, cx, cy),
            35 => MouseEvent::Release(cx, cy),
            64 => MouseEvent::Hold(cx, cy),
            96 | 97 => MouseEvent::Press(MouseButton::WheelUp, cx, cy),
            _ => MouseEvent::Unknown,
        };

        Ok(Some(InternalEvent::Input(InputEvent::Mouse(
            mouse_input_event,
        ))))
    }
}

struct X10MouseEncoding;

impl MouseEncoding for X10MouseEncoding {
    // Buffer does NOT contain: ESC [ M
    fn parse_csi(buffer: &[u8]) -> Result<Option<InternalEvent>> {
        // X10 emulation mouse encoding: ESC [ M CB Cx Cy (6 characters only).
        // NOTE (@imdaveho): cannot find documentation on this

        if buffer.len() < 3 {
            return Ok(None);
        }

        let cb = buffer[1] as i8 - 32;
        // See http://www.xfree86.org/current/ctlseqs.html#Mouse%20Tracking
        // The upper left character position on the terminal is denoted as 1,1.
        // Subtract 1 to keep it synced with cursor
        let cx = buffer[2].saturating_sub(32) as u16 - 1;
        let cy = buffer[3].saturating_sub(32) as u16 - 1;

        let mouse_input_event = match cb & 0b11 {
            0 => {
                if cb & 0x40 != 0 {
                    MouseEvent::Press(MouseButton::WheelUp, cx, cy)
                } else {
                    MouseEvent::Press(MouseButton::Left, cx, cy)
                }
            }
            1 => {
                if cb & 0x40 != 0 {
                    MouseEvent::Press(MouseButton::WheelDown, cx, cy)
                } else {
                    MouseEvent::Press(MouseButton::Middle, cx, cy)
                }
            }
            2 => MouseEvent::Press(MouseButton::Right, cx, cy),
            3 => MouseEvent::Release(cx, cy),
            _ => MouseEvent::Unknown,
        };

        Ok(Some(InternalEvent::Input(InputEvent::Mouse(
            mouse_input_event,
        ))))
    }
}

struct XTermMouseEncoding;

impl MouseEncoding for XTermMouseEncoding {
    // Buffer does NOT contain: ESC [ <
    fn parse_csi(buffer: &[u8]) -> Result<Option<InternalEvent>> {
        // ESC [ < Cb ; Cx ; Cy (;) (M or m)
        if !buffer.ends_with(&[b'm']) && !buffer.ends_with(&[b'M']) {
            return Ok(None);
        }

        let s = std::str::from_utf8(&buffer[..buffer.len() - 1])
            .map_err(|_| could_not_parse_event_error())?;
        let mut split = s.split(';');

        let cb = next_parsed::<u16>(&mut split)?;

        // See http://www.xfree86.org/current/ctlseqs.html#Mouse%20Tracking
        // The upper left character position on the terminal is denoted as 1,1.
        // Subtract 1 to keep it synced with cursor
        let cx = next_parsed::<u16>(&mut split)? - 1;
        let cy = next_parsed::<u16>(&mut split)? - 1;

        let input_event = match cb {
            0..=2 | 64..=65 => {
                let button = match cb {
                    0 => MouseButton::Left,
                    1 => MouseButton::Middle,
                    2 => MouseButton::Right,
                    64 => MouseButton::WheelUp,
                    65 => MouseButton::WheelDown,
                    _ => unreachable!(),
                };
                match buffer.last().unwrap() {
                    b'M' => InputEvent::Mouse(MouseEvent::Press(button, cx, cy)),
                    b'm' => InputEvent::Mouse(MouseEvent::Release(cx, cy)),
                    _ => InputEvent::Unknown,
                }
            }
            32 => InputEvent::Mouse(MouseEvent::Hold(cx, cy)),
            3 => InputEvent::Mouse(MouseEvent::Release(cx, cy)),
            _ => InputEvent::Unknown,
        };

        Ok(Some(InternalEvent::Input(input_event)))
    }
}

examples/foo.rs

src/input/unix.rs

src/lib.rs

src/sys/unix.rs

TimonPost · 2019-10-11T15:42:41Z

I am not sure about the manual threading work. To fix the current bugs and problems this is okay with me. For a longer-term solution, we definitely need to do something else. I propose to go on with this design. Then we can come back to my proposed design which fixes some of those issues with a new API. This new API is going to be created once all crates are merged. Most of your code can be re-used like: the abstractions like TtyRaw event parsing, libc reading. But the channels and thread work will be replaced with something more robust.

related crossterm-rs/crossterm#257 (comment)

zrzka · 2019-10-13T19:58:19Z

I am not sure about the manual threading work.

Can you elaborate? I don't know what you mean with this. It's already there. See:

crossterm-input/src/input/unix_input.rs

Lines 115 to 117 in b594d9c

    
           thread::spawn(move || loop { 
        
               function(&event_tx, &thread_shutdown); 
        
           });

This PR changes it in a way that there's just one thread no matter how many AsyncReader instances you crate. Without this PR, there was one additional thread per AsyncReader instance.

zrzka · 2019-10-13T20:04:11Z

maybe we can put the UNIX mouse encoding into its own trait and implement it for various encodings. Because the parse function is shared logic and can be unified into a single trait.

trait MouseEncoding {
    fn parse_csi(buffer: &[u8]) -> Result<Option<InternalEvent>>;
}

Is it worth it? I mean now, before merge?
What's the benefit?
Why just mouse? Everything is kind of CSI parser like ...

trait ParseCsi {
    fn parse_csi(buffer: &[u8]) -> Result<Option<InternalEvent>>;
}

... my personal opinion is that we should rewrite parsing completely to make it maintainable and easily readable. And if we will do this, why do we need to change it in this way now? Am I missing something?

TimonPost · 2019-10-14T10:04:57Z

I mean, working with receivers, senders, mutexes, locks is just a pain. I understand we need it. So that's why I don't care. It is more the idea behind it, and we are going to fix it in future versions so I am okay with it to merge it now.

You are not missing something, yea we are going to do the rewrite, and if you think it is to much work, then just leave it like it is. It was just an idea to improve the code.

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka · 2019-10-14T14:11:43Z

Here's the examples/foo.rs, which is going to be removed from the PR.

// TEMPORARY IN THIS PR - IT WONT BE PART OF THE FINAL PR

#![allow(dead_code)]
use std::io::{stdout, Write};

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

//
// This is sample `crossterm_cursor::pos_raw` implementation we will have to use.
//
// Once the crates will be merged, `pos_raw` will gain access to `internal_event_receiver()`
// function and thus we can drop `InputEvent::CursorPosition` and use `InternalEvent::CursorPosition`
// to hide this implementation detail from the user.
//
fn pos_raw() -> Result<(u16, u16)> {
    let input = TerminalInput::new();
    let mut reader = input.read_sync();

    // Write command
    let mut stdout = stdout();
    stdout.write_all(b"\x1B[6n")?;
    stdout.flush()?;

    loop {
        if let Some(InputEvent::CursorPosition(x, y)) = reader.next() {
            return Ok((x, y));
        }
    }
}

fn async_test() -> 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::Char('c')) => println!("Cursor: {:?}", pos_raw()),
                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),
                        _ => {}
                    };
                }
                InputEvent::CursorPosition(_, _) => {}
                e => {
                    println!("Event: {:?}", e);
                }
            };
        }
        std::thread::sleep(std::time::Duration::from_millis(100));
        println!(".");
    }

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

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

    let mut reader = input.read_sync();
    loop {
        if let Some(event) = reader.next() {
            match event {
                InputEvent::Keyboard(KeyEvent::Esc) => break,
                InputEvent::Keyboard(KeyEvent::Char('c')) => println!("Cursor: {:?}", pos_raw()),
                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),
                        _ => {}
                    };
                }
                InputEvent::CursorPosition(_, _) => {}
                e => {
                    println!("Event: {:?}", e);
                }
            };
        }
        println!(".");
    }
    input.disable_mouse_mode()?;
    Ok(())
}

fn main() -> Result<()> {
    // async_test()
    sync_test()
}

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka · 2019-10-14T14:30:24Z

@TimonPost addressed all comments, final review please. Changes I made:

removed examples/foo.rs (it's in the previous comment)
changelog entry added
added some tests (not exhaustive, just to avoid simple mistakes)
all parse_ functions accepts full buffer, removed the dance with stripped ESC [, etc.
InternalEvent, etc. is #[cfg(unix)]ed for now, InputEvent::CursorPosition as well
refactored impl Iterator for AsyncReader & SyncReader to avoid self.rx.take() & self.rx = Some(rx)

What's way more important:

We have to test it on Linux/macOS, I mean, all parsers/events.
Did some testing on macOS, but didn't finish yet.
Would be nice if you can do some tests too to be sure that nothing was broken.

Once approved, squashed & merged, I'll update the crossterm_cursor to use SyncReader & InputEvent::CursorPosition.

zrzka · 2019-10-14T15:01:09Z

Just a note for me - check the rxvt mouse csi parser, if top left is 0, 0 or panic.

Reason: rxvt mouse parsing wasn't modified in #7 (others x10, xterm, ...) were. So I did add -1 in this PR to make it sync. But rather double check than introduce another bug.

TimonPost · 2019-10-14T16:56:50Z

Tested Mint Linux:

Sync Reader

Worked:

ran all tests
Character press
CTRL press
Mouse Event Press/Release/Move and correct position
Fn Keys
Page Up, Down, Home, Delete, Insert
(Back) Tab

Failed:

ALT + char, FN keys, special keys not working results in Event: Keyboard(Char('\u{1b}'))

Async Reader

Worked:

ran all tests
Character press
CTRL press
Mouse Event Press/Release/Move and correct position
Fn Keys
Page Up, Down, Home, Delete, Insert
(Back) Tab

Failed:

ALT + char, FN keys, special keys not working results in Event: Keyboard(Char('\u{1b}'))

zrzka · 2019-10-14T17:50:33Z

Thanks, will check what's wrong. What do you mean with Fn keys working & also not working?

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka · 2019-10-14T18:32:31Z

@TimonPost

ALT + char - fixed, see this commit
FN keys - works for me, booting Linux to check
special keys not working results in Event: Keyboard(Char('\u{1b}')) - I saw this when the ALT + char wasn't fixed

Can you be more specific in those two cases which doesn't work for you?

FN keys - they're listed in both working / non-working in your report, puzzled a bit,
Special keys - can you name which ones?

zrzka · 2019-10-14T19:22:14Z

Following works on Ubuntu & Terminal, clean install.

parse_event

Enter, Esc, Tab
Alt
Char

parse_csi

Left, Right, Up, Down

parse_csi_cursor_position

Reports position as expected

parse_csi_modifier_key_code

ShiftLeft, ShiftRight, ShiftUp, ShiftDown

parse_csi_special_key_code

Insert

parse_csi_rxvt_mouse

Works as expected in rxvt

parse_csi_xterm_mouse

Works as expected

@TimonPost can you try with the latest commit and if it doesn't work for you, can you say what terminal do you use & exact list of keys which don't work? Thanks.

TimonPost · 2019-10-15T08:00:51Z

What I meant with ALT + char, FN keys, special keys not working results in Event: Keyboard(Char('\u{1b}')) is that any combination with ALT does not work

TimonPost · 2019-10-15T08:25:19Z

All keys seem to work on linux mint now. So that commit fixed it.

imdaveho · 2019-10-16T08:41:32Z

I am not sure about the manual threading work.

Can you elaborate? I don't know what you mean with this. It's already there. See:

crossterm-input/src/input/unix_input.rs

Lines 115 to 117 in b594d9c

thread::spawn(move || loop {

function(&event_tx, &thread_shutdown);

});

This PR changes it in a way that there's just one thread no matter how many AsyncReader instances you crate. Without this PR, there was one additional thread per AsyncReader instance.

@zrzka Could you elaborate on the part where you're noticing one additional thread per AsyncReader? I was noticing the same thing when I was passing the reader to some functions to handle events, and after the first function ran, I noticed there were +1 threads when to the next function.

Do you know why?

On a separate note, I like how you're making it easier by having a single entity handle reading from the /dev/tty, but I would still like to know what you diagnosed as having this problem of the threads not cleaning up after being out of scope and dropped...

I guess I'm still playing catch up to a lot of changes that you and @TimonPost are making so I'm still trying to wrap my head around these issues. I've been trying to find a way to contribute, but things are moving quickly and the API is changing quick -- so it's been tough following the breadcrumbs or, I guess, it's more like the wake / burned rubber of you two moving so quickly 😅

pseudo-code:

fn main() {
    
    let _raw = RawScreen::into_raw_mode()?;
    let mut reader = input.read_async();
    input.enable_mouse_mode()?

    handle_something(); // <-- has 2 threads, 1 main and 1 for the AsyncReader
    handle_something_else(); // <-- has 3 threads, 1 main, and 2 others...
    // etc.. // and so on and so forth...
}

fn handle_something() {
    let input = TerminalInput::new();
    let mut reader = input.read_async()
    let result = thread::spawn(move || {
        loop {
            if let Some(evt) = reader.next() {
                 // handle events, and if it is xyz event, return something
                 // that is running concurrently and return (ending and dropping the reader)
            }
        }
    });
    
    // doing something concurrently...
}

fn handle_something_else() {
    // essentially the same as the above, but with different handling...
}

zrzka · 2019-10-16T09:24:58Z

@imdaveho

Could you elaborate on the part where you're noticing one additional thread per AsyncReader? I was noticing the same thing when I was passing the reader to some functions to handle events, and after the first function ran, I noticed there were +1 threads when to the next function.

The code in the master branch does it.

crossterm-input/src/input/unix.rs

Lines 161 to 175 in bb53032

    
           pub fn new(function: Box<dyn Fn(&Sender<u8>, &Arc<AtomicBool>) + Send>) -> AsyncReader { 
        
               let shutdown_handle = Arc::new(AtomicBool::new(false)); 
        
               let (event_tx, event_rx) = mpsc::channel(); 
        
               let thread_shutdown = shutdown_handle.clone(); 
        
               thread::spawn(move || loop { 
        
                   function(&event_tx, &thread_shutdown); 
        
               }); 
        
               AsyncReader { 
        
                   event_rx, 
        
                   shutdown: shutdown_handle, 
        
               } 
        
           }

Every AsyncReader instance aka .read_async() call spawns a new thread.

I would still like to know what you diagnosed as having this problem of the threads not cleaning up after being out of scope and dropped...

There're lot of problems with the master branch code design. Especially when combined with the cursor::pos() where all these threads are stealing /dev/tty (or stdin) input from each other and it leads to unknown/not parsable events. Not mentioning alive threads and other things.

Anyway, this PR (still working on it) just fixes all these issues and is kind of temporary work to keep the crossterm_input in a good shape. There will be major overhaul of the input processing when we merge all the crates into one crate.

Signed-off-by: Robert Vojta <rvojta@me.com>

imdaveho · 2019-10-16T13:23:05Z

@zrzka I get that section of code causing the issues. Just to play devil's advocate -- since this is a large breaking change -- why not ~~simply~~ add locks to the Readers of dev/tty?

(Edit: not really an option at this point since this PR is well on it's way. But might be an alternative work considering if we didn't want to add too many new dependencies and break the previous API. But again, this is just a consideration without implementation to see if adding locks would indeed solve the problem...)

Also, a bit of further playing around with the current threaded case for AsyncReader, could it be that this block is not terminating when cancellation_token == true due to .bytes() blocking for additional input until the for loop can check that cancellation_token == true?

crossterm-input/src/input/unix.rs

Lines 35 to 43 in bb53032

    
           for i in get_tty().unwrap().bytes() { 
        
               if event_tx.send(i.unwrap()).is_err() { 
        
                   return; 
        
               } 
        
               if cancellation_token.load(Ordering::SeqCst) { 
        
                   return; 
        
               } 
        
           }

Perhaps this is why the current implementation is not allowing the spawned thread to finish, and therefore, each time we call .read_async() this causes the thread count to increase...(or never terminating when explicitly calling thread.join())

Again, since I did the first pass on this module, largely taking the parse_event logic from Termion as a reference point to handle common ANSI sequences -- I'm curious about the previous implementation's shortcomings. As for WinCon support, I also want to see if there is any way to contribute to that -- so just trying to wrap my head around these changes. Thanks.

zrzka · 2019-10-16T13:41:00Z

@imdaveho

since this is a large breaking change

Can you elaborate? The only breaking change is that the AsyncReader::new has different signature. Yes, from the public API point of view, it's a breaking change. Also, it shouldn't be public anyway.

What else is a breaking change? I'm not aware of any. Yes, there's change in the internal behaviour where we have one reading thread producing InternalEvent/InputEvent instead of multiple threads doing the same thing.

why not simply add locks to the Readers of dev/tty?

One piece of code does use stdin() which locks self during read() for example. There's another piece of code which directly opens /dev/tty and reads from it. Adding a lock to lock everything to prevent races is not a good idea IMO.

I'm curious about the previous implementation's shortcomings

The design evolved into something which is not very useful. Why do we provide ability to instantiate unlimited amount of SyncReader/AsyncReaders? Why are we parsing events on every single spawned thread? Why don't we force the user to handle all the input events on one thread like desktop applications do? If user wants to do heavy computing, he can spawn his own thread or use any other way. There should be one place which fires all possible events, support for an event loop and that's all. Rest should be done by the user.

Again, this PR is not about a large breaking change, it's about fixing current bugs. All the input will be rewritten in the (near) future.

Signed-off-by: Robert Vojta <rvojta@me.com>

imdaveho · 2019-10-16T13:58:23Z

@zrzka I guess I might have been misunderstood on my comment on "large breaking changes". I meant that a significant shift from my mental model of the library. It is understandable that, to make the project more robust, these refactors need to be done.

I just wanted ask and poke to get clarification to help my understanding of how the project is evolving, and separate the aspects of the implementation that 1) are design optimizations for a easier-to-reason-about and reduce bugs and 2) broken implementations.

It seems from your comments that this is largely 1) which has led to several bugs and issues that you are currently resolving with this new approach.

Why do we provide ability to instantiate unlimited amount of SyncReader/AsyncReaders? Why are we parsing events on every single spawned thread? Why don't we force the user to handle all the input events on one thread like desktop applications do?

I'm assuming initially the reason for AsyncReader was a quick, straight-forward way to handle events in the background. Of course this implementation coupled the listening of events with the user application of events. I guess what you're trying to do is separate those two things.

Thanks for walking me through your thoughts on the redesign. Looks promising.

TimonPost · 2019-10-16T14:06:02Z

Related to discussion:

This PR is related to this issue: crossterm-rs/crossterm#265 (comment) . If you give phase 3 the last issue a look, you will find the tasks for this PR with related issues that this PR is solving.

Besides, it solves some problems spoken about in this issue: crossterm-rs/crossterm#257

zrzka · 2019-10-16T14:10:05Z

@imdaveho

I'm not sure we understand each other in this thread, based on your responses like ...

It seems from your comments that this is largely 1) which has led to several bugs and issues that you are currently resolving with this new approach.

&

Thanks for walking me through your thoughts on the redesign. Looks promising.

This PR just fixes couple of bugs. The reason is that all the subcrates (crossterm_) will be merged to the crossterm crate. But to do this, we have to keep them in a good shape before deprecation. You can read more here crossterm-rs/crossterm#265.

Once merged, merged crossterm crate released (no other changes), ... we can continue with the discussion about new design, redesign, make a large breaking changes, ...

In other words, this PR is not a new design. There're other issues to use for future discussion how it should look like.

imdaveho · 2019-10-16T15:11:08Z

@zrzka Cool -- we can discuss this later. I guess with a PR title starting with "Refactor" it's easy to confuse the design update vs strictly fixing bugs, but that's not really important. Also further discussion is off-topic at this point.

I had wanted to get a sense of your direction taking this approach to fix the bugs you mentioned. I think I have a good sense of that now.

TimonPost · 2019-10-18T08:56:26Z

Cargo.toml

@@ -22,4 +22,5 @@ libc = "0.2.51"
 crossterm_utils = { version = "0.3.1" }
 crossterm_screen = { version = "0.3.1" }
 lazy_static = "1.4"
+mio = "0.6.19"


minor version needed?

isn't mio a unix only dependency now?

By default, it's a caret requirements, which means:

^1.2.3 := >=1.2.3 <2.0.0 ^1.2 := >=1.2.0 <2.0.0

I did it with 0.6.19 and didn't test it with < 0.6.19. So, I'd rather keep it as it is otherwise we will have to test it again with previous versions.

CHANGELOG.md

TimonPost · 2019-10-18T09:00:11Z

Cargo.toml

@@ -22,4 +22,5 @@ libc = "0.2.51"
 crossterm_utils = { version = "0.3.1" }
 crossterm_screen = { version = "0.3.1" }
 lazy_static = "1.4"
+mio = "0.6.19"


isn't mio a unix only dependency now?

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka · 2019-10-18T09:15:50Z

CI is still running, but it already passed on stable. The only remaining job is nightly, which isn't mandatory. Going to merge.

zrzka requested a review from TimonPost as a code owner October 9, 2019 15:59

Refactor unix (a)sync readers

0efd4c1

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka force-pushed the zrzka/unix-async-events branch from 914bf99 to 0efd4c1 Compare October 9, 2019 16:07

TimonPost reviewed Oct 10, 2019

View reviewed changes

Slightly different approach

d1af8c8

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka force-pushed the zrzka/unix-async-events branch from f0ef81f to d1af8c8 Compare October 11, 2019 11:53

Make it more readable

a9fd45a

Signed-off-by: Robert Vojta <rvojta@me.com>

TimonPost reviewed Oct 11, 2019

View reviewed changes

zrzka added 5 commits October 14, 2019 13:13

Rename sentinel to stop_event

56d3bd5

Signed-off-by: Robert Vojta <rvojta@me.com>

Refactor (A)SyncReader next()

69428dc

Signed-off-by: Robert Vojta <rvojta@me.com>

Add cfg(unix) for cursor positions

3d0c3c9

Signed-off-by: Robert Vojta <rvojta@me.com>

Replace 27 with b'\x1B' to keep the style

2d98497

Signed-off-by: Robert Vojta <rvojta@me.com>

Parsing refactoring

391fb8a

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka added 3 commits October 14, 2019 16:14

Fix Windows build

f0415f5

Signed-off-by: Robert Vojta <rvojta@me.com>

Update changelog

582db72

Signed-off-by: Robert Vojta <rvojta@me.com>

Remove examples/foo.rs

899e2e2

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka changed the title ~~[WIP] Refactor unix (a)sync readers~~ Refactor unix (a)sync readers Oct 14, 2019

Fix Alt+key parser

73adfe6

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka mentioned this pull request Oct 14, 2019

Use SyncReader & InputEvent::CursorPosition for pos_raw() crossterm-rs/crossterm-cursor#10

Merged

1 task

zrzka added 2 commits October 16, 2019 15:19

Add mio dependency

65104f3

Signed-off-by: Robert Vojta <rvojta@me.com>

Refactor reading thread polling

a68f779

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka added 2 commits October 16, 2019 15:53

Update MouseEvent::Hold documentation

e9a603d

Signed-off-by: Robert Vojta <rvojta@me.com>

Add TODO

d15b815

Signed-off-by: Robert Vojta <rvojta@me.com>

zrzka mentioned this pull request Oct 17, 2019

Merge sub-crates to the crossterm crate crossterm-rs/crossterm#265

Closed

43 tasks

TimonPost reviewed Oct 18, 2019

View reviewed changes

zrzka added 2 commits October 18, 2019 11:03

Move mio to unix deps

1245d25

Signed-off-by: Robert Vojta <rvojta@me.com>

Update changelog

f893c28

Signed-off-by: Robert Vojta <rvojta@me.com>

TimonPost approved these changes Oct 18, 2019

View reviewed changes

zrzka merged commit 6d045a8 into master Oct 18, 2019

zrzka deleted the zrzka/unix-async-events branch October 18, 2019 09:41

TimonPost mentioned this pull request Oct 19, 2019

Release 0.12 crossterm-rs/crossterm#277

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor unix (a)sync readers #9

Refactor unix (a)sync readers #9

zrzka commented Oct 9, 2019 •

edited

Loading

zrzka commented Oct 10, 2019

TimonPost commented Oct 10, 2019

zrzka commented Oct 10, 2019

TimonPost commented Oct 11, 2019 •

edited

Loading

TimonPost commented Oct 11, 2019 •

edited

Loading

zrzka commented Oct 13, 2019

zrzka commented Oct 13, 2019

TimonPost commented Oct 14, 2019

zrzka commented Oct 14, 2019

zrzka commented Oct 14, 2019

zrzka commented Oct 14, 2019 •

edited

Loading

TimonPost commented Oct 14, 2019 •

edited

Loading

zrzka commented Oct 14, 2019

zrzka commented Oct 14, 2019

zrzka commented Oct 14, 2019

TimonPost commented Oct 15, 2019

TimonPost commented Oct 15, 2019

imdaveho commented Oct 16, 2019

zrzka commented Oct 16, 2019

imdaveho commented Oct 16, 2019 •

edited

Loading

zrzka commented Oct 16, 2019

imdaveho commented Oct 16, 2019

TimonPost commented Oct 16, 2019 •

edited

Loading

zrzka commented Oct 16, 2019

imdaveho commented Oct 16, 2019 •

edited

Loading

TimonPost Oct 18, 2019

TimonPost Oct 18, 2019

zrzka Oct 18, 2019 •

edited

Loading

TimonPost Oct 18, 2019

zrzka commented Oct 18, 2019

Refactor unix (a)sync readers #9

Refactor unix (a)sync readers #9

Conversation

zrzka commented Oct 9, 2019 • edited Loading

Changes

No longer valid

zrzka commented Oct 10, 2019

TimonPost commented Oct 10, 2019

zrzka commented Oct 10, 2019

TimonPost commented Oct 11, 2019 • edited Loading

TimonPost commented Oct 11, 2019 • edited Loading

zrzka commented Oct 13, 2019

zrzka commented Oct 13, 2019

TimonPost commented Oct 14, 2019

zrzka commented Oct 14, 2019

zrzka commented Oct 14, 2019

zrzka commented Oct 14, 2019 • edited Loading

TimonPost commented Oct 14, 2019 • edited Loading

Sync Reader

Async Reader

zrzka commented Oct 14, 2019

zrzka commented Oct 14, 2019

zrzka commented Oct 14, 2019

TimonPost commented Oct 15, 2019

TimonPost commented Oct 15, 2019

imdaveho commented Oct 16, 2019

zrzka commented Oct 16, 2019

imdaveho commented Oct 16, 2019 • edited Loading

zrzka commented Oct 16, 2019

imdaveho commented Oct 16, 2019

TimonPost commented Oct 16, 2019 • edited Loading

zrzka commented Oct 16, 2019

imdaveho commented Oct 16, 2019 • edited Loading

TimonPost Oct 18, 2019

Choose a reason for hiding this comment

TimonPost Oct 18, 2019

Choose a reason for hiding this comment

zrzka Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

TimonPost Oct 18, 2019

Choose a reason for hiding this comment

zrzka commented Oct 18, 2019

zrzka commented Oct 9, 2019 •

edited

Loading

TimonPost commented Oct 11, 2019 •

edited

Loading

TimonPost commented Oct 11, 2019 •

edited

Loading

zrzka commented Oct 14, 2019 •

edited

Loading

TimonPost commented Oct 14, 2019 •

edited

Loading

imdaveho commented Oct 16, 2019 •

edited

Loading

TimonPost commented Oct 16, 2019 •

edited

Loading

imdaveho commented Oct 16, 2019 •

edited

Loading

zrzka Oct 18, 2019 •

edited

Loading