fix(console): reduce decimal digits in UI

[hds]

The durations in the tokio-console UI are shown with a unit, so the
digits after the decimal separator are generally not relevant. Since
space is at a premium in a terminal UI, it makes sense to cut down where
possible.

This change removes all digits after the decimal separator in the tasks
table view. In the task detail view, 2 decimal places are kept.

Additionally, 4 new duration formats are added to represent
minutes-with-secondsi, hours-with-minutes, days-with-hours and days.
These are only applied once the leading unit is greater than zero.

For example, 59 seconds will be shown as seconds: 99s and then 60
seconds will be shown as minutes-with-seconds: 1m00s. New colors have
been chosen for each format.

NOTE: I had to make a small unrelated change in
task::Details::make_percentiles_widget to make clippy happy.

Fixes: #224

PR Notes (not to be commited)

Here are some screenshots showing the reduction in decimal digits. It also
shows the new time formats and colors.

Tasks table (all colors):

Task detail (all colors):

Tasks table (8 colors):

[hawkw]

I think this change makes sense overall --- I'm a fan of saving horizontal space in the tasks/resources list views, and i don't think that we need to display these values with as much precision as we do currently on those views.

I had some minor feedback on the implementation, most of which isn't a hard blocker. However, I would really prefer it if we could find a way to preserve the color gradients from smallest to longest durations --- I think that's a useful way of communicating this information visually. I'll be happy to merge this when that's addressed.

[hawkw]

it's probably worth documenting what the width value does here, and what the difference between passing None and Some (and None versus Some(0)) is?

[hds]

Style question here. An Option<usize> seemed like a good idea at the time, but None and Some(0) are equivalent. Do you have any opinion on the merits of just passing a bare usize instead?

[hawkw]

You could also use an Option<NonZeroUsize> i suppose (which has the same representation as a single usize, with the actual zero value representing None)...but this is an internal API, so misuse-resistent API design is less important. I definitely prefer None to indicate "no width limit" than 0, though --- when reading the code you might expect "0 width" to mean "no characters", i guess?

[hds]

Okay. That sounds like a better option (using NonZero). Internal or no, as you said, None to mean no width restriction makes more sense than 0. I'll document the function too, so that it's clear. Thanks!

[hawkw]

sounds good to me!

[hds]

So, this is a bit ugly because I now need this line when declaring DUR_LEN or I need to be unwraping in a bunch of places because new returns a Option<NonZeroUsize>.

// SAFETY: 6 is non-zero.
const DUR_LEN: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(6) };

https://github.com/tokio-rs/console/pull/402/files#diff-295fe639b204aaddc6b81e4c68fbb9bc7f2fb3aef43fbca622e99620dfc7debaR22-R23

Thoughts? Should I just go back to the Option<usize> and explain that Some(0) and None are equivalent?

[hawkw]

ugh, that's annoying. i would prefer not to put unsafe code in the const. at the callsite, we could call time_units(..., NonZeroUsize::new(DUR_LEN)) but that's more verbose. i think it would be fine to allow Some(0) and None to mean the same thing, or just get rid of the option...

[hds]

Changed it back to Option<usize>.

[hawkw]

i don't love that this is unsafe. note that NonZeroUsize::new is const fn in our MSRV, so I believe this could just be

Suggested change

	const DUR_LEN: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(6) };
	const DUR_LEN: NonZeroUsize = NonZeroUsize::new(6).expect("6 is non-zero");

alternatively, we could just get rid of the NonZeroUsize, which i would also be fine about.

[hds]

The unwrap wasn't const in our MSRV I believe.

I changed this back to Option<usize>.

[hds]

Yeah, I get the following when trying that code:

error: `std::option::Option::<T>::expect` is not yet stable as a const fn
  --> tokio-console/src/view/mod.rs:22:31
   |
22 | const DUR_LEN: NonZeroUsize = NonZeroUsize::new(6).expect("6 is non-zero");
   |        

So it seems like while NonZeroUsize::new() is stable, we would also need const Option::unwrap(), which isn't yet stable.

Another thing I've learnt today.

[hawkw]

looks good, thank you!