From 9cb1128aaab8f5c19ec37b94ac1a86a91915827f Mon Sep 17 00:00:00 2001 From: YizhePKU Date: Wed, 23 Oct 2024 00:40:21 +0800 Subject: [PATCH] Reduce typing latency caused by `POLL_WAIT` (#846) * Rework `read_line_helper()` * Add a timeout when external printers are enabled * Tiny rework for `ReedlineRawEvent` Technically not part of the PR, but let me smuggle this in real quick. * Fix format --- src/edit_mode/emacs.rs | 12 ++-- src/edit_mode/vi/mod.rs | 16 ++--- src/engine.rs | 144 ++++++++++++++++++++-------------------- src/enums.rs | 45 ++++++------- 4 files changed, 108 insertions(+), 109 deletions(-) diff --git a/src/edit_mode/emacs.rs b/src/edit_mode/emacs.rs index 773618fe..994120b2 100644 --- a/src/edit_mode/emacs.rs +++ b/src/edit_mode/emacs.rs @@ -194,7 +194,7 @@ mod test { #[test] fn ctrl_l_leads_to_clear_screen_event() { let mut emacs = Emacs::default(); - let ctrl_l = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( + let ctrl_l = ReedlineRawEvent::try_from(Event::Key(KeyEvent::new( KeyCode::Char('l'), KeyModifiers::CONTROL, ))) @@ -214,7 +214,7 @@ mod test { ); let mut emacs = Emacs::new(keybindings); - let ctrl_l = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( + let ctrl_l = ReedlineRawEvent::try_from(Event::Key(KeyEvent::new( KeyCode::Char('l'), KeyModifiers::CONTROL, ))) @@ -227,7 +227,7 @@ mod test { #[test] fn inserting_character_works() { let mut emacs = Emacs::default(); - let l = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( + let l = ReedlineRawEvent::try_from(Event::Key(KeyEvent::new( KeyCode::Char('l'), KeyModifiers::NONE, ))) @@ -244,7 +244,7 @@ mod test { fn inserting_capital_character_works() { let mut emacs = Emacs::default(); - let uppercase_l = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( + let uppercase_l = ReedlineRawEvent::try_from(Event::Key(KeyEvent::new( KeyCode::Char('l'), KeyModifiers::SHIFT, ))) @@ -262,7 +262,7 @@ mod test { let keybindings = Keybindings::default(); let mut emacs = Emacs::new(keybindings); - let ctrl_l = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( + let ctrl_l = ReedlineRawEvent::try_from(Event::Key(KeyEvent::new( KeyCode::Char('l'), KeyModifiers::CONTROL, ))) @@ -276,7 +276,7 @@ mod test { fn inserting_capital_character_for_non_ascii_remains_as_is() { let mut emacs = Emacs::default(); - let uppercase_l = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( + let uppercase_l = ReedlineRawEvent::try_from(Event::Key(KeyEvent::new( KeyCode::Char('😀'), KeyModifiers::SHIFT, ))) diff --git a/src/edit_mode/vi/mod.rs b/src/edit_mode/vi/mod.rs index dc3fcb0a..81fa5b04 100644 --- a/src/edit_mode/vi/mod.rs +++ b/src/edit_mode/vi/mod.rs @@ -185,11 +185,9 @@ mod test { #[test] fn esc_leads_to_normal_mode_test() { let mut vi = Vi::default(); - let esc = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( - KeyCode::Esc, - KeyModifiers::NONE, - ))) - .unwrap(); + let esc = + ReedlineRawEvent::try_from(Event::Key(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE))) + .unwrap(); let result = vi.parse_event(esc); assert_eq!( @@ -215,7 +213,7 @@ mod test { ..Default::default() }; - let esc = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( + let esc = ReedlineRawEvent::try_from(Event::Key(KeyEvent::new( KeyCode::Char('e'), KeyModifiers::NONE, ))) @@ -241,7 +239,7 @@ mod test { ..Default::default() }; - let esc = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( + let esc = ReedlineRawEvent::try_from(Event::Key(KeyEvent::new( KeyCode::Char('$'), KeyModifiers::SHIFT, ))) @@ -267,7 +265,7 @@ mod test { ..Default::default() }; - let esc = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( + let esc = ReedlineRawEvent::try_from(Event::Key(KeyEvent::new( KeyCode::Char('$'), KeyModifiers::SUPER, ))) @@ -287,7 +285,7 @@ mod test { ..Default::default() }; - let esc = ReedlineRawEvent::convert_from(Event::Key(KeyEvent::new( + let esc = ReedlineRawEvent::try_from(Event::Key(KeyEvent::new( KeyCode::Char('q'), KeyModifiers::NONE, ))) diff --git a/src/engine.rs b/src/engine.rs index 21595214..7c78a923 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -51,13 +51,17 @@ use { // a POLL_WAIT of zero means that every single event is treated as soon as it // arrives. This doesn't allow for the possibility of more than 1 event // happening at the same time. -const POLL_WAIT: u64 = 10; -// Since a paste event is multiple Event::Key events happening at the same time, we specify -// how many events should be in the crossterm_events vector before it is considered -// a paste. 10 events in 10 milliseconds is conservative enough (unlikely somebody -// will type more than 10 characters in 10 milliseconds) +const POLL_WAIT: Duration = Duration::from_millis(100); +// Since a paste event is multiple `Event::Key` events happening at the same +// time, we specify how many events should be in the `crossterm_events` vector +// before it is considered a paste. 10 events is conservative enough. const EVENTS_THRESHOLD: usize = 10; +/// Maximum time Reedline will block on input before yielding control to +/// external printers. +#[cfg(feature = "external_printer")] +const EXTERNAL_PRINTER_WAIT: Duration = Duration::from_millis(100); + /// Determines if inputs should be used to extend the regular line buffer, /// traverse the history in the standard prompt or edit the search string in the /// reverse search @@ -695,12 +699,7 @@ impl Reedline { self.repaint(prompt)?; - let mut crossterm_events: Vec = vec![]; - let mut reedline_events: Vec = vec![]; - loop { - let mut paste_enter_state = false; - #[cfg(feature = "external_printer")] if let Some(ref external_printer) = self.external_printer { // get messages from printer as crlf separated "lines" @@ -716,76 +715,81 @@ impl Reedline { } } - let mut latest_resize = None; - loop { - // There could be multiple events queued up! - // pasting text, resizes, blocking this thread (e.g. during debugging) - // We should be able to handle all of them as quickly as possible without causing unnecessary output steps. - if !event::poll(Duration::from_millis(POLL_WAIT))? { - break; + // Helper function that returns true if the input is complete and + // can be sent to the hosting application. + fn completed(events: &[Event]) -> bool { + if let Some(event) = events.last() { + matches!( + event, + Event::Key(KeyEvent { + code: KeyCode::Enter, + modifiers: KeyModifiers::NONE, + .. + }) + ) + } else { + false } + } - match event::read()? { - Event::Resize(x, y) => { - latest_resize = Some((x, y)); - } - enter @ Event::Key(KeyEvent { - code: KeyCode::Enter, - modifiers: KeyModifiers::NONE, - .. - }) => { - let enter = ReedlineRawEvent::convert_from(enter); - if let Some(enter) = enter { - crossterm_events.push(enter); - // Break early to check if the input is complete and - // can be send to the hosting application. If - // multiple complete entries are submitted, events - // are still in the crossterm queue for us to - // process. - paste_enter_state = crossterm_events.len() > EVENTS_THRESHOLD; - break; - } - } - x => { - let raw_event = ReedlineRawEvent::convert_from(x); - if let Some(evt) = raw_event { - crossterm_events.push(evt); - } - } - } + let mut events: Vec = vec![]; + + // If the `external_printer` feature is enabled, we need to + // periodically yield so that external printers get a chance to + // print. Otherwise, we can just block until we receive an event. + #[cfg(feature = "external_printer")] + if event::poll(EXTERNAL_PRINTER_WAIT)? { + events.push(crossterm::event::read()?); } + #[cfg(not(feature = "external_printer"))] + events.push(crossterm::event::read()?); - if let Some((x, y)) = latest_resize { - reedline_events.push(ReedlineEvent::Resize(x, y)); + // Receive all events in the queue without blocking. Will stop when + // a line of input is completed. + while !completed(&events) && event::poll(Duration::from_millis(0))? { + events.push(crossterm::event::read()?); } - // Accelerate pasted text by fusing `EditCommand`s - // - // (Text should only be `EditCommand::InsertChar`s) - let mut last_edit_commands = None; - for event in crossterm_events.drain(..) { - match (&mut last_edit_commands, self.edit_mode.parse_event(event)) { - (None, ReedlineEvent::Edit(ec)) => { - last_edit_commands = Some(ec); - } - (None, other_event) => { - reedline_events.push(other_event); - } - (Some(ref mut last_ecs), ReedlineEvent::Edit(ec)) => { - last_ecs.extend(ec); - } - (ref mut a @ Some(_), other_event) => { - reedline_events.push(ReedlineEvent::Edit(a.take().unwrap())); + // If we believe there's text pasting or resizing going on, batch + // more events at the cost of a slight delay. + if events.len() > EVENTS_THRESHOLD + || events.iter().any(|e| matches!(e, Event::Resize(_, _))) + { + while !completed(&events) && event::poll(POLL_WAIT)? { + events.push(crossterm::event::read()?); + } + } - reedline_events.push(other_event); + // Convert `Event` into `ReedlineEvent`. Also, fuse consecutive + // `ReedlineEvent::EditCommand` into one. Also, if there're multiple + // `ReedlineEvent::Resize`, only keep the last one. + let mut reedline_events: Vec = vec![]; + let mut edits = vec![]; + let mut resize = None; + for event in events { + if let Ok(event) = ReedlineRawEvent::try_from(event) { + match self.edit_mode.parse_event(event) { + ReedlineEvent::Edit(edit) => edits.extend(edit), + ReedlineEvent::Resize(x, y) => resize = Some((x, y)), + event => { + if !edits.is_empty() { + reedline_events + .push(ReedlineEvent::Edit(std::mem::take(&mut edits))); + } + reedline_events.push(event); + } } } } - if let Some(ec) = last_edit_commands { - reedline_events.push(ReedlineEvent::Edit(ec)); + if !edits.is_empty() { + reedline_events.push(ReedlineEvent::Edit(edits)); + } + if let Some((x, y)) = resize { + reedline_events.push(ReedlineEvent::Resize(x, y)); } - for event in reedline_events.drain(..) { + // Handle reedline events. + for event in reedline_events { match self.handle_event(prompt, event)? { EventStatus::Exits(signal) => { // Check if we are merely suspended (to process an ExecuteHostCommand event) @@ -798,9 +802,7 @@ impl Reedline { return Ok(signal); } EventStatus::Handled => { - if !paste_enter_state { - self.repaint(prompt)?; - } + self.repaint(prompt)?; } EventStatus::Inapplicable => { // Nothing changed, no need to repaint diff --git a/src/enums.rs b/src/enums.rs index b56896be..cde42b1a 100644 --- a/src/enums.rs +++ b/src/enums.rs @@ -697,41 +697,40 @@ pub(crate) enum EventStatus { Exits(Signal), } -/// A simple wrapper for [crossterm::event::Event] +/// A wrapper for [crossterm::event::Event]. /// -/// Which will make sure that the given event doesn't contain [KeyEventKind::Release] -/// and convert from [KeyEventKind::Repeat] to [KeyEventKind::Press] -pub struct ReedlineRawEvent { - inner: Event, -} +/// It ensures that the given event doesn't contain [KeyEventKind::Release] +/// (which is rejected) or [KeyEventKind::Repeat] (which is converted to +/// [KeyEventKind::Press]). +pub struct ReedlineRawEvent(Event); + +impl TryFrom for ReedlineRawEvent { + type Error = (); -impl ReedlineRawEvent { - /// It will return None if `evt` is released Key. - pub fn convert_from(evt: Event) -> Option { - match evt { + fn try_from(event: Event) -> Result { + match event { Event::Key(KeyEvent { kind: KeyEventKind::Release, .. - }) => None, + }) => Err(()), Event::Key(KeyEvent { code, modifiers, kind: KeyEventKind::Repeat, state, - }) => Some(Self { - inner: Event::Key(KeyEvent { - code, - modifiers, - kind: KeyEventKind::Press, - state, - }), - }), - other => Some(Self { inner: other }), + }) => Ok(Self(Event::Key(KeyEvent { + code, + modifiers, + kind: KeyEventKind::Press, + state, + }))), + other => Ok(Self(other)), } } +} - /// Consume and get crossterm event object. - pub fn into(self) -> Event { - self.inner +impl From for Event { + fn from(event: ReedlineRawEvent) -> Self { + event.0 } }