Fix substring bug

When one directory was a substring of another the directory would appear
as a subdirectory instead of a sibling

Fixed originally by this: 6e03dd77e6
Broken by this: db6c8a019d

Added integration test as this has bitten me before

src/main.rs

@@ -2,6 +2,7 @@
 extern crate clap;
 
 use self::display::draw_it;
+use crate::utils::is_a_parent_of;
 use clap::{App, AppSettings, Arg};
 use utils::{find_big_ones, get_dir_tree, simplify_dir_names, sort, trim_deep_ones, Node};
 
@@ -106,7 +107,6 @@ fn main() {
         }
     };
     let tree = build_tree(biggest_ones, depth);
-    //println!("{:?}", tree);
 
     draw_it(
         permissions,
@@ -126,7 +126,8 @@ fn build_tree(biggest_ones: Vec<(String, u64)>, depth: Option<u64>) -> Node {
             size: b.1,
             children: Vec::default(),
         };
-        recursively_build_tree(&mut top_parent, n, depth)
+        recursively_build_tree(&mut top_parent, n, depth);
+        top_parent.children.sort_unstable()
     }
     top_parent
 }
@@ -140,7 +141,7 @@ fn recursively_build_tree(parent_node: &mut Node, new_node: Node, depth: Option<
     if let Some(c) = parent_node
         .children
         .iter_mut()
-        .find(|c| new_node.name.starts_with(&c.name))
+        .find(|c| is_a_parent_of(&c.name, &new_node.name))
     {
         recursively_build_tree(c, new_node, new_depth);
     } else {

src/test_dir2/dir/hello

@@ -0,0 +1 @@
+hello

src/test_dir2/dir_name_clash

@@ -0,0 +1 @@
+hello

src/test_dir2/dir_substring/hello

@@ -0,0 +1 @@
+hello

src/tests.rs

@@ -138,7 +138,7 @@ pub fn test_d_flag_works() {
         .unwrap();
 }
 
-fn build_temp_file(dir: &TempDir) -> (PathBuf) {
+fn build_temp_file(dir: &TempDir) -> PathBuf {
     let file_path = dir.path().join("notes.txt");
     let mut file = File::create(&file_path).unwrap();
     writeln!(file, "I am a temp file").unwrap();
@@ -164,7 +164,7 @@ pub fn test_soft_sym_link() {
     let r = soft_sym_link_output(dir_s, file_path_s, link_name_s);
 
     // We cannot guarantee which version will appear first.
-    // TODO: Consider adding predictable itteration order (sort file entries by name?)
+    // TODO: Consider adding predictable iteration order (sort file entries by name?)
     assert_cli::Assert::main_binary()
         .with_args(&[dir_s])
         .stdout()
@@ -308,3 +308,45 @@ fn recursive_sym_link_output(dir: &str, link_name: &str) -> String {
         format_string(link_name, true, true, "   0B", " └──",),
     )
 }
+
+// Check against directories and files whos names are substrings of each other
+#[test]
+#[cfg(target_os = "macos")]
+pub fn test_substring_of_names() {
+    assert_cli::Assert::main_binary()
+        .with_args(&["src/test_dir2"])
+        .stdout()
+        .contains(" ─┬ test_dir2")
+        .stdout()
+        .contains("  ├─┬ dir")
+        .stdout()
+        .contains("  │ └── hello")
+        .stdout()
+        .contains("  ├── dir_name_clash")
+        .stdout()
+        .contains("  └─┬ dir_substring")
+        .stdout()
+        .contains("    └── hello")
+        .unwrap();
+}
+
+// Check against directories and files whos names are substrings of each other
+#[test]
+#[cfg(target_os = "linux")]
+pub fn test_substring_of_names() {
+    assert_cli::Assert::main_binary()
+        .with_args(&["src/test_dir2"])
+        .stdout()
+        .contains(" ─┬ test_dir2")
+        .stdout()
+        .contains("  ├─┬ dir")
+        .stdout()
+        .contains("  │ └── hello")
+        .stdout()
+        .contains("  ├─┬ dir_substring")
+        .stdout()
+        .contains("  │ └── hello")
+        .stdout()
+        .contains("  └── dir_name_clash")
+        .unwrap();
+}

src/utils/mod.rs

@@ -7,26 +7,52 @@ use jwalk::WalkDir;
 mod platform;
 use self::platform::*;
 
-#[derive(Debug, Default)]
+#[derive(Debug, Default, Eq)]
 pub struct Node {
     pub name: String,
     pub size: u64,
     pub children: Vec<Node>,
 }
 
+impl Ord for Node {
+    fn cmp(&self, other: &Self) -> Ordering {
+        if self.size == other.size {
+            self.name.cmp(&other.name)
+        } else {
+            self.size.cmp(&other.size)
+        }
+    }
+}
+
+impl PartialOrd for Node {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl PartialEq for Node {
+    fn eq(&self, other: &Self) -> bool {
+        self.name == other.name && self.size == other.size && self.children == other.children
+    }
+}
+
+pub fn is_a_parent_of(parent: &str, child: &str) -> bool {
+    child.starts_with(parent) && child.chars().nth(parent.chars().count()) == Some('/')
+}
+
 pub fn simplify_dir_names(filenames: Vec<&str>) -> HashSet<String> {
     let mut top_level_names: HashSet<String> = HashSet::with_capacity(filenames.len());
     let mut to_remove: Vec<String> = Vec::with_capacity(filenames.len());
 
     for t in filenames {
-        let top_level_name = ensure_end_slash(t);
+        let top_level_name = strip_end_slash(t);
         let mut can_add = true;
 
         for tt in top_level_names.iter() {
-            if top_level_name.starts_with(tt) {
-                can_add = false;
-            } else if tt.starts_with(&top_level_name) {
+            if is_a_parent_of(&top_level_name, tt) {
                 to_remove.push(tt.to_string());
+            } else if is_a_parent_of(tt, &top_level_name) {
+                can_add = false;
             }
         }
         to_remove.sort_unstable();
@@ -62,14 +88,6 @@ pub fn get_dir_tree(
     (permissions == 0, data)
 }
 
-pub fn ensure_end_slash(s: &str) -> String {
-    let mut new_name = String::from(s);
-    while new_name.ends_with('/') || new_name.ends_with("/.") {
-        new_name.pop();
-    }
-    new_name + "/"
-}
-
 pub fn strip_end_slash(mut new_name: &str) -> &str {
     while (new_name.ends_with('/') || new_name.ends_with("/.")) && new_name.len() > 1 {
         new_name = &new_name[..new_name.len() - 1];
@@ -206,4 +224,12 @@ mod tests {
         correct.insert("src".to_string());
         assert_eq!(simplify_dir_names(vec!["src/."]), correct);
     }
+
+    #[test]
+    fn test_simplify_dir_substring_names() {
+        let mut correct = HashSet::new();
+        correct.insert("src".to_string());
+        correct.insert("src_v2".to_string());
+        assert_eq!(simplify_dir_names(vec!["src/", "src_v2"]), correct);
+    }
 }