Add key bindings in TUI mode #206
Conversation
| @@ -90,7 +92,7 @@ pub async fn run_tui( | |||
| .ok_or_else(|| miette!("No more crossterm events"))? | |||
| .into_diagnostic() | |||
| .wrap_err("Failed to get next crossterm event")?; | |||
| handle_event(&mut tui, event)?; | |||
| handle_event(&mut tui, event, terminal.get_frame())?; | |||
There was a problem hiding this comment.
I remember seeing in flamegraphs for my text editor that get_frame and equivalents were expensive to call.
It would be more efficient if you deferred calling get_frame until you absolutely need it (in this case, only for frame-dependent movements like Ctrl-u and Ctrl-d).
You could achieve this by putting the TerminalGuard inside the Tui, which you already have an exclusive reference to in handle_event.
In practice, I don't think we need to worry about performance issues like this at the moment, but if it's easy to do (my mental borrow checker isn't very good) it's worth considering anyways.
There was a problem hiding this comment.
I actually have another change to implement some of this! I'll merge this as-is and then fix it up.
| @@ -90,7 +92,7 @@ pub async fn run_tui( | |||
| .ok_or_else(|| miette!("No more crossterm events"))? | |||
| .into_diagnostic() | |||
| .wrap_err("Failed to get next crossterm event")?; | |||
| handle_event(&mut tui, event)?; | |||
| handle_event(&mut tui, event, terminal.get_frame())?; | |||
There was a problem hiding this comment.
You should pass the Rect representing the size of the Frame, rather than the entire Frame (which has explicit lifetimes, access to the terminal Buffer, etc.).
| handle_event(&mut tui, event, terminal.get_frame())?; | |
| handle_event(&mut tui, event, terminal.get_frame().size())?; |
| Event::Key(key) => match key.modifiers { | ||
| KeyModifiers::NONE => match key.code { | ||
| KeyCode::Char('j') => { | ||
| tui.scroll_offset += 1; | ||
| } | ||
| KeyCode::Char('k') => { | ||
| tui.scroll_offset = tui.scroll_offset.saturating_sub(1); | ||
| } | ||
| KeyCode::Char('g') => { | ||
| tui.scroll_offset = 0; | ||
| } | ||
| _ => {} | ||
| }, | ||
|
|
||
| #[allow(clippy::single_match)] | ||
| KeyModifiers::SHIFT => match key.code { | ||
| KeyCode::Char('G') => { | ||
| tui.scroll_offset = last_line; | ||
| } | ||
| _ => {} | ||
| }, |
There was a problem hiding this comment.
You might be interested in the declarative macros I wrote for my text editor to match on crossterm's Keys more concisely:
- Usage example
match event { Event::Key(key) if k!(key, Left) || k!(key, CONTROL, 'b') => { command_mode.move_backward(1); } Event::Key(key) if k!(key, Right) || k!(key, CONTROL, 'f') => { command_mode.move_forward(1); } Event::Key(key) if k!(key, Left) || k!(key, ALT, 'b') => { command_mode.move_backward_word(); } // ...
- Implementation
There was a problem hiding this comment.
Hmmm I'll add a TODO for this
9999years
force-pushed
the
rebeccat/tui-key-bindings
branch
from
198d9b0 to
8fdeece
Compare
Adds a bunch of Vim/
less-like key bindings I wanted when I was testing the original TUI PR. These will probably be removed as soon as we do anything interesting with the UI, but they're nice for now.