diff options
author | Piotr Kufel <qfel@users.noreply.github.com> | 2024-08-09 18:07:50 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-08-09 18:07:50 -0700 |
commit | 983014cc40e6a96548c731b7de26e3a1a70d6426 (patch) | |
tree | 604139d6b8d8d3bba29d956c788b93beac4db1e8 | |
parent | 4ff33933dd7701240c37e4eee24929ebc8dccd92 (diff) |
Clean up key event handling (#13574)
# Description
Cleanups:
- Add "key_press" to event reading function names to match reality
- Move the relevant comment about why only key press events are
interesting one layer up
- Remove code duplication in handle_events
- Make `try_next` try harder (instead of bail on a boring event); I
think that was the original intention
- Remove recursion from `next` (I think that's clearer? but maybe just
what I'm used to)
# User-Facing Changes
None
# Tests + Formatting
This cleans up existing code, no new test coverage.
-rw-r--r-- | crates/nu-explore/src/pager/events.rs | 51 | ||||
-rw-r--r-- | crates/nu-explore/src/pager/mod.rs | 47 |
2 files changed, 49 insertions, 49 deletions
diff --git a/crates/nu-explore/src/pager/events.rs b/crates/nu-explore/src/pager/events.rs index b408b10f1..6023ddd8e 100644 --- a/crates/nu-explore/src/pager/events.rs +++ b/crates/nu-explore/src/pager/events.rs @@ -32,38 +32,35 @@ impl UIEvents { } } - pub fn next(&self) -> Result<Option<KeyEvent>> { - let now = Instant::now(); - match poll(self.tick_rate) { - Ok(true) => { - if let Event::Key(event) = read()? { - // We are only interested in Pressed events; - // It's crucial because there are cases where terminal MIGHT produce false events; - // 2 events 1 for release 1 for press. - // Want to react only on 1 of them so we do. - if event.kind == KeyEventKind::Press { - return Ok(Some(event)); - } + /// Read the next key press event, dropping any other preceding events. Returns None if no + /// relevant event is found within the configured tick_rate. + pub fn next_key_press(&self) -> Result<Option<KeyEvent>> { + let deadline = Instant::now() + self.tick_rate; + loop { + let timeout = deadline.saturating_duration_since(Instant::now()); + if !poll(timeout)? { + return Ok(None); + } + if let Event::Key(event) = read()? { + if event.kind == KeyEventKind::Press { + return Ok(Some(event)); } - - let time_spent = now.elapsed(); - let rest = self.tick_rate.saturating_sub(time_spent); - - Self { tick_rate: rest }.next() } - Ok(false) => Ok(None), - Err(err) => Err(err), } } - pub fn try_next(&self) -> Result<Option<KeyEvent>> { - match poll(Duration::default()) { - Ok(true) => match read()? { - Event::Key(event) if event.kind == KeyEventKind::Press => Ok(Some(event)), - _ => Ok(None), - }, - Ok(false) => Ok(None), - Err(err) => Err(err), + /// Read the next key press event, dropping any other preceding events. If no key event is + /// available, returns immediately. + pub fn try_next_key_press(&self) -> Result<Option<KeyEvent>> { + loop { + if !poll(Duration::ZERO)? { + return Ok(None); + } + if let Event::Key(event) = read()? { + if event.kind == KeyEventKind::Press { + return Ok(Some(event)); + } + } } } } diff --git a/crates/nu-explore/src/pager/mod.rs b/crates/nu-explore/src/pager/mod.rs index 4a043476b..5ea560093 100644 --- a/crates/nu-explore/src/pager/mod.rs +++ b/crates/nu-explore/src/pager/mod.rs @@ -191,6 +191,12 @@ fn render_ui( })?; } + // Note that this will return within the configured tick_rate of events. In particular this + // means this loop keeps redrawing the UI about 4 times a second, whether it needs to or + // not. That's OK-ish because ratatui will detect real changes and not send unnecessary + // output to the terminal (something that may especially be important with ssh). While not + // needed at the moment, the idea is that this behavior allows for some degree of + // animation (so that the UI can update over time, even without user input). let transition = handle_events( engine_state, stack, @@ -612,33 +618,25 @@ fn handle_events<V: View>( command: &mut CommandBuf, mut view: Option<&mut V>, ) -> Option<Transition> { - let key = match events.next() { + // We are only interested in Pressed events; + // It's crucial because there are cases where terminal MIGHT produce false events; + // 2 events 1 for release 1 for press. + // Want to react only on 1 of them so we do. + let mut key = match events.next_key_press() { Ok(Some(key)) => key, - _ => return None, + Ok(None) => return None, + Err(e) => { + log::error!("Failed to read key event: {e}"); + return None; + } }; - let result = handle_event( - engine_state, - stack, - layout, - info, - search, - command, - view.as_deref_mut(), - key, - ); - - if result.is_some() { - return result; - } - // Sometimes we get a BIG list of events; // for example when someone scrolls via a mouse either UP or DOWN. // This MIGHT cause freezes as we have a 400 delay for a next command read. // // To eliminate that we are trying to read all possible commands which we should act upon. - - while let Ok(Some(key)) = events.try_next() { + loop { let result = handle_event( engine_state, stack, @@ -649,13 +647,18 @@ fn handle_events<V: View>( view.as_deref_mut(), key, ); - if result.is_some() { return result; } + match events.try_next_key_press() { + Ok(Some(next_key)) => key = next_key, + Ok(None) => return None, + Err(e) => { + log::error!("Failed to peek key event: {e}"); + return None; + } + } } - - result } #[allow(clippy::too_many_arguments)] |