fix: add one second to the live timebar_ratio now time to align percentages with expectations #10

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<i64>,
     #[clap(skip)]
-    last_reset: Option<DateTime<Local>>,
+    pub(crate) last_reset: Option<DateTime<Local>>,
 }
 
 #[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!(
+        #[cfg(debug_assertions)]
-            "data after update: {:#?}\nconsidered changed: {}",
+        if self.changed() {
-            self,
+            trace!("update with change: {:#?}", self);
-            self.changed()
+        }
-        );
     }
 
     /// 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];
+        self.fdate[0] != self.fdate[1] || self.ftime[0] != self.ftime[1]
-        trace!("changed: {r}");
-        r
     }
 
     #[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<f64> {
+    pub fn timebar_ratio(&self) -> Option<f64> {
         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<f64> {
+    pub(crate) fn timebar_ratio(&self, current_time: DateTime<Local>) -> Option<f64> {
         let len = self.timebar_len()?;
-        let since = (Local::now()
+        let since = current_time
             .signed_duration_since(self.last_reset.unwrap())
-            .num_seconds()
+            .num_seconds() as f64;
-            + 1) 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)?;
             }

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");
+}