feat(backend): Add convenience methods for setting up terminals #1180

joshka · 2024-06-16T00:38:48Z

Adds:

CrosstermBackend::stdout and CrosstermBackend::stderr for creating
backends with std::io::stdout and std::io::stderr respectively.
CrosstermBackend::stdout_with_defaults and
CrosstermBackend::stderr_with_defaults for creating
backends with that use the alternate screen and raw mode.
CrosstermBackend::with_raw_mode for enabling raw mode.
CrosstermBackend::with_alternate_screen for switching to the
alternate screen.
CrosstermBackend::with_mouse_capture for enabling mouse support.
CrosstermBackend::reset for resetting the terminal to its default
state.
Drop implementation for restoring raw mode, mouse capture, and
alternate screen on drop.

Most applications should generally use stdout_with_defaults to create
a backend with raw mode and the alternate screen, and now longer need
to worry about restoring the terminal to its default state when the
application exits. The reset method can be used to restore the terminal to
its default state if needed (e.g. in a panic handler).

use ratatui::{backend::CrosstermBackend};

let backend = CrosstermBackend::stdout_with_defaults()?;
// or
let backend = CrosstermBackend::stderr_with_defaults()?;
// or with custom settings
let backend = CrosstermBackend::stdout()
    .with_raw_mode()?
    .with_alternate_screen()?
    .with_mouse_capture()?

use std::io::stderr();

// in a panic / error handler:
CrosstermBackend::reset(stderr())

codecov · 2024-06-16T00:39:57Z

Codecov Report

Attention: Patch coverage is 0% with 83 lines in your changes missing coverage. Please review.

Project coverage is 93.9%. Comparing base (3f2f2cd) to head (97bf4a3).
Report is 56 commits behind head on main.

Files	Patch %	Lines
src/backend/crossterm.rs	0.0%	83 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1180     +/-   ##
=======================================
- Coverage   94.4%   93.9%   -0.5%     
=======================================
  Files         62      62             
  Lines      14941   15127    +186     
=======================================
+ Hits       14110   14217    +107     
- Misses       831     910     +79

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

joshka · 2024-06-16T01:36:19Z

Note - this is intentionally only implemented for crossterm right now, and it's not carried through to the examples as yet to allow for review of the concept without the bulk.

Note also the comments / previous effort on #280

EdJoPaTo

I like seeing the with_* methods and the internal state to allow for a simpler reset. This will simplify a lot of code.

With other stuff I have my problems, noted on them directly for easier, threaded discussions.

src/backend/crossterm.rs

In <#1180> and <#1181>, we introduced some simpler ways to create a backend and terminal, and to setup color_eyre to handle errors. This PR updates the examples to use these new methods, and removes a bunch of unnecessary boilerplate code.

src/backend/crossterm.rs

Changes addressed

EdJoPaTo

I really like the simplicity that will come with this and a built-in panic handler. I still want to provoke some thoughts about the current state here. (Also, I want to test this branch to get a feel for its real world usage, which I haven't done yet.)

src/backend/crossterm.rs

examples/minimal.rs

examples/colors_rgb.rs

src/backend/crossterm.rs

examples/colors_rgb.rs

Adds: - `CrosstermBackend::stdout` and `CrosstermBackend::stderr` for creating backends with `std::io::stdout` and `std::io::stderr` respectively. - `CrosstermBackend::into_terminal` for converting a backend into a terminal. - `CrosstermBackend::into_terminal_with_options` for converting a backend into a terminal with options. - `CrosstermBackend::into_terminal_with_defaults` for converting a backend into a terminal with default settings (raw mode and alternate screen). - `CrosstermBackend::with_raw_mode` for enabling raw mode. - `CrosstermBackend::with_alternate_screen` for switching to the alternate screen. - `CrosstermBackend::with_mouse_support` for enabling mouse support. - `CrosstermBackend::reset` for resetting the terminal to its default state. - `Drop` implementation for restoring raw mode, mouse capture, and alternate screen on drop. Most applications can now just use the `into_terminal_with_defaults` method to set up the terminal with raw mode and the alternate screen, and now longer need to worry about restoring the terminal to its default state when the application exits. The reset method can be used to restore the terminal to its default state if needed (e.g. in a panic handler). ```rust use ratatui::backend::CrosstermBackend; let terminal = CrosstermBackend::stdout() .into_terminal_with_defaults()?; ```

…ted changes

EdJoPaTo · 2024-06-28T12:23:18Z

I am curious why crossterm itself doesn't have something like this. I think everyone using crossterm would benefit from something like this. Should this be proposed for crossterm?

joshka · 2024-06-28T12:42:54Z

I am curious why crossterm itself doesn't have something like this. I think everyone using crossterm would benefit from something like this. Should this be proposed for crossterm?

Define "this".

Regardless it doesn't change whether we'd want this PR to land in the meantime as the development cycle on that side of the fence is pretty slow.

@EdJoPaTo do you have any hard blockers for this PR getting merged? This PR underpins a massive simplification of all the example code in Ratatui as well as customer apps, tutorials, recipes, etc. I'm blocked on completing all of that by this not being merged.

EdJoPaTo

I still like the simplification that will come with this a lot. Some curiosity / minor nitpicks remain, some docs should improve, then I think this can be merged.

@joshka dismissed my review, I think this should be enabled automatically for this repo: dismiss reviews on new commits. Either it's quite fast to review them again or it might have changed something that should be reviewed again. (Maybe also a review of multiple and not just one person might be ideal especially for bigger things.)

examples/minimal.rs

EdJoPaTo · 2024-06-28T13:21:31Z

src/backend/crossterm.rs

+/// to create a `CrosstermBackend` with [`std::io::stdout`] or [`std::io::stderr`] as the writer
+/// with default settings to enable raw mode and switch to the alternate screen.


The default went away from the method name. It provides the most used ways of using it. The docs should reflect it in my opinion.

Suggested change

/// to create a `CrosstermBackend` with [`std::io::stdout`] or [`std::io::stderr`] as the writer

/// with default settings to enable raw mode and switch to the alternate screen.

/// to create a `CrosstermBackend` with [`std::io::stdout`] or [`std::io::stderr`] as the writer

/// with the often used settings to enable raw mode and switch to the alternate screen.

Marked as unresolved. Should be adapted once a better name is figured out (#1180 (comment)).

EdJoPaTo · 2024-06-28T13:23:33Z

src/backend/crossterm.rs

+/// If the default settings are not desired, the `CrosstermBackend` can be configured using the
+/// `with_*` methods. These methods return an [`io::Result`] containing self so that they can be


Remove the mentioning of the default. Also, it sounds like it can be reverted while ::stdout().without_raw_mode() doesn't exist, so phrase it better.

(Will need reformatting for alignment for sure)

Suggested change

/// If the default settings are not desired, the `CrosstermBackend` can be configured using the

/// `with_*` methods. These methods return an [`io::Result`] containing self so that they can be

/// Additional settings be configured using the

/// `with_*` methods. These methods return an [`io::Result`] containing self so that they can be

I think this is clear enough and seems like too much of a nitpick to worry about changing.

Reading this again: It's also way easier to read and understand in my opinion.

src/backend/crossterm.rs

examples/colors_rgb.rs

EdJoPaTo · 2024-06-28T13:38:31Z

src/backend/crossterm.rs

+ Ok(self)
+ }
+
+ /// Restores the terminal to its default state.


Be a bit more clear that this is for an argument and not &self (or similar).

Suggested change

/// Restores the terminal to its default state.

/// Restores the given terminal to its default state.

Still think this would be helpful to differentiate more clearly between the &self methods and this static method consuming some other reader.

EdJoPaTo · 2024-06-28T13:40:34Z

src/backend/crossterm.rs

+ /// If you have created a `CrosstermBackend` using the `with_*` methods, the settings are
+ /// restored when the `CrosstermBackend` is dropped, so you do not need to call this method
+ /// manually.


Maybe move this comment up? General description, why might I don't need to read further, what does it in detail.

src/backend/crossterm.rs

EdJoPaTo · 2024-06-28T13:42:44Z

src/backend/crossterm.rs

+ if self.restore_raw_mode_on_drop {
+ let _ = disable_raw_mode();
+ }
+ if self.restore_mouse_capture_on_drop {
+ let _ = execute!(self.writer, DisableMouseCapture);
+ }
+ if self.restore_alternate_screen_on_drop {
+ let _ = execute!(self.writer, LeaveAlternateScreen);
+ }
+ if self.restore_bracketed_paste_on_drop {
+ let _ = execute!(self.writer, DisableBracketedPaste);
+ }
+ if self.restore_focus_change_on_drop {
+ let _ = execute!(self.writer, DisableFocusChange);
+ }
+ if self.restore_keyboard_enhancement_flags_on_drop {
+ let _ = execute!(self.writer, PopKeyboardEnhancementFlags);
+ }


Same curiosity: Is the order relevant?

Searched the crossterm docs for a bit and didn't find an obvious hint to make sure of some order here. So probably fine to invoke them in any order.

src/backend/crossterm.rs

joshka · 2024-06-28T14:47:38Z

@joshka dismissed my review, I think this should be enabled automatically for this repo: dismiss reviews on new commits. Either it's quite fast to review them again or it might have changed something that should be reviewed again. (Maybe also a review of multiple and not just one person might be ideal especially for bigger things.)

That approach causes a long turn around time for things which don't matter. I prefer to work based on trust that we'll do the right thing once given the go ahead to merge.

Co-authored-by: EdJoPaTo <rfc-conform-git-commit-email@funny-long-domain-label-everyone-hates-as-it-is-too-long.edjopato.de>

joshka

I've noted that a most of these points are nitpicks and my tolerance for handling this amount of not relevant feedback on this PR is rapidly approaching zero. There's nothing that would block this from being merged or that would cause a user not to understand how these methods work. As such resolving them.

examples/colors_rgb.rs

examples/minimal.rs

src/backend/crossterm.rs

joshka · 2024-06-28T15:03:35Z

src/backend/crossterm.rs

+/// If the default settings are not desired, the `CrosstermBackend` can be configured using the
+/// `with_*` methods. These methods return an [`io::Result`] containing self so that they can be


I think this is clear enough and seems like too much of a nitpick to worry about changing.

src/backend/crossterm.rs

joshka · 2024-06-28T15:12:49Z

src/backend/crossterm.rs

+ Ok(self)
+ }
+
+ /// Restores the terminal to its default state.


joshka · 2024-06-28T15:13:00Z

src/backend/crossterm.rs

+ /// If you have created a `CrosstermBackend` using the `with_*` methods, the settings are
+ /// restored when the `CrosstermBackend` is dropped, so you do not need to call this method
+ /// manually.


src/backend/crossterm.rs

joshka · 2024-06-28T15:13:32Z

src/backend/crossterm.rs

+ if self.restore_raw_mode_on_drop {
+ let _ = disable_raw_mode();
+ }
+ if self.restore_mouse_capture_on_drop {
+ let _ = execute!(self.writer, DisableMouseCapture);
+ }
+ if self.restore_alternate_screen_on_drop {
+ let _ = execute!(self.writer, LeaveAlternateScreen);
+ }
+ if self.restore_bracketed_paste_on_drop {
+ let _ = execute!(self.writer, DisableBracketedPaste);
+ }
+ if self.restore_focus_change_on_drop {
+ let _ = execute!(self.writer, DisableFocusChange);
+ }
+ if self.restore_keyboard_enhancement_flags_on_drop {
+ let _ = execute!(self.writer, PopKeyboardEnhancementFlags);
+ }


src/backend/crossterm.rs

joshka · 2024-06-28T15:46:49Z

@EdJoPaTo see my email about nits etc.

kdheepak · 2024-06-29T01:08:34Z

Most of the PR looks good to me (but I haven't read all the comments on this thread), will do so over the weekend.

kdheepak · 2024-06-29T01:10:13Z

src/backend/crossterm.rs

+ pub fn stdout() -> io::Result<Self> {
+ Self::new(io::stdout())
+ .with_raw_mode()?
+ .with_alternate_screen()
+ }


The PR description says:

CrosstermBackend::stdout and CrosstermBackend::stderr for creating backends with std::io::stdout and std::io::stderr respectively.

CrosstermBackend::stdout_with_defaults and CrosstermBackend::stderr_with_defaults for creating backends with that use the alternate screen and raw mode.

which doesn't match this implementation.

I prefer it to work the way the PR description mentions. It makes it much more clearer imo.

Alternatively, CrosstermBackend::stdout_tui and CrosstermBackend::stderr_tui.

The idea behind doing some reasonable defaults (raw/alternate screen) on these is that this is necessary for 90% of all apps. Anything else can do CrosstermBackend::new(stdout()).with_xxx()...

It'll be a little confusing imo to find in a Ratatui app's user code two things that are very similar but do different things

CrosstermBackend::new(stdout()) for inline

CrosstermBackend::stdout() for a TUI

I like the reasonable defaults in place here, I'm just not convinced that the name conveys those defaults clearly.

I like the idea of CrosstermBackend::stdout_tui() with the alternative of CrossteermBackend::new(stdout()).with_* is actually quite good. It removes the CrosstermBackend::stdout() == CrosstermBackend::new(stdout()) similarity and is not just default which is more confusing as there are different defaults for different use cases.

EdJoPaTo · 2024-07-31T10:41:46Z

src/backend/crossterm.rs

+ execute!(
+ writer,
+ LeaveAlternateScreen,
+ DisableMouseCapture,
+ DisableBracketedPaste,
+ DisableFocusChange,
+ PopKeyboardEnhancementFlags
+ )?;
+ writer.flush()


execute! calls flush already. This likely wont change in crossterm as execute is queue + flush.

Suggested change

execute!(

writer,

LeaveAlternateScreen,

DisableMouseCapture,

DisableBracketedPaste,

DisableFocusChange,

PopKeyboardEnhancementFlags

)?;

writer.flush()

execute!(

writer,

LeaveAlternateScreen,

DisableMouseCapture,

DisableBracketedPaste,

DisableFocusChange,

PopKeyboardEnhancementFlags

)

EdJoPaTo · 2024-07-31T10:46:55Z

src/backend/crossterm.rs

+ fn drop(&mut self) {
+ // note that these are not checked for errors because there is nothing that can be done if
+ // they fail. The terminal is likely in a bad state, and the application is exiting anyway.
+ if self.restore_raw_mode_on_drop {
+ let _ = disable_raw_mode();
+ }
+ if self.restore_mouse_capture_on_drop {
+ let _ = execute!(self.writer, DisableMouseCapture);
+ }
+ if self.restore_alternate_screen_on_drop {
+ let _ = execute!(self.writer, LeaveAlternateScreen);
+ }
+ if self.restore_bracketed_paste_on_drop {
+ let _ = execute!(self.writer, DisableBracketedPaste);
+ }
+ if self.restore_focus_change_on_drop {
+ let _ = execute!(self.writer, DisableFocusChange);
+ }
+ if self.restore_keyboard_enhancement_flags_on_drop {
+ let _ = execute!(self.writer, PopKeyboardEnhancementFlags);
+ }
+ let _ = self.writer.flush();
+ }


Thought: execute! is queue! + flush. This method ends with flush already. It could use queue! for all the calls and then flush once in the end.
Even when something fails the errors are ignored and the flush call is definitely executed.

It's probably not that relevant as it's not called that often, but I still think a worthy improvement as queue and execute both exist for exactly this reason.

EdJoPaTo · 2024-07-31T10:50:32Z

src/backend/crossterm.rs

+ fn drop(&mut self) {
+ // note that these are not checked for errors because there is nothing that can be done if
+ // they fail. The terminal is likely in a bad state, and the application is exiting anyway.
+ if self.restore_raw_mode_on_drop {
+ let _ = disable_raw_mode();
+ }
+ if self.restore_mouse_capture_on_drop {
+ let _ = execute!(self.writer, DisableMouseCapture);
+ }
+ if self.restore_alternate_screen_on_drop {
+ let _ = execute!(self.writer, LeaveAlternateScreen);
+ }
+ if self.restore_bracketed_paste_on_drop {
+ let _ = execute!(self.writer, DisableBracketedPaste);
+ }
+ if self.restore_focus_change_on_drop {
+ let _ = execute!(self.writer, DisableFocusChange);
+ }
+ if self.restore_keyboard_enhancement_flags_on_drop {
+ let _ = execute!(self.writer, PopKeyboardEnhancementFlags);
+ }
+ let _ = self.writer.flush();
+ }


Given that CrosstermBackend::restore just does all these without checking for boolean flags. Is there a downside to do it that way in all the cases?

This would simplify this to just call CrosstermBackend::restore(self.writer) and removes a lot of boolean flags resulting in less complexity or possibility for mistakes.

EdJoPaTo · 2024-08-04T11:56:58Z

As a crossterm version change is included with the upcoming release I would like to reduce the stuff needed from crossterm even further with the 0.28 release. Can we get this into the release? (Without rushing it, I would prefer to wait with the release in order to resolve all the thoughts to make this right)

joshka · 2024-08-04T12:23:28Z

As a crossterm version change is included with the upcoming release I would like to reduce the stuff needed from crossterm even further with the 0.28 release. Can we get this into the release? (Without rushing it, I would prefer to wait with the release in order to resolve all the thoughts to make this right)

I think this might be worth missing the 0.28.0 release. There's a lot of small nuanced things to go through, and then once this lands it would be nice to revamp all the examples to use this, that's a fair bit bigger than I'd like to commit to before releasing 0.28, and I'd really like those two things to go together.

joshka · 2024-08-05T04:04:11Z

Something to consider as a total simplification would be to just have Terminal::init() and Terminal::restore() do the right thing (crossterm, stdout, raw mode, alternate screen). -> #1289

joshka requested review from orhun, kdheepak, Valentin271 and EdJoPaTo as code owners June 16, 2024 00:38

joshka changed the title ~~feat(backend): add convenience methods~~ feat(backend): Add convenience methods for setting up terminals Jun 16, 2024

joshka force-pushed the jm/crossterm-init branch 2 times, most recently from aacf548 to 1d176d9 Compare June 16, 2024 01:28

EdJoPaTo requested changes Jun 16, 2024

View reviewed changes

EdJoPaTo previously requested changes Jun 17, 2024

View reviewed changes

src/backend/crossterm.rs Outdated Show resolved Hide resolved

src/backend/crossterm.rs Outdated Show resolved Hide resolved

src/backend/crossterm.rs Outdated Show resolved Hide resolved

joshka mentioned this pull request Jun 18, 2024

chore(examples): simplify all examples #1189

Draft

joshka requested a review from EdJoPaTo June 18, 2024 06:07

This was referenced Jun 19, 2024

Generalize Terminal Backend cleanup #1042

Open

feat(color-eyre): Add color eyre to the default app terminal setup #1181

Draft

joshka added Status: Controversial There is some discussion about whether this is the right direction. Clarify and seek consensus. Status: Review Needed PR needs a review / Issue needs buy-in from other maintainers / users labels Jun 19, 2024

EdJoPaTo mentioned this pull request Jun 19, 2024

feat(docs): add tracing example #1192

Merged

orhun reviewed Jun 19, 2024

View reviewed changes

src/backend/crossterm.rs Show resolved Hide resolved

joshka requested a review from orhun June 20, 2024 00:50

EdJoPaTo reviewed Jun 20, 2024

View reviewed changes

joshka added 7 commits June 20, 2024 19:13

docs(examples): use backend convenience methods for minimal example

6de838d

docs(examples): use backend convenience methods for colors_rgb example

d45c433

fix: move to_terminal to Backend, added other configs and review rela…

a2b1eda

…ted changes

fix: import

aecd5eb

fix: formatting and clippy

fd6a8aa

fix: doc fix

d9a71b5

joshka force-pushed the jm/crossterm-init branch from 0b75ddc to 48d6c5a Compare June 21, 2024 02:18

joshka requested a review from EdJoPaTo June 21, 2024 02:18

fix: missing ? in doc comments

c23872a

joshka mentioned this pull request Jun 27, 2024

Feedback on tui with terminal and event handler recipe ratatui-org/ratatui-website#648

Open

This was referenced Jun 28, 2024

Exports should only be in a single location #1210

Open

build: update ratatui to v0.27 EdJoPaTo/tui-rs-tree-widget#40

Merged

EdJoPaTo reviewed Jun 28, 2024

View reviewed changes

fix: docs

0a8f2f2

Co-authored-by: EdJoPaTo <rfc-conform-git-commit-email@funny-long-domain-label-everyone-hates-as-it-is-too-long.edjopato.de>

joshka commented Jun 28, 2024

View reviewed changes

fix: Update src/backend/crossterm.rs

97bf4a3

This comment was marked as duplicate.

Sign in to view

joshka requested a review from EdJoPaTo June 28, 2024 15:30

kdheepak reviewed Jun 29, 2024

View reviewed changes

joshka mentioned this pull request Jul 1, 2024

feat: Add spawn vim docs ratatui-org/ratatui-website#651

Open

deepanchal mentioned this pull request Jul 14, 2024

feat: Add ExecuteCommand action in component template ratatui-org/templates#62

Open

joshka mentioned this pull request Jul 17, 2024

docs(examples): simplify the barchart example #1079

Merged

EdJoPaTo mentioned this pull request Jul 31, 2024

Idea: Merge Terminal with the Backend trait #1251

Open

EdJoPaTo reviewed Jul 31, 2024

View reviewed changes

joshka added Status: Revision Needed PR changes requested Status: On Hold Not actively being worked on for now due to time / other constraints and removed Status: Review Needed PR needs a review / Issue needs buy-in from other maintainers / users labels Aug 4, 2024

joshka mentioned this pull request Aug 5, 2024

feat(terminal): Add Terminal::init() and restore() #1289

Open

		/// to create a `CrosstermBackend` with [`std::io::stdout`] or [`std::io::stderr`] as the writer
		/// with default settings to enable raw mode and switch to the alternate screen.

		/// If the default settings are not desired, the `CrosstermBackend` can be configured using the
		/// `with_*` methods. These methods return an [`io::Result`] containing self so that they can be

	/// Restores the terminal to its default state.
	/// Restores the given terminal to its default state.

feat(backend): Add convenience methods for setting up terminals #1180

Are you sure you want to change the base?

feat(backend): Add convenience methods for setting up terminals #1180

Conversation

joshka commented Jun 16, 2024 • edited Loading

codecov bot commented Jun 16, 2024 • edited Loading

Codecov Report

joshka commented Jun 16, 2024

EdJoPaTo left a comment

Choose a reason for hiding this comment

EdJoPaTo left a comment

Choose a reason for hiding this comment

EdJoPaTo commented Jun 28, 2024

joshka commented Jun 28, 2024

EdJoPaTo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshka commented Jun 28, 2024

joshka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

joshka commented Jun 28, 2024

kdheepak commented Jun 29, 2024

kdheepak Jun 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kdheepak Jun 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EdJoPaTo commented Aug 4, 2024

joshka commented Aug 4, 2024

joshka commented Aug 5, 2024 • edited Loading

joshka commented Jun 16, 2024 •

edited

Loading

codecov bot commented Jun 16, 2024 •

edited

Loading

kdheepak Jun 29, 2024 •

edited

Loading

kdheepak Jun 29, 2024 •

edited

Loading

joshka commented Aug 5, 2024 •

edited

Loading