From adff43fbb4ce7b0663759d445a944286583acaf4 Mon Sep 17 00:00:00 2001
From: Marc Tiehuis <marctiehuis@gmail.com>
Date: Wed, 1 Mar 2017 18:38:06 +1300
Subject: [PATCH] Remove clap validator + add max-filesize integration tests

---
 build.rs         |  1 -
 src/app.rs       | 25 +------------------------
 src/args.rs      | 16 +++++++---------
 tests/tests.rs   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 tests/workdir.rs |  7 +++++++
 5 files changed, 60 insertions(+), 34 deletions(-)

diff --git a/build.rs b/build.rs
index 4d0e2d13..8a7c4900 100644
--- a/build.rs
+++ b/build.rs
@@ -2,7 +2,6 @@
 extern crate clap;
 #[macro_use]
 extern crate lazy_static;
-extern crate regex;
 
 use std::env;
 use std::fs;
diff --git a/src/app.rs b/src/app.rs
index 6d5177c4..e2f4b010 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -1,7 +1,6 @@
 use std::collections::HashMap;
 
 use clap::{App, AppSettings, Arg, ArgSettings};
-use regex::Regex;
 
 const ABOUT: &'static str = "
 ripgrep (rg) recursively searches your current directory for a regex pattern.
@@ -147,8 +146,7 @@ fn app<F>(next_line_help: bool, doc: F) -> App<'static, 'static>
              .short("m").value_name("NUM").takes_value(true)
              .validator(validate_number))
         .arg(flag("max-filesize")
-             .value_name("NUM+SUFFIX?").takes_value(true)
-             .validator(validate_max_filesize))
+             .value_name("NUM+SUFFIX?").takes_value(true))
         .arg(flag("maxdepth")
              .value_name("NUM").takes_value(true)
              .validator(validate_number))
@@ -502,24 +500,3 @@ lazy_static! {
 fn validate_number(s: String) -> Result<(), String> {
     s.parse::<usize>().map(|_|()).map_err(|err| err.to_string())
 }
-
-fn validate_max_filesize(s: String) -> Result<(), String> {
-    let re = Regex::new(r#"^(\d+)([KMG])?$"#).unwrap();
-    let caps = try!(re.captures(&s)
-                      .ok_or("invalid format for max-filesize argument"));
-
-    let value = caps.get(1);
-    let suffix = caps.get(2).map(|x| x.as_str());
-
-    match value {
-        Some(value) => {
-            try!(value.as_str().parse::<u64>().map_err(|err| err.to_string()));
-        }
-        None => ()
-    }
-
-    match suffix {
-        None | Some("K") | Some("M") | Some("G") => Ok(()),
-        _ => Err(From::from("invalid suffix for max-filesize argument"))
-    }
-}
diff --git a/src/args.rs b/src/args.rs
index 5f6d9916..8cf7cd69 100644
--- a/src/args.rs
+++ b/src/args.rs
@@ -791,20 +791,18 @@ impl<'a> ArgMatches<'a> {
             None => return Ok(None)
         };
 
-        let re = Regex::new(r#"^(\d+)([KMG])?$"#).unwrap();
+        let re = Regex::new("^([0-9]+)([KMG])?$").unwrap();
         let caps = try!(re.captures(&max_filesize)
                           .ok_or("invalid format for max-filesize argument"));
 
-        let value = match caps.get(1) {
-            Some(value) => Some(try!(value.as_str().parse::<u64>())),
-            None => None
-        };
+        let value = try!(caps[1].parse::<u64>().map_err(|err| err.to_string()));
         let suffix = caps.get(2).map(|x| x.as_str());
+
         match suffix {
-            None      => Ok(value),
-            Some("K") => Ok(value.map(|x| x * 1024)),
-            Some("M") => Ok(value.map(|x| x * 1024 * 1024)),
-            Some("G") => Ok(value.map(|x| x * 1024 * 1024 * 1024)),
+            None      => Ok(Some(value)),
+            Some("K") => Ok(Some(value * 1024)),
+            Some("M") => Ok(Some(value * 1024 * 1024)),
+            Some("G") => Ok(Some(value * 1024 * 1024 * 1024)),
             _ => Err(From::from("invalid suffix for max-filesize argument"))
         }
     }
diff --git a/tests/tests.rs b/tests/tests.rs
index fd60d672..e21ea42f 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -444,6 +444,51 @@ sherlock!(max_filesize_parse_error_suffix, "Sherlock", ".",
     wd.assert_err(&mut cmd);
 });
 
+sherlock!(max_filesize_parse_no_suffix, "Sherlock", ".",
+|wd: WorkDir, mut cmd: Command| {
+    wd.remove("sherlock");
+    wd.create_size("foo", 40);
+    wd.create_size("bar", 60);
+
+    cmd.arg("--max-filesize").arg("50").arg("--files");
+    let lines: String = wd.stdout(&mut cmd);
+    let expected = "\
+foo
+";
+
+    assert_eq!(lines, expected);
+});
+
+sherlock!(max_filesize_parse_k_suffix, "Sherlock", ".",
+|wd: WorkDir, mut cmd: Command| {
+    wd.remove("sherlock");
+    wd.create_size("foo", 3048);
+    wd.create_size("bar", 4100);
+
+    cmd.arg("--max-filesize").arg("4K").arg("--files");
+    let lines: String = wd.stdout(&mut cmd);
+    let expected = "\
+foo
+";
+
+    assert_eq!(lines, expected);
+});
+
+sherlock!(max_filesize_parse_m_suffix, "Sherlock", ".",
+|wd: WorkDir, mut cmd: Command| {
+    wd.remove("sherlock");
+    wd.create_size("foo", 1000000);
+    wd.create_size("bar", 1400000);
+
+    cmd.arg("--max-filesize").arg("1M").arg("--files");
+    let lines: String = wd.stdout(&mut cmd);
+    let expected = "\
+foo
+";
+
+    assert_eq!(lines, expected);
+});
+
 sherlock!(ignore_hidden, "Sherlock", ".", |wd: WorkDir, mut cmd: Command| {
     wd.remove("sherlock");
     wd.create(".sherlock", hay::SHERLOCK);
diff --git a/tests/workdir.rs b/tests/workdir.rs
index 7ae56d2c..a5fe9e54 100644
--- a/tests/workdir.rs
+++ b/tests/workdir.rs
@@ -46,6 +46,13 @@ impl WorkDir {
         self.create_bytes(name, contents.as_bytes());
     }
 
+    /// Create a new file with the given name and size.
+    pub fn create_size<P: AsRef<Path>>(&self, name: P, filesize: u64) {
+        let path = self.dir.join(name);
+        let file = nice_err(&path, File::create(&path));
+        nice_err(&path, file.set_len(filesize));
+    }
+
     /// Create a new file with the given name and contents in this directory.
     pub fn create_bytes<P: AsRef<Path>>(&self, name: P, contents: &[u8]) {
         let path = self.dir.join(name);