clean-up first implementation of modification date filter

sharkdp · Oct 10, 2018 · 574a16a · 574a16a
1 parent 3274169
commit 574a16a
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 59 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -41,7 +41,6 @@ regex = "1.0.0"
 regex-syntax = "0.6"
 ctrlc = "3.1"
 humantime = "1.1.1"
-if_chain = "0.1.3"
 
 [dependencies.clap]
 version = "2.31.2"

diff --git a/src/internal.rs b/src/internal.rs
@@ -108,23 +108,23 @@ pub enum TimeFilter {
 }
 
 impl TimeFilter {
-    fn from_str(s: &str) -> Option<SystemTime> {
+    fn from_str(ref_time: &SystemTime, s: &str) -> Option<SystemTime> {
         use humantime;
         humantime::parse_duration(s)
-            .map(|duration| SystemTime::now() - duration)
+            .map(|duration| *ref_time - duration)
             .or_else(|_| humantime::parse_rfc3339_weak(s))
             .ok()
     }
 
-    pub fn before(s: &str) -> Option<TimeFilter> {
-        Some(TimeFilter::Before(TimeFilter::from_str(s)?))
+    pub fn before(ref_time: &SystemTime, s: &str) -> Option<TimeFilter> {
+        TimeFilter::from_str(ref_time, s).map(TimeFilter::Before)
     }
 
-    pub fn after(s: &str) -> Option<TimeFilter> {
-        Some(TimeFilter::After(TimeFilter::from_str(s)?))
+    pub fn after(ref_time: &SystemTime, s: &str) -> Option<TimeFilter> {
+        TimeFilter::from_str(ref_time, s).map(TimeFilter::After)
     }
 
-    pub fn is_within(&self, t: &SystemTime) -> bool {
+    pub fn applies_to(&self, t: &SystemTime) -> bool {
         match self {
             TimeFilter::Before(limit) => t <= limit,
             TimeFilter::After(limit) => t >= limit,
@@ -196,7 +196,7 @@ pub struct FdOptions {
     pub size_constraints: Vec<SizeFilter>,
 
     /// Constraints on last modification time of files
-    pub modification_constraints: Vec<TimeFilter>,
+    pub time_constraints: Vec<TimeFilter>,
 }
 
 /// Print error message to stderr.
@@ -505,16 +505,65 @@ mod tests {
     }
 
     #[test]
-    fn is_time_within() {
-        let now = SystemTime::now();
-        assert!(TimeFilter::after("1min").unwrap().is_within(&now));
-        assert!(!TimeFilter::before("1min").unwrap().is_within(&now));
-
-        let t1m_ago = SystemTime::now() - time::Duration::from_secs(60);
-        assert!(!TimeFilter::after("30sec").unwrap().is_within(&t1m_ago));
-        assert!(TimeFilter::after("2min").unwrap().is_within(&t1m_ago));
+    fn is_time_filter_applicable() {
+        use humantime;
 
-        assert!(TimeFilter::before("30sec").unwrap().is_within(&t1m_ago));
-        assert!(!TimeFilter::before("2min").unwrap().is_within(&t1m_ago));
+        let ref_time = humantime::parse_rfc3339("2010-10-10T10:10:10Z").unwrap();
+        assert!(
+            TimeFilter::after(&ref_time, "1min")
+                .unwrap()
+                .applies_to(&ref_time)
+        );
+        assert!(
+            !TimeFilter::before(&ref_time, "1min")
+                .unwrap()
+                .applies_to(&ref_time)
+        );
+
+        let t1m_ago = ref_time - 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::before(&ref_time, "30sec")
+                .unwrap()
+                .applies_to(&t1m_ago)
+        );
+        assert!(
+            !TimeFilter::before(&ref_time, "2min")
+                .unwrap()
+                .applies_to(&t1m_ago)
+        );
+
+        let t10s_before = "2010-10-10 10:10:00";
+        assert!(
+            !TimeFilter::before(&ref_time, t10s_before)
+                .unwrap()
+                .applies_to(&ref_time)
+        );
+        assert!(
+            TimeFilter::before(&ref_time, t10s_before)
+                .unwrap()
+                .applies_to(&t1m_ago)
+        );
+
+        assert!(
+            TimeFilter::after(&ref_time, t10s_before)
+                .unwrap()
+                .applies_to(&ref_time)
+        );
+        assert!(
+            !TimeFilter::after(&ref_time, t10s_before)
+                .unwrap()
+                .applies_to(&t1m_ago)
+        );
     }
 }
diff --git a/src/main.rs b/src/main.rs
@@ -19,8 +19,6 @@ extern crate libc;
 extern crate num_cpus;
 extern crate regex;
 extern crate regex_syntax;
-#[macro_use]
-extern crate if_chain;
 
 mod app;
 mod exec;
@@ -153,17 +151,18 @@ fn main() {
         })
         .unwrap_or_else(|| vec![]);
 
-    let mut modification_constraints: Vec<TimeFilter> = Vec::new();
+    let now = time::SystemTime::now();
+    let mut time_constraints: Vec<TimeFilter> = Vec::new();
     if let Some(t) = matches.value_of("changed-within") {
-        if let Some(f) = TimeFilter::after(t) {
-            modification_constraints.push(f);
+        if let Some(f) = TimeFilter::after(&now, t) {
+            time_constraints.push(f);
         } else {
             print_error_and_exit(&format!("Error: {} is not a valid time.", t));
         }
     }
     if let Some(t) = matches.value_of("changed-before") {
-        if let Some(f) = TimeFilter::before(t) {
-            modification_constraints.push(f);
+        if let Some(f) = TimeFilter::before(&now, t) {
+            time_constraints.push(f);
         } else {
             print_error_and_exit(&format!("Error: {} is not a valid time.", t));
         }
@@ -245,7 +244,7 @@ fn main() {
             .map(|vs| vs.map(PathBuf::from).collect())
             .unwrap_or_else(|| vec![]),
         size_constraints: size_limits,
-        modification_constraints,
+        time_constraints,
     };
 
     match RegexBuilder::new(&pattern_regex)

diff --git a/src/walk.rs b/src/walk.rs
@@ -290,18 +290,21 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<FdOptions>) {
             }
 
             // Filter out unwanted modification times
-            if !config.modification_constraints.is_empty() {
-                if_chain!{
-                    if entry_path.is_file();
-                    if let Ok(metadata) = entry_path.metadata();
-                    if let Ok(modified) = metadata.modified();
-                    if config.modification_constraints.iter().all(|tf| tf.is_within(&modified));
-                    then {
-                        // When all is good, we just continue with pattern match
-                    } else {
-                        return ignore::WalkState::Continue;
+            if !config.time_constraints.is_empty() {
+                let mut matched = false;
+                if entry_path.is_file() {
+                    if let Ok(metadata) = entry_path.metadata() {
+                        if let Ok(modified) = metadata.modified() {
+                            matched = config
+                                .time_constraints
+                                .iter()
+                                .all(|tf| tf.applies_to(&modified));
+                        }
                     }
                 }
+                if !matched {
+                    return ignore::WalkState::Continue;
+                }
             }
 
             let search_str_o = if config.search_full_path {

diff --git a/tests/tests.rs b/tests/tests.rs
@@ -1125,31 +1125,32 @@ fn create_file_with_modified<P: AsRef<Path>>(path: P, duration_in_secs: u64) {
 #[test]
 fn test_modified_relative() {
     let te = TestEnv::new(&[], &[]);
-    create_file_with_modified(te.test_root().join("0_now"), 0);
-    create_file_with_modified(te.test_root().join("1_min"), 60);
-    create_file_with_modified(te.test_root().join("10_min"), 600);
-    create_file_with_modified(te.test_root().join("1_h"), 60 * 60);
-    create_file_with_modified(te.test_root().join("2_h"), 2 * 60 * 60);
-    create_file_with_modified(te.test_root().join("1_day"), 24 * 60 * 60);
+    create_file_with_modified(te.test_root().join("foo_0_now"), 0);
+    create_file_with_modified(te.test_root().join("bar_1_min"), 60);
+    create_file_with_modified(te.test_root().join("foo_10_min"), 600);
+    create_file_with_modified(te.test_root().join("bar_1_h"), 60 * 60);
+    create_file_with_modified(te.test_root().join("foo_2_h"), 2 * 60 * 60);
+    create_file_with_modified(te.test_root().join("bar_1_day"), 24 * 60 * 60);
 
     te.assert_output(
         &["", "--changed-within", "15min"],
-        "0_now
-        1_min
-        10_min",
+        "foo_0_now
+        bar_1_min
+        foo_10_min",
     );
 
     te.assert_output(
         &["", "--changed-before", "15min"],
-        "1_h
-        2_h
-        1_day",
+        "bar_1_h
+        foo_2_h
+        bar_1_day",
     );
 
     te.assert_output(
-        &["min", "--changed-within", "12h"],
-        "1_min
-        10_min",
+        &["foo", "--changed-within", "12h"],
+        "foo_0_now
+        foo_10_min
+        foo_2_h",
     );
 }