AMythicDev / minus

An asynchronous, runtime data feedable terminal paging library for Rust
https://crates.io/crates/minus/
Apache License 2.0
317 stars 23 forks source link

`search` input is duplicated on Windows #140

Closed ErichDonGubler closed 3 months ago

ErichDonGubler commented 3 months ago

Describe the bug

Key input in search is duplicated by non-press events reported by crossterm (e.g., Release events). This only affects Windows, as far as I know, since it's the only platform that has KeyboardEnhancementFlags::REPORT_EVENT_TYPES always set. This feature was introduced with the consumption of crossterm 0.26.1, viz., 39224c3cba9c88c2822359b86279ba76e8c31f52, where key repeat and release events were added but not handled in minus code.

To Reproduce

  1. Enter search functionality with some input suitable for testing.
  2. Begin typing a search query, i.e., cat. Observe that letters are repeated, i.e., ccaatt, cacatt, etc.
I was able to reproduce this using this file layout in a new Cargo project: * `Cargo.toml`: ```toml [package] name = "tmp-xz0anm" version = "0.1.0" edition = "2021" [dependencies] lipsum = "0.9.1" minus = { version = "5.6.1", features = ["dynamic_output", "search"] } ``` * `src/main.rs`: ```rs use std::{fmt::Write, thread, time::Duration}; fn main() { let mut pager = minus::Pager::new(); let pager_thread = thread::spawn({ let pager = pager.clone(); move || minus::dynamic_paging(pager) }); for word in lipsum::lipsum(2_000).split_whitespace() { writeln!(pager, "{word}").unwrap(); thread::sleep(Duration::from_millis(10)); } pager_thread.join().unwrap().unwrap(); } ```

Expected behavior

Screenshots

I don't think this is necessary, but can provide screenshots on request.

Environment:

Additional context

Noticed as the root cause of https://github.com/martinvonz/jj/issues/3448.

crossterm upstream issue: https://github.com/crossterm-rs/crossterm/issues/797

crossterm upstream is planning on making the KeyboardEnhancementFlags::REPORT_EVENT_TYPES feature opt-in with https://github.com/crossterm-rs/crossterm/pull/778. However, this change still seems desirable, since it's more robust and correct overall.