From b65f1ba5e94e488123674702853f5e67db59a65d Mon Sep 17 00:00:00 2001 From: PlexSheep Date: Thu, 11 Jul 2024 16:16:25 +0200 Subject: [PATCH] fix: add one second to the live timebar_ratio now time to align percentages with expectations #10 --- src/clock.rs | 65 +++++++++++++++++++++++++++++++++------------------- src/main.rs | 38 ++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 24 deletions(-) diff --git a/src/clock.rs b/src/clock.rs index 6452cdf..220682e 100644 --- a/src/clock.rs +++ b/src/clock.rs @@ -1,12 +1,12 @@ #![warn(clippy::pedantic, clippy::style, clippy::nursery)] #![allow(clippy::question_mark_used)] -use chrono::{DateTime, Local, SubsecRound, Timelike}; +use chrono::{DateTime, Local, SubsecRound, TimeZone, Timelike}; use clap::Parser; use libpt::cli::args::HELP_TEMPLATE; use libpt::cli::clap::ArgGroup; use libpt::cli::{args::VerbosityLevel, clap}; -use libpt::log::{debug, trace}; +use libpt::log::{debug, info, trace}; use ratatui::backend::CrosstermBackend; use ratatui::crossterm::event::{self, poll, Event, KeyCode, KeyModifiers}; use ratatui::layout::{Alignment, Constraint, Direction, Layout, Rect}; @@ -64,7 +64,7 @@ pub struct Clock { #[clap(short, long)] pub custom: Option, #[clap(skip)] - last_reset: Option>, + pub(crate) last_reset: Option>, } #[derive(Debug, Clone, PartialEq, Default)] @@ -82,11 +82,10 @@ impl UiData { self.fdate[self.data_idx] = fdate; self.ftime[self.data_idx] = ftime; self.timebar_ratio[self.data_idx] = timebar_ratio; - trace!( - "data after update: {:#?}\nconsidered changed: {}", - self, - self.changed() - ); + #[cfg(debug_assertions)] + if self.changed() { + trace!("update with change: {:#?}", self); + } } /// did the data change with the last update? @@ -95,27 +94,25 @@ impl UiData { pub fn changed(&self) -> bool { // the timebar ratio is discarded, so that we only render the ui when the time // (second) changes - let r = self.fdate[0] != self.fdate[1] || self.ftime[0] != self.ftime[1]; - trace!("changed: {r}"); - r + self.fdate[0] != self.fdate[1] || self.ftime[0] != self.ftime[1] } #[must_use] #[inline] - pub(crate) fn fdate(&self) -> &str { + pub fn fdate(&self) -> &str { &self.fdate[self.data_idx] } #[must_use] #[inline] - pub(crate) fn ftime(&self) -> &str { + pub fn ftime(&self) -> &str { &self.ftime[self.data_idx] } #[must_use] #[inline] #[allow(clippy::missing_const_for_fn)] // no it's not const - pub(crate) fn timebar_ratio(&self) -> Option { + pub fn timebar_ratio(&self) -> Option { self.timebar_ratio[self.data_idx] } } @@ -136,22 +133,22 @@ impl Clock { } } - // FIXME: This generally works, but we want 0% at the start and 100% at the end, which does not - // fully work. We also want 50% at the half etc. #10 #[allow(clippy::cast_precision_loss)] // okay, good to know, but I accept the loss. It // shouldn't come to more than 2^52 seconds anyway - fn timebar_ratio(&self) -> Option { + pub(crate) fn timebar_ratio(&self, current_time: DateTime) -> Option { let len = self.timebar_len()?; - let since = (Local::now() + let since = current_time .signed_duration_since(self.last_reset.unwrap()) - .num_seconds() - + 1) as f64; + .num_seconds() as f64; + #[cfg(debug_assertions)] + if since < 1.0 { + trace!("ratio calculation since is now <1: {:#?}", since); + } Some((since / len.as_secs() as f64).clamp(0.0, 1.0)) } - fn maybe_reset_since_zero(&mut self) { + pub(crate) fn maybe_reset_since_zero(&mut self) { if let Some(len) = self.timebar_len() { - trace!("Local Time: {}", Local::now()); let since_last_reset = Local::now().signed_duration_since(self.last_reset.unwrap()); match len { TimeBarLength::Custom(_) => { @@ -217,7 +214,7 @@ impl Clock { } #[allow(clippy::unnecessary_wraps)] // we have that to be future proof - fn setup(&mut self) -> anyhow::Result<()> { + pub(crate) fn setup(&mut self) -> anyhow::Result<()> { self.setup_last_reset(); Ok(()) } @@ -239,7 +236,27 @@ impl Clock { .map(str::to_string) .collect(); - uidata.update(splits[0].clone(), splits[1].clone(), self.timebar_ratio()); + // We somehow fill timebar_ratio with a bad value here if we don't add 1 second. It's + // always the value that would be right for now-1s. The start of the minute is + // special, with this strategy it is 100%. #10 + // + // If we manually add a second, it works as expected, but it feels weird. We use the + // same time for all of the datapoints here, so it can't be because of time diff in + // calculation. I noticed that we don't start at 0% this way (with len=minute) + // . Normally, chrono does not include 60 seconds, only letting it range between 0 and + // 59. This makes sense but feels weird to the human understanding, of course there are + // seconds in a minute! If we do it this way, we don't quite start at 0%, but 100%, + // which feels correct. + // + // In short: if we add a second here, we get the correct percentages. 01:00 is 100%, + // 01:30 is 50%, 01:59 is 98%, 01:60 does not exist because that's how counting from + // 0 works. + + uidata.update( + splits[0].clone(), + splits[1].clone(), + self.timebar_ratio(raw_time + chrono::Duration::seconds(1)), + ); if uidata.changed() { self.ui(terminal, &uidata)?; } diff --git a/src/main.rs b/src/main.rs index 80091a1..065a46b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -29,6 +29,9 @@ fn main() -> anyhow::Result<()> { } debug!("set up logger"); + #[cfg(debug_assertions)] + mock_tests(); + debug!("taking over terminal"); // setup terminal enable_raw_mode()?; @@ -53,3 +56,38 @@ fn main() -> anyhow::Result<()> { debug!("done"); result } + +#[cfg(debug_assertions)] +#[allow(clippy::cast_precision_loss)] +fn mock_tests() { + use chrono::{Local, Timelike}; + use libpt::log::info; + + use self::clock::UiData; + info!("doing the mock tests"); + { + let mut c = Clock::parse_from(["some exec", "-mvvv"]); + let now = Local::now(); + c.last_reset = Some(now.with_second(0).unwrap()); + + assert_eq!(c.timebar_ratio(now.with_second(30).unwrap()), Some(0.5)); + info!("30s=0.5"); + assert_eq!( + c.timebar_ratio(now.with_second(59).unwrap()), + Some(0.9833333333333333) + ); + info!("60s=1.0"); + assert_eq!(c.timebar_ratio(now.with_second(0).unwrap()), Some(0.0)); + info!("0s=0.0"); + } + { + let mut data = UiData::default(); + data.update("date".to_owned(), "time".to_owned(), Some(0.1)); + assert_eq!(data.timebar_ratio(), Some(0.1)); + data.update("date".to_owned(), "time".to_owned(), Some(0.2)); + assert_eq!(data.timebar_ratio(), Some(0.2)); + data.update("date".to_owned(), "time".to_owned(), Some(0.3)); + assert_eq!(data.timebar_ratio(), Some(0.3)); + } + info!("finished the mock tests"); +}