Some small improvements to time filter code after switch to jiff

CHANGELOG.md

@@ -10,7 +10,11 @@
 
 ## Changes
 
-- Replace `humantime` crate and `chrono` crate with `jiff` crate, see #1690 (@sorairolake)
+- Replace `humantime` crate and `chrono` crate with `jiff` crate, see #1690 (@sorairolake). This has some small changes to the
+  way dates given to options such `--changed-within` and `--changed-before` including:
+  - 'M' no longer means "month", as that could be confusing with minutes. Use "mo", "mos", "month" or "months" instead.
+  - month and year now account for variability in the calander rather than being a hard-coded number of seconds. That is probably
+    what you would expect, but it is a slight change in behavior.
 
 ## Other
 

src/filter/time.rs

@@ -1,6 +1,6 @@
 use jiff::{civil::DateTime, tz::TimeZone, Span, Timestamp, Zoned};
 
-use std::time::SystemTime;
+use std::time::{Duration, SystemTime, UNIX_EPOCH};
 
 /// Filter based on time ranges.
 #[derive(Debug, PartialEq, Eq)]
@@ -9,40 +9,49 @@ pub enum TimeFilter {
     After(SystemTime),
 }
 
+#[cfg(not(test))]
+fn now() -> Zoned {
+    Zoned::now()
+}
+
+#[cfg(test)]
+thread_local! {
+    static TESTTIME: std::cell::RefCell<Option<Zoned>> = None.into();
+}
+
+/// This allows us to set a specific time when running tests
+#[cfg(test)]
+fn now() -> Zoned {
+    TESTTIME.with_borrow(|reftime| reftime.as_ref().cloned().unwrap_or_else(Zoned::now))
+}
+
 impl TimeFilter {
-    fn from_str(ref_time: &SystemTime, s: &str) -> Option<SystemTime> {
-        s.parse::<Span>()
-            .and_then(|duration| {
-                Zoned::try_from(*ref_time).and_then(|zdt| zdt.checked_sub(duration))
-            })
-            .ok()
-            .or_else(|| {
-                let local_tz = TimeZone::system();
-                s.parse::<Timestamp>()
-                    .map(|ts| ts.to_zoned(TimeZone::UTC))
-                    .ok()
-                    .or_else(|| {
-                        s.parse::<DateTime>()
-                            .map(|dt| local_tz.to_ambiguous_zoned(dt))
-                            .and_then(|zdt| zdt.later())
-                            .ok()
-                    })
-                    .or_else(|| {
-                        let timestamp_secs = s.strip_prefix('@')?.parse().ok()?;
-                        Timestamp::from_second(timestamp_secs)
-                            .map(|ts| ts.to_zoned(TimeZone::UTC))
-                            .ok()
-                    })
-            })
-            .map(SystemTime::from)
+    fn from_str(s: &str) -> Option<SystemTime> {
+        if let Ok(span) = s.parse::<Span>() {
+            let datetime = now().checked_sub(span).ok()?;
+            Some(datetime.into())
+        } else if let Ok(timestamp) = s.parse::<Timestamp>() {
+            Some(timestamp.into())
+        } else if let Ok(datetime) = s.parse::<DateTime>() {
+            Some(
+                TimeZone::system()
+                    .to_ambiguous_zoned(datetime)
+                    .later()
+                    .ok()?
+                    .into(),
+            )
+        } else {
+            let timestamp_secs: u64 = s.strip_prefix('@')?.parse().ok()?;
+            Some(UNIX_EPOCH + Duration::from_secs(timestamp_secs))
+        }
     }
 
-    pub fn before(ref_time: &SystemTime, s: &str) -> Option<TimeFilter> {
-        TimeFilter::from_str(ref_time, s).map(TimeFilter::Before)
+    pub fn before(s: &str) -> Option<TimeFilter> {
+        TimeFilter::from_str(s).map(TimeFilter::Before)
     }
 
-    pub fn after(ref_time: &SystemTime, s: &str) -> Option<TimeFilter> {
-        TimeFilter::from_str(ref_time, s).map(TimeFilter::After)
+    pub fn after(s: &str) -> Option<TimeFilter> {
+        TimeFilter::from_str(s).map(TimeFilter::After)
     }
 
     pub fn applies_to(&self, t: &SystemTime) -> bool {
@@ -58,106 +67,115 @@ mod tests {
     use super::*;
     use std::time::Duration;
 
+    struct TestTime(SystemTime);
+
+    impl TestTime {
+        fn new(time: Zoned) -> Self {
+            TESTTIME.with_borrow_mut(|t| *t = Some(time.clone()));
+            TestTime(time.into())
+        }
+
+        fn set(&mut self, time: Zoned) {
+            TESTTIME.with_borrow_mut(|t| *t = Some(time.clone()));
+            self.0 = time.into();
+        }
+
+        fn timestamp(&self) -> SystemTime {
+            self.0
+        }
+    }
+
+    impl Drop for TestTime {
+        fn drop(&mut self) {
+            // Stop using manually set times
+            TESTTIME.with_borrow_mut(|t| *t = None);
+        }
+    }
+
     #[test]
     fn is_time_filter_applicable() {
         let local_tz = TimeZone::system();
-        let ref_time = local_tz
-            .to_ambiguous_zoned("2010-10-10 10:10:10".parse::<DateTime>().unwrap())
-            .later()
-            .unwrap()
-            .into();
+        let mut test_time = TestTime::new(
+            local_tz
+                .to_ambiguous_zoned("2010-10-10 10:10:10".parse::<DateTime>().unwrap())
+                .later()
+                .unwrap(),
+        );
+        let mut ref_time = test_time.timestamp();
 
-        assert!(TimeFilter::after(&ref_time, "1min")
-            .unwrap()
-            .applies_to(&ref_time));
-        assert!(!TimeFilter::before(&ref_time, "1min")
-            .unwrap()
-            .applies_to(&ref_time));
+        assert!(TimeFilter::after("1min").unwrap().applies_to(&ref_time));
+        assert!(!TimeFilter::before("1min").unwrap().applies_to(&ref_time));
 
         let t1m_ago = ref_time - Duration::from_secs(60);
-        assert!(!TimeFilter::after(&ref_time, "30sec")
-            .unwrap()
-            .applies_to(&t1m_ago));
-        assert!(TimeFilter::after(&ref_time, "2min")
-            .unwrap()
-            .applies_to(&t1m_ago));
+        assert!(!TimeFilter::after("30sec").unwrap().applies_to(&t1m_ago));
+        assert!(TimeFilter::after("2min").unwrap().applies_to(&t1m_ago));
 
-        assert!(TimeFilter::before(&ref_time, "30sec")
-            .unwrap()
-            .applies_to(&t1m_ago));
-        assert!(!TimeFilter::before(&ref_time, "2min")
-            .unwrap()
-            .applies_to(&t1m_ago));
+        assert!(TimeFilter::before("30sec").unwrap().applies_to(&t1m_ago));
+        assert!(!TimeFilter::before("2min").unwrap().applies_to(&t1m_ago));
 
         let t10s_before = "2010-10-10 10:10:00";
-        assert!(!TimeFilter::before(&ref_time, t10s_before)
+        assert!(!TimeFilter::before(t10s_before)
             .unwrap()
             .applies_to(&ref_time));
-        assert!(TimeFilter::before(&ref_time, t10s_before)
+        assert!(TimeFilter::before(t10s_before)
             .unwrap()
             .applies_to(&t1m_ago));
 
-        assert!(TimeFilter::after(&ref_time, t10s_before)
+        assert!(TimeFilter::after(t10s_before)
             .unwrap()
             .applies_to(&ref_time));
-        assert!(!TimeFilter::after(&ref_time, t10s_before)
-            .unwrap()
-            .applies_to(&t1m_ago));
+        assert!(!TimeFilter::after(t10s_before).unwrap().applies_to(&t1m_ago));
 
         let same_day = "2010-10-10";
-        assert!(!TimeFilter::before(&ref_time, same_day)
-            .unwrap()
-            .applies_to(&ref_time));
-        assert!(!TimeFilter::before(&ref_time, same_day)
-            .unwrap()
-            .applies_to(&t1m_ago));
-
-        assert!(TimeFilter::after(&ref_time, same_day)
-            .unwrap()
-            .applies_to(&ref_time));
-        assert!(TimeFilter::after(&ref_time, same_day)
-            .unwrap()
-            .applies_to(&t1m_ago));
-
-        let ref_time = "2010-10-10T10:10:10+00:00"
-            .parse::<Timestamp>()
-            .unwrap()
-            .into();
+        assert!(!TimeFilter::before(same_day).unwrap().applies_to(&ref_time));
+        assert!(!TimeFilter::before(same_day).unwrap().applies_to(&t1m_ago));
+
+        assert!(TimeFilter::after(same_day).unwrap().applies_to(&ref_time));
+        assert!(TimeFilter::after(same_day).unwrap().applies_to(&t1m_ago));
+
+        test_time.set(
+            "2010-10-10T10:10:10+00:00"
+                .parse::<Timestamp>()
+                .unwrap()
+                .to_zoned(local_tz.clone()),
+        );
+        ref_time = test_time.timestamp();
         let t1m_ago = ref_time - Duration::from_secs(60);
         let t10s_before = "2010-10-10T10:10:00+00:00";
-        assert!(!TimeFilter::before(&ref_time, t10s_before)
+        assert!(!TimeFilter::before(t10s_before)
             .unwrap()
             .applies_to(&ref_time));
-        assert!(TimeFilter::before(&ref_time, t10s_before)
+        assert!(TimeFilter::before(t10s_before)
             .unwrap()
             .applies_to(&t1m_ago));
 
-        assert!(TimeFilter::after(&ref_time, t10s_before)
+        assert!(TimeFilter::after(t10s_before)
             .unwrap()
             .applies_to(&ref_time));
-        assert!(!TimeFilter::after(&ref_time, t10s_before)
-            .unwrap()
-            .applies_to(&t1m_ago));
+        assert!(!TimeFilter::after(t10s_before).unwrap().applies_to(&t1m_ago));
 
         let ref_timestamp = 1707723412u64; // Mon Feb 12 07:36:52 UTC 2024
-        let ref_time = "2024-02-12T07:36:52+00:00"
-            .parse::<Timestamp>()
-            .unwrap()
-            .into();
+        test_time.set(
+            "2024-02-12T07:36:52+00:00"
+                .parse::<Timestamp>()
+                .unwrap()
+                .to_zoned(local_tz),
+        );
+        ref_time = test_time.timestamp();
         let t1m_ago = ref_time - Duration::from_secs(60);
         let t1s_later = ref_time + Duration::from_secs(1);
         // Timestamp only supported via '@' prefix
-        assert!(TimeFilter::before(&ref_time, &ref_timestamp.to_string()).is_none());
-        assert!(TimeFilter::before(&ref_time, &format!("@{ref_timestamp}"))
+        assert!(TimeFilter::before(&ref_timestamp.to_string()).is_none());
+        assert!(TimeFilter::before(&format!("@{ref_timestamp}"))
             .unwrap()
             .applies_to(&t1m_ago));
-        assert!(!TimeFilter::before(&ref_time, &format!("@{ref_timestamp}"))
+        assert!(!TimeFilter::before(&format!("@{ref_timestamp}"))
             .unwrap()
             .applies_to(&t1s_later));
-        assert!(!TimeFilter::after(&ref_time, &format!("@{ref_timestamp}"))
+        assert!(!TimeFilter::after(&format!("@{ref_timestamp}"))
             .unwrap()
             .applies_to(&t1m_ago));
-        assert!(TimeFilter::after(&ref_time, &format!("@{ref_timestamp}"))
+        assert!(TimeFilter::after(&format!("@{ref_timestamp}"))
             .unwrap()
             .applies_to(&t1s_later));
     }

src/main.rs

@@ -17,7 +17,6 @@ use std::env;
 use std::io::IsTerminal;
 use std::path::Path;
 use std::sync::Arc;
-use std::time;
 
 use anyhow::{anyhow, bail, Context, Result};
 use clap::{CommandFactory, Parser};
@@ -429,10 +428,9 @@ fn determine_ls_command(colored_output: bool) -> Result<Vec<&'static str>> {
 }
 
 fn extract_time_constraints(opts: &Opts) -> Result<Vec<TimeFilter>> {
-    let now = time::SystemTime::now();
     let mut time_constraints: Vec<TimeFilter> = Vec::new();
     if let Some(ref t) = opts.changed_within {
-        if let Some(f) = TimeFilter::after(&now, t) {
+        if let Some(f) = TimeFilter::after(t) {
             time_constraints.push(f);
         } else {
             return Err(anyhow!(
@@ -442,7 +440,7 @@ fn extract_time_constraints(opts: &Opts) -> Result<Vec<TimeFilter>> {
         }
     }
     if let Some(ref t) = opts.changed_before {
-        if let Some(f) = TimeFilter::before(&now, t) {
+        if let Some(f) = TimeFilter::before(t) {
             time_constraints.push(f);
         } else {
             return Err(anyhow!(