feat: add ratatui::run() method #1707
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1707 +/- ##
=======================================
- Coverage 93.0% 92.9% -0.1%
=======================================
Files 80 80
Lines 17625 17634 +9
=======================================
Hits 16395 16395
- Misses 1230 1239 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ratatui/src/init.rs
Outdated
There was a problem hiding this comment.
This is the main change to review.
This introduces a new `ratatui::run()` method which runs a closure with
a terminal initialized with reasonable defaults for most applications.
This calls `ratatui::init()` before running the closure and
`ratatui::restore()` after the closure completes, and returns the result
of the closure.
A minimal hello world example using the new `ratatui::run()` method:
```rust
use crossterm::event;
fn main() -> Result<(), Box<dyn std::error::Error>> {
ratatui::run(|terminal| loop {
terminal.draw(|frame| frame.render_widget("Hello, world!", frame.area()))?;
if matches!(event::read()?, event::Event::Key(_)) {
break Ok(());
}
})
}
```
Also simplify a few of the examples
|
I like it! Thanks for making it easy to review by splitting the commits + pointing us to the exact diff. You make this too easy :) Would it be useful to get some user feedback on this? I'm assuming we are going to be advocating for this to be the new way users are introduced to ratatui. |
Yup. Mentioned it in discord. This started named with_terminal, but I think run works much better. |
|
Perfect, but why take a mutable reference and not give ownership? |
There was a problem hiding this comment.
LGTM.
I'm okay with this being crossterm specific, like the other convenience functions such as init etc.
For the naming, maybe we can do something like run_with_terminal but I mean... what else are we going to run it with? Oh! Maybe run_with_web in ratzilla in the future. I think run should be good for now.
P.S. 0.30.0 release is going to be packed 🫠
|
I don't see an advantage of giving a mutable reference over ownership. I think it makes more sense to give ownership to give the user more control over the terminal. |
Choosing to move the terminal rather than pass a mutable reference here cedes control over when the terminal value is dropped to the closure. Passing it as a mutable reference means that it will be dropped after the closure completes, just before the terminal is restored. Also, none of the methods on Terminal require ownership, so there's no real upside to doing so that I can see. Some questions to think about to help clarify this:
|
|
From discord:
|
|
Would it make sense to have |
Doing the right thing in a terminal on panics, errors, and normal flow is surprisingly difficult to get right, and you end up choosing an option that's not necessarily ideal, but good enough. The major problem with this idea is that the panic hooks which display the error information run before the drop methods. In general apps that crash should disable raw mode and leave the alternate screen before displaying the panic messages. The second (less impactful) problem is that Third, the code to actually do the restore can in some circumstances itself crash, so we want that code to be intentionally part of the app's flow rather than part of the drop scope handling. Otherwise the error messages are a pain. The metric for validating how well |
|
I personally store the terminal in a separate struct in my still in development xkcd viewer. It being a mutable reference wouldn't imply much except me adding a few &muts and 'as. It would be a minor annoyance but a minor annoyance is still worse than no annoyance |
| let frame_timeout = Duration::from_secs_f64(1.0 / 60.0); // run at 60 FPS | ||
| ratatui::run(|terminal| loop { | ||
| terminal.draw(render)?; | ||
| if event::poll(frame_timeout)? && matches!(event::read()?, Event::Key(_)) { |
There was a problem hiding this comment.
| if event::poll(frame_timeout)? && matches!(event::read()?, Event::Key(_)) { | |
| if event::poll(frame_timeout)? && matches!(event::read()?, Event::Key(x) if x.kind == KeyEventKind::Press ) { |
There was a problem hiding this comment.
For the most part, because this immediately exits, I'm not too concerned about checking for whether it's a key press or release. Currently unreleased in crossterm there is code that adds an Event::is_key_press() method, so these checks will become e.g. if event::poll(frame_timeout)? && event::read()?.is_key_press() { when that is released.
The only real concern that really require this is that I have heard it rumored that sometimes windows terminals will pass through the key release event of the enter key that was used to run the command that runs the app. I don't use windows often enough to have seen this personally though, so it's a hypothetical problem right now.
There was a problem hiding this comment.
The only real concern that really require this is that I have heard it rumored that sometimes windows terminals will pass through the key release event of the enter key that was used to run the command that runs the app. I don't use windows often enough to have seen this personally though, so it's a hypothetical problem right now.
I've seen this actually happen a few times since then, but the change would be to check event::read()?.is_key_press() which was introduced in crossterm 0.29 instead.
|
Should we rebase and finalize this PR? |
|
Yep. This is on my list for today. The examples changed enough that it doesn't clearly rebase easily, so it's a bit of work to do this. |
This introduces a new
ratatui::run()method which runs a closure witha terminal initialized with reasonable defaults for most applications.
This calls
ratatui::init()before running the closure andratatui::restore()after the closure completes, and returns the resultof the closure.
A minimal hello world example using the new
ratatui::run()method:Of course, this also works both with apps that use free methods and structs: