Review improvements + unittest of sorting.

4 files changed, 87 insertions, 15 deletions

diff --git a/src/batch.rs b/src/batch.rs
index b203932..2b576a6 100644
--- a/src/batch.rs
+++ b/src/batch.rs
@@ -30,8 +30,8 @@ impl Batch {
         self.0.len()
     }
 
-    pub fn sort(&mut self, sort: (SortFlag, SortOrder)) {
-        self.0.sort_unstable_by(|a, b| sort_by_meta(a, b, sort));
+    pub fn sort(&mut self, flags: Flags) {
+        self.0.sort_unstable_by(|a, b| sort_by_meta(a, b, flags));
     }
 
     pub fn get_short_output(&self, colors: &Colors, icons: &Icons, flags: Flags) -> Vec<String> {
@@ -138,9 +138,9 @@ impl Batch {
     }
 }
 
-fn sort_by_meta(a: &Meta, b: &Meta, (sort_flag, sort_order): (SortFlag, SortOrder)) -> Ordering {
-    let ord = match sort_flag {
-        SortFlag::Lexicographical => {
+fn sort_by_meta(a: &Meta, b: &Meta, flags: Flags) -> Ordering {
+    let ord = match flags.sort_by {
+        SortFlag::Name => {
             if a.file_type == FileType::Directory && b.file_type != FileType::Directory {
                 Ordering::Less
             } else if b.file_type == FileType::Directory && a.file_type != FileType::Directory {
@@ -152,8 +152,76 @@ fn sort_by_meta(a: &Meta, b: &Meta, (sort_flag, sort_order): (SortFlag, SortOrde
         // most recently modified first
         SortFlag::Time => b.date.cmp(&a.date).then(a.name.cmp(&b.name)),
     };
-    match sort_order {
+    match flags.sort_order {
         SortOrder::Default => ord,
         SortOrder::Reverse => ord.reverse(),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::fs::{create_dir, File};
+    use tempdir::TempDir;
+
+    #[test]
+    fn test_sort_by_meta() {
+        let tmp_dir = TempDir::new("test_dir").expect("failed to create temp dir");
+
+        // Create a file;
+        let path_a = tmp_dir.path().join("a.txt");
+        File::create(&path_a).expect("failed to create file");
+        let meta_a = Meta::from_path(&path_a).expect("failed to get meta");
+
+        // Create a dir;
+        let path_b = tmp_dir.path().join("b");
+        create_dir(&path_b).expect("failed to create dir");
+        let meta_b = Meta::from_path(&path_b).expect("failed to get meta");
+
+        // Sort by name
+        assert_eq!(
+            sort_by_meta(&meta_a, &meta_b, Flags::default()),
+            Ordering::Greater
+        );
+
+        // Sort by name reversed
+        assert_eq!(
+            sort_by_meta(
+                &meta_a,
+                &meta_b,
+                Flags {
+                    sort_order: SortOrder::Reverse,
+                    ..Flags::default()
+                }
+            ),
+            Ordering::Less
+        );
+
+        // Sort by time
+        assert_eq!(
+            sort_by_meta(
+                &meta_a,
+                &meta_b,
+                Flags {
+                    sort_by: SortFlag::Time,
+                    ..Flags::default()
+                }
+            ),
+            Ordering::Greater
+        );
+
+        // Sort by time reversed
+        assert_eq!(
+            sort_by_meta(
+                &meta_a,
+                &meta_b,
+                Flags {
+                    sort_by: SortFlag::Time,
+                    sort_order: SortOrder::Reverse,
+                    ..Flags::default()
+                }
+            ),
+            Ordering::Less
+        );
+    }
+}
diff --git a/src/core.rs b/src/core.rs
index 27fd9c4..2263d99 100644
--- a/src/core.rs
+++ b/src/core.rs
@@ -71,7 +71,7 @@ impl Core {
 
         if !files.is_empty() && !self.flags.display_tree {
             let mut file_batch = Batch::from(files);
-            file_batch.sort(self.flags.sort);
+            file_batch.sort(self.flags);
             self.display
                 .print_outputs(self.get_batch_outputs(&file_batch));
         }
@@ -163,7 +163,7 @@ impl Core {
         }
 
         let mut batch = Batch::from(metas);
-        batch.sort(self.flags.sort);
+        batch.sort(self.flags);
 
         Some(batch)
     }
diff --git a/src/flags.rs b/src/flags.rs
index 3e55a0e..9bc4af1 100644
--- a/src/flags.rs
+++ b/src/flags.rs
@@ -8,7 +8,8 @@ pub struct Flags {
     pub display_tree: bool,
     pub display_indicators: bool,
     pub recursive: bool,
-    pub sort: (SortFlag, SortOrder),
+    pub sort_by: SortFlag,
+    pub sort_order: SortOrder,
     pub date: DateFlag,
     pub color: WhenFlag,
     pub icon: WhenFlag,
@@ -20,10 +21,10 @@ impl<'a> From<ArgMatches<'a>> for Flags {
         let icon_inputs: Vec<&str> = matches.values_of("icon").unwrap().collect();
         let date_inputs: Vec<&str> = matches.values_of("date").unwrap().collect();
 
-        let sort_flag = if matches.is_present("timesort") {
+        let sort_by = if matches.is_present("timesort") {
             SortFlag::Time
         } else {
-            SortFlag::Lexicographical
+            SortFlag::Name
         };
         let sort_order = if matches.is_present("reverse") {
             SortOrder::Reverse
@@ -38,7 +39,8 @@ impl<'a> From<ArgMatches<'a>> for Flags {
             display_tree: matches.is_present("tree"),
             display_indicators: matches.is_present("indicators"),
             recursive: matches.is_present("recursive"),
-            sort: (sort_flag, sort_order),
+            sort_by,
+            sort_order,
             // Take only the last value
             date: DateFlag::from(date_inputs[date_inputs.len() - 1]),
             color: WhenFlag::from(color_inputs[color_inputs.len() - 1]),
@@ -56,7 +58,8 @@ impl Default for Flags {
             display_tree: false,
             display_indicators: false,
             recursive: false,
-            sort: (SortFlag::Lexicographical, SortOrder::Default),
+            sort_by: SortFlag::Name,
+            sort_order: SortOrder::Default,
             date: DateFlag::Date,
             color: WhenFlag::Auto,
             icon: WhenFlag::Auto,
@@ -100,7 +103,7 @@ impl<'a> From<&'a str> for WhenFlag {
 
 #[derive(Clone, Debug, Copy, PartialEq, Eq)]
 pub enum SortFlag {
-    Lexicographical,
+    Name,
     Time,
 }
 
diff --git a/src/meta/indicator.rs b/src/meta/indicator.rs
index 85698a2..ce5d340 100644
--- a/src/meta/indicator.rs
+++ b/src/meta/indicator.rs
@@ -76,7 +76,8 @@ mod test {
             display_tree: true,
             display_indicators: true,
             recursive: true,
-            sort: (SortFlag::Lexicographical, SortOrder::Default),
+            sort_by: SortFlag::Name,
+            sort_order: SortOrder::Default,
             date: DateFlag::Relative,
             color: WhenFlag::Always,
             icon: WhenFlag::Always,