Open joshka opened 2 weeks ago
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 10 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
src/backend/crossterm.rs | 0.0% | 83 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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 https://github.com/ratatui-org/ratatui/pull/280
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?
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.
@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.
@EdJoPaTo see my email about nits etc.
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.
Adds:
CrosstermBackend::stdout
andCrosstermBackend::stderr
for creating backends withstd::io::stdout
andstd::io::stderr
respectively.CrosstermBackend::stdout_with_defaults
andCrosstermBackend::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).