chore(deps): bump ratatui to 0.26.2 and crossterm to 0.27.0 #515

joshka · 2024-02-03T00:45:22Z

Bumps MSRV to 1.74.0

This necessitated 3 changes to the codebase:

Spans was renamed to the more ergonomic Line type.
Frame no longer requires a backend type parameter.
Table::new requires a widths parameter, so we use
Table::default().rows(rows) instead of Table::new(rows).

Crossterm on Windows triggers events for key press as well as release
and repeat, which causes duplicate key presses. This change filters out
those events.

hawkw · 2024-02-06T17:25:13Z

It looks like this change requires a MSRV bump, as the new ratatui seems not to support Rust 1.64.0: https://github.com/tokio-rs/console/actions/runs/7763271246/job/21175134601#step:8:1

joshka · 2024-02-06T18:06:05Z

It looks like this change requires a MSRV bump, as the new ratatui seems not to support Rust 1.64.0: https://github.com/tokio-rs/console/actions/runs/7763271246/job/21175134601#step:8:1

Thanks - bumped this to 1.70

hawkw · 2024-02-08T17:49:09Z

@joshka mind rebasing this branch onto the latest master and resolving the lockfile conflicts? Once that's done, this should be good to merge!

joshka · 2024-02-15T21:38:50Z

1:31 PM]joshka: I notice that some tests are failing now - digging to check what's happening
[1:31 PM]Hayden (he/him): if it's the self wakes test then it's known flaky. 
[1:31 PM]Hayden (he/him): (I need to remove it)
[1:37 PM]joshka: Ah got it - yep it was that. I did't a quick cargo update on main and saw the tests fail there too (it didn't fail before then)

joshka · 2024-04-01T21:33:24Z

Squashed and rebased

hi-rustin

Regarding the failed test, it is failing consistently. Maybe it is not an unstable test. More like we updated some dependencies causing a change in behaviour?
Thanks for your contribution! 🤟 🖤

Cargo.lock

hi-rustin · 2024-04-09T10:50:10Z

There are some proto changes:

/// Each `ResourceUpdate` contains any resource data that has changed since the last
diff --git a/console-api/src/generated/rs.tokio.console.tasks.rs b/console-api/src/generated/rs.tokio.console.tasks.rs
index 41b8396..e8ee5fa 100644
--- a/console-api/src/generated/rs.tokio.console.tasks.rs
+++ b/console-api/src/generated/rs.tokio.console.tasks.rs
@@ -1,3 +1,4 @@
+// This file is @generated by prost-build.
 /// A task state update.
 ///
 /// Each `TaskUpdate` contains any task data that has changed since the last
diff --git a/console-api/src/generated/rs.tokio.console.trace.rs b/console-api/src/generated/rs.tokio.console.trace.rs
index [220](https://github.com/tokio-rs/console/actions/runs/8603263859/job/23574759021?pr=515#step:6:221)b55a..be3516e 100644
--- a/console-api/src/generated/rs.tokio.console.trace.rs
+++ b/console-api/src/generated/rs.tokio.console.trace.rs
@@ -1,3 +1,4 @@
+// This file is @generated by prost-build.
 /// Start watching trace events with the provided filter.
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]
test bootstrap ... FAILED

Because there are some other issues in this PR, I would recommend:

This PR only focuses on bumping the rataui version.
If the rataui requires a higher MSRV, we bump the MSRV but do not change other dependencies' versions.

Perhaps we could address the issue of the failed test that was caused by changes in other dependencies.

Also, this would help us make sure the scope of this PR is not extended. We can only focus on bumping the rataui version.

Thank you for your patch again!

hawkw

I've noticed a large number of unrelated formatting changes. I would really prefer not to merge those as part of this PR, because then, when viewing the Git history, those lines will have been changed as part of the Ratatui update, when it was not actually part of that change. I'm happy to accept formatting changes if they are considered better or more idiomatic style, but please make those changes in a separate branch.

If you're manually reformatting TOML as part of this PR, can you please put it back the way it was? Or, if your editor is doing this automatically, can you please disable that setting for this repo? Thank you!

tokio-console/Cargo.toml

tokio-console/README.md

xtask/Cargo.toml

console-subscriber/README.md

console-subscriber/Cargo.toml

joshka · 2024-04-25T20:26:48Z

I've fixed the PR to just focus on the Ratatui changes (and bumped it to 0.26.2 which was released after this PR was created). I've also bumped crossterm to 0.27.0 to avoid incompatible versions, and fixed a small bug which hits most crossterm based apps on Windows (duplicate key presses).

I've bumped all the projects to MSRV 1.74 as this is the version that we build with in Ratatui. I'm not sure if it was right to just bump the tokio-console project or all the files, but this seems to be the least likely to cause CI pain (e.g. building the libs with 1.64 and the tui app with 1.74).

hds

Thank you so much for following through with this change @joshka!

This looks good to me now.

@hawkw Your change request will be blocking this change from being merged now. Would you mind having another look.

hi-rustin

The rest looks good to me.

Thank you very much for working on this!

hi-rustin · 2024-05-06T12:47:51Z

tokio-console/src/main.rs

@@ -95,6 +95,11 @@ async fn main() -> color_eyre::Result<()> {
 let input = input
 .ok_or_else(|| eyre!("keyboard input stream ended early"))
 .with_section(|| "this is probably a bug".header("Note:"))??;
+
+ if input::should_ignore_key_event(&input) {


console/tokio-console/src/main.rs

Line 81 in bb463c1

let mut input = Box::pin(input::EventStream::new().try_filter(|event| {

We already have a filter here to ignore the release event. Maybe we should consider merging them together.

I missed seeing that, fixing it by just using the should_ignore_key_event instead of the filter. Either way would be fine I guess.

nit: @hds @hawkw Do you have any thoughts about which one is better? It seems both work well. The only difference is that the try_filter will filter it asynchronously.

hi-rustin

Looks good to me. Thanks! 👍

hi-rustin · 2024-05-08T13:18:02Z

Lint CI fixed in #548

hi-rustin · 2024-05-09T03:02:22Z

@joshka There are some unused imports after your change. Could you please help to remove them? Thanks!

joshka · 2024-05-09T05:26:06Z

@joshka There are some unused imports after your change. Could you please help to remove them? Thanks!

Done

hds · 2024-05-09T08:35:39Z

@hawkw Could you please rereview and approve (if you're happy with it) this PR? Your request for changes is blocking at the moment. Thanks!

Bumps MSRV to 1.74.0 This necessitated 3 changes to the codebase: - `Spans` was renamed to the more ergonomic `Line` type. - `Frame` no longer requires a backend type parameter. - `Table::new` requires a widths parameter, so we use `Table::default().rows(rows)` instead of `Table::new(rows)`. Crossterm on Windows triggers events for key press as well as release and repeat, which causes duplicate key presses. This change filters out those events.

The changes that Eliza requested have been made, and she also previously approved the PR.

joshka requested a review from a team as a code owner February 3, 2024 00:45

hawkw approved these changes Feb 8, 2024

View reviewed changes

damccull mentioned this pull request Feb 15, 2024

Key presses happen twice on Windows #514

Open

joshka force-pushed the dev branch from 2e2814e to 4ead8d7 Compare February 15, 2024 21:29

joshka changed the title ~~chore(deps): update ratatui to 0.26.0~~ chore(deps): update ratatui to 0.26.1 Feb 15, 2024

joshka force-pushed the dev branch 2 times, most recently from 3e4f923 to 331a22d Compare February 17, 2024 04:59

joshka force-pushed the dev branch from 331a22d to 1f9c92a Compare April 1, 2024 21:33

joshka requested a review from hawkw April 4, 2024 04:24

hi-rustin reviewed Apr 8, 2024

View reviewed changes

Cargo.lock Outdated Show resolved Hide resolved

joshka force-pushed the dev branch from 1f9c92a to 25e1efc Compare April 8, 2024 16:07

hds mentioned this pull request Apr 10, 2024

chore: get rid of remove_dir_all #542

Merged

hawkw previously requested changes Apr 10, 2024

View reviewed changes

joshka force-pushed the dev branch 2 times, most recently from 3127d33 to 4413a62 Compare April 25, 2024 20:19

joshka force-pushed the dev branch from 4413a62 to fdabae6 Compare April 25, 2024 20:28

joshka requested review from hi-rustin and hawkw April 25, 2024 20:29

joshka changed the title ~~chore(deps): update ratatui to 0.26.1~~ chore(deps): bump ratatui to 0.26.2 and crossterm to 0.27.0 Apr 25, 2024

hds approved these changes Apr 26, 2024

View reviewed changes

hi-rustin reviewed May 6, 2024

View reviewed changes

hi-rustin approved these changes May 8, 2024

View reviewed changes

hi-rustin force-pushed the dev branch from fadfbf3 to 17107af Compare May 8, 2024 14:04

hi-rustin mentioned this pull request May 11, 2024

chore(deps): bump tonic to 0.11 and raise MSRV to 1.70 #547

Merged

joshka added 3 commits May 11, 2024 22:35

fix: use should_ignore_key_event instead of filter

2f3d58e

style: cleanup imports

2b11d29

hi-rustin force-pushed the dev branch from aa47342 to 2b11d29 Compare May 11, 2024 13:35

hds merged commit 6cbd6db into tokio-rs:main May 16, 2024
17 checks passed

joshka deleted the dev branch May 19, 2024 04:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(deps): bump ratatui to 0.26.2 and crossterm to 0.27.0 #515

chore(deps): bump ratatui to 0.26.2 and crossterm to 0.27.0 #515

joshka commented Feb 3, 2024 •

edited

hawkw commented Feb 6, 2024

joshka commented Feb 6, 2024

hawkw commented Feb 8, 2024

joshka commented Feb 15, 2024

joshka commented Apr 1, 2024

hi-rustin left a comment •

edited

hi-rustin commented Apr 9, 2024

hawkw left a comment

joshka commented Apr 25, 2024

hds left a comment

hi-rustin left a comment

hi-rustin May 6, 2024

joshka May 6, 2024

hi-rustin May 8, 2024 •

edited

hi-rustin left a comment

hi-rustin commented May 8, 2024

hi-rustin commented May 9, 2024

joshka commented May 9, 2024

hds commented May 9, 2024

chore(deps): bump ratatui to 0.26.2 and crossterm to 0.27.0 #515

chore(deps): bump ratatui to 0.26.2 and crossterm to 0.27.0 #515

Conversation

joshka commented Feb 3, 2024 • edited

hawkw commented Feb 6, 2024

joshka commented Feb 6, 2024

hawkw commented Feb 8, 2024

joshka commented Feb 15, 2024

joshka commented Apr 1, 2024

hi-rustin left a comment • edited

Choose a reason for hiding this comment

hi-rustin commented Apr 9, 2024

hawkw left a comment

Choose a reason for hiding this comment

joshka commented Apr 25, 2024

hds left a comment

Choose a reason for hiding this comment

hi-rustin left a comment

Choose a reason for hiding this comment

hi-rustin May 6, 2024

Choose a reason for hiding this comment

joshka May 6, 2024

Choose a reason for hiding this comment

hi-rustin May 8, 2024 • edited

Choose a reason for hiding this comment

hi-rustin left a comment

Choose a reason for hiding this comment

hi-rustin commented May 8, 2024

hi-rustin commented May 9, 2024

joshka commented May 9, 2024

hds commented May 9, 2024

joshka commented Feb 3, 2024 •

edited

hi-rustin left a comment •

edited

hi-rustin May 8, 2024 •

edited