-
-
Notifications
You must be signed in to change notification settings - Fork 128
WIP: bump ratatui to 0.30.0 #1588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Blocked on MSRV:
Ratatui Trippy |
7a2b622
to
da12751
Compare
crates/trippy-tui/src/frontend.rs
Outdated
fn run_app<B: Backend>(terminal: &mut Terminal<B>, app: &mut TuiApp) -> io::Result<ExitAction> { | ||
fn run_app( | ||
terminal: &mut Terminal<CrosstermBackend<Stdout>>, | ||
app: &mut TuiApp, | ||
) -> io::Result<ExitAction> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@orhun is this the right way to handle the change in Terminal
in 0.30.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks correct to me... I don't remember the change led to this though. Can you share some references that requires this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I bumped the Ratatui version and tested it failed with:
error[E0277]: `?` couldn't convert the error to `std::io::Error`
--> crates/trippy-tui/src/frontend.rs:80:55
|
73 | fn run_app<B: Backend>(terminal: &mut Terminal<B>, app: &mut TuiApp) -> io::Result<ExitAction> {
| ---------------------- expected `std::io::Error` because of this
...
80 | terminal.draw(|f| render::app::render(f, app))?;
| -------------------------------------^ the trait `std::convert::From<<B as ratatui::backend::Backend>::Error>` is not implemented for `std::io::Error`
| |
| this can't be annotated with `?` because it has type `Result<_, <B as ratatui::backend::Backend>::Error>`
|
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement
|
73 | fn run_app<B: Backend>(terminal: &mut Terminal<B>, app: &mut TuiApp) -> io::Result<ExitAction> where std::io::Error: std::convert::From<<B as ratatui::backend::Backend>::Error> {
| +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
I did some digging at the time and I think it relates to this: ratatui/ratatui#1775 (which is correctly marked as a breaking change)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also works and is likely better:
fn run_app<B: Backend<Error = io::Error>>(
terminal: &mut Terminal<B>,
app: &mut TuiApp,
) -> Result<ExitAction, B::Error> {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 0.30 io::Error
is no longer explicitly used for Backend
errors, built-in backends (except TestBackend
) still use io::Error
, so this should in most cases be backward compatible, but when using the Backend
trait directly, the associated error type is unknown, so it requires additional bound (as you did).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can probably use DefaultTerminal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can probably use
DefaultTerminal
Oh I didn't see your comment, yeah: #1588 (comment)
da12751
to
cc5a108
Compare
f0e0f47
to
b3036a0
Compare
crates/trippy-tui/src/frontend.rs
Outdated
fn run_app<B: Backend<Error = io::Error>>( | ||
terminal: &mut Terminal<B>, | ||
app: &mut TuiApp, | ||
) -> Result<ExitAction, B::Error> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be simply
) -> Result<ExitAction, B::Error> { | |
) -> io::Result<ExitAction> { |
since your bound already requires B::Error
to be io::Error
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, since you are only using crossterm, you can also just get rid of the generic type parameter completely:
fn run_app(
terminal: &mut DefaultTerminal,
app: &mut TuiApp,
) -> io::Result<ExitAction> {
DefaultTerminal
is an alias for Terminal<CrosstermBackend>
and CrosstermBackend
uses io::Error
.
This is an error, the MSRV is 1.85.0 |
b3036a0
to
528842c
Compare
528842c
to
0cbe825
Compare
c4fd430
to
791a695
Compare
This definitely needs rust 1.85 now due to the 2024 edition changes. |
@joshka yes, i'm considering reducing Trippy's Rust version policy from 12 months down to 6 months, which would put the next release on 1.85, roughly. It is a balance between trying to use an old enough toolchain to be supported on various distros and new enough to keep up with the Rust library ecosystem. I've found that in practice 12 months is to long, the Rust world moves on to quickly (especially around the edition bump) and so I think 6 months may be a better balance. |
Our philosophy in Ratatui is that we generally won't update the required version beyond N-2. This gives people at least 3 months to work out what they're doing and doesn't artificially prevent us from taking advantage of new things for too long (e.g. we waited until 1.87 was out to flip over to the 2024 edition). Obviously your use cases aren't ours.
Good news - Debian Trixie looks like it will support 1.85. but by the time it comes out I'd guess Ratatui will likely have moved past that version... That said, I'm very apprehensive about distros pinning to versions like this. Artificial limitations on calling functions that have been available for years (rust 1.63 was released in 2022), are a game that I choose not to participate in unless forced to. Case in point, I had a job interview the other day where one part of the solution was a simple method introduced in the std lib in 1.80, but coderpad runs 1.72, so I had to polyfill the impl live. That was somewhat annoying. |
See https://github.com/ratatui/ratatui/releases/tag/ratatui-v0.30.0-alpha.3