diff --git a/src/dir_walker.rs b/src/dir_walker.rs index f3b7345..b958237 100644 --- a/src/dir_walker.rs +++ b/src/dir_walker.rs @@ -34,14 +34,11 @@ pub fn walk_it(dirs: HashSet, walk_data: WalkData) -> (Vec, bool) let top_level_nodes: Vec<_> = dirs .into_iter() .filter_map(|d| { - let n = walk(d, &permissions_flag, &walk_data, 0); - match n { - Some(n) => { - let mut inodes: HashSet<(u64, u64)> = HashSet::new(); - clean_inodes(n, &mut inodes, walk_data.use_apparent_size) - } - None => None, - } + clean_inodes( + walk(d, &permissions_flag, &walk_data, 0)?, + &mut HashSet::new(), + walk_data.use_apparent_size, + ) }) .collect(); (top_level_nodes, permissions_flag.into_inner()) @@ -55,10 +52,9 @@ fn clean_inodes( ) -> Option { if !use_apparent_size { if let Some(id) = x.inode_device { - if inodes.contains(&id) { + if !inodes.insert(id) { return None; } - inodes.insert(id); } } @@ -133,34 +129,35 @@ fn walk( ) -> Option { let mut children = vec![]; - if let Ok(entries) = fs::read_dir(dir.clone()) { + if let Ok(entries) = fs::read_dir(&dir) { children = entries .into_iter() .par_bridge() .filter_map(|entry| { if let Ok(ref entry) = entry { // uncommenting the below line gives simpler code but - // rayon doesn't parallelise as well giving a 3X performance drop + // rayon doesn't parallelize as well giving a 3X performance drop // hence we unravel the recursion a bit // return walk(entry.path(), permissions_flag, ignore_directories, allowed_filesystems, use_apparent_size, by_filecount, ignore_hidden); if !ignore_file(entry, walk_data) { if let Ok(data) = entry.file_type() { - if data.is_dir() && !data.is_symlink() { - return walk(entry.path(), permissions_flag, walk_data, depth + 1); - } - return build_node( - entry.path(), - vec![], - walk_data.filter_regex, - walk_data.invert_filter_regex, - walk_data.use_apparent_size, - data.is_symlink(), - data.is_file(), - walk_data.by_filecount, - depth, - ); + return if data.is_dir() && !data.is_symlink() { + walk(entry.path(), permissions_flag, walk_data, depth + 1) + } else { + build_node( + entry.path(), + vec![], + walk_data.filter_regex, + walk_data.invert_filter_regex, + walk_data.use_apparent_size, + data.is_symlink(), + data.is_file(), + walk_data.by_filecount, + depth, + ) + }; } } } else { @@ -204,24 +201,26 @@ mod tests { } #[test] + #[allow(clippy::redundant_clone)] fn test_should_ignore_file() { let mut inodes = HashSet::new(); let n = create_node(); // First time we insert the node - assert!(clean_inodes(n.clone(), &mut inodes, false) == Some(n.clone())); + assert_eq!(clean_inodes(n.clone(), &mut inodes, false), Some(n.clone())); // Second time is a duplicate - we ignore it - assert!(clean_inodes(n.clone(), &mut inodes, false) == None); + assert_eq!(clean_inodes(n.clone(), &mut inodes, false), None); } #[test] + #[allow(clippy::redundant_clone)] fn test_should_not_ignore_files_if_using_apparent_size() { let mut inodes = HashSet::new(); let n = create_node(); // If using apparent size we include Nodes, even if duplicate inodes - assert!(clean_inodes(n.clone(), &mut inodes, true) == Some(n.clone())); - assert!(clean_inodes(n.clone(), &mut inodes, true) == Some(n.clone())); + assert_eq!(clean_inodes(n.clone(), &mut inodes, true), Some(n.clone())); + assert_eq!(clean_inodes(n.clone(), &mut inodes, true), Some(n.clone())); } } diff --git a/src/display.rs b/src/display.rs index 0d4cbd6..2544910 100644 --- a/src/display.rs +++ b/src/display.rs @@ -22,7 +22,7 @@ static BLOCKS: [char; 5] = ['█', '▓', '▒', '░', ' ']; pub struct DisplayData { pub short_paths: bool, pub is_reversed: bool, - pub colors_on: bool, + pub colors: ColorState, pub by_filecount: bool, pub num_chars_needed_on_left_most: usize, pub base_size: u64, @@ -106,26 +106,40 @@ impl DrawData<'_> { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ColorState { + Disabled, + Enabled, +} + +impl ColorState { + #[inline] + #[must_use] + fn enabled(self) -> bool { + self == ColorState::Enabled + } +} + #[allow(clippy::too_many_arguments)] pub fn draw_it( use_full_path: bool, is_reversed: bool, - no_colors: bool, + colors: ColorState, no_percents: bool, terminal_width: usize, by_filecount: bool, - root_node: DisplayNode, + root_node: &DisplayNode, iso: bool, skip_total: bool, ) { - let biggest = if skip_total { - root_node + let biggest = match skip_total { + false => root_node, + true => root_node .get_children_from_node(false) .next() - .unwrap_or_else(|| root_node.clone()) - } else { - root_node.clone() + .unwrap_or(root_node), }; + let num_chars_needed_on_left_most = if by_filecount { let max_size = biggest.size; max_size.separate_with_commas().chars().count() @@ -141,7 +155,7 @@ pub fn draw_it( let allowed_width = terminal_width - num_chars_needed_on_left_most - 2; let num_indent_chars = 3; let longest_string_length = - find_longest_dir_name(&root_node, num_indent_chars, allowed_width, !use_full_path); + find_longest_dir_name(root_node, num_indent_chars, allowed_width, !use_full_path); let max_bar_length = if no_percents || longest_string_length + 7 >= allowed_width as usize { 0 @@ -149,12 +163,12 @@ pub fn draw_it( allowed_width as usize - longest_string_length - 7 }; - let first_size_bar = repeat(BLOCKS[0]).take(max_bar_length).collect::(); + let first_size_bar = repeat(BLOCKS[0]).take(max_bar_length).collect(); let display_data = DisplayData { short_paths: !use_full_path, is_reversed, - colors_on: !no_colors, + colors, by_filecount, num_chars_needed_on_left_most, base_size: biggest.size, @@ -209,14 +223,14 @@ fn find_longest_dir_name( .fold(longest, max) } -fn display_node(node: DisplayNode, draw_data: &DrawData, is_biggest: bool, is_last: bool) { +fn display_node(node: &DisplayNode, draw_data: &DrawData, is_biggest: bool, is_last: bool) { // hacky way of working out how deep we are in the tree let indent = draw_data.get_new_indent(!node.children.is_empty(), is_last); let level = ((indent.chars().count() - 1) / 2) - 1; - let bar_text = draw_data.generate_bar(&node, level); + let bar_text = draw_data.generate_bar(node, level); let to_print = format_string( - &node, + node, &*indent, &*bar_text, is_biggest, @@ -359,7 +373,7 @@ fn get_pretty_size(node: &DisplayNode, is_biggest: bool, display_data: &DisplayD let spaces_to_add = display_data.num_chars_needed_on_left_most - output.chars().count(); let output = " ".repeat(spaces_to_add) + output.as_str(); - if is_biggest && display_data.colors_on { + if is_biggest && display_data.colors.enabled() { format!("{}", Red.paint(output)) } else { output @@ -371,11 +385,11 @@ fn get_pretty_name( name_and_padding: String, display_data: &DisplayData, ) -> String { - if display_data.colors_on { - let meta_result = fs::metadata(node.name.clone()); + if display_data.colors.enabled() { + let meta_result = fs::metadata(&node.name); let directory_color = display_data .ls_colors - .style_for_path_with_metadata(node.name.clone(), meta_result.as_ref().ok()); + .style_for_path_with_metadata(&node.name, meta_result.as_ref().ok()); let ansi_style = directory_color .map(Style::to_ansi_term_style) .unwrap_or_default(); @@ -411,7 +425,7 @@ mod tests { DisplayData { short_paths: true, is_reversed: false, - colors_on: false, + colors: ColorState::Disabled, by_filecount: false, num_chars_needed_on_left_most: 5, base_size: 1, diff --git a/src/display_node.rs b/src/display_node.rs index d26b074..8c72964 100644 --- a/src/display_node.rs +++ b/src/display_node.rs @@ -1,46 +1,24 @@ -use std::cmp::Ordering; use std::path::PathBuf; -#[derive(Debug, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] pub struct DisplayNode { - pub name: PathBuf, //todo: consider moving to a string? + // Note: the order of fields in important here, for PartialEq and PartialOrd pub size: u64, + pub name: PathBuf, //todo: consider moving to a string? pub children: Vec, } -impl Ord for DisplayNode { - 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 DisplayNode { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl PartialEq for DisplayNode { - fn eq(&self, other: &Self) -> bool { - self.name == other.name && self.size == other.size && self.children == other.children - } -} - impl DisplayNode { pub fn num_siblings(&self) -> u64 { self.children.len() as u64 } - pub fn get_children_from_node(&self, is_reversed: bool) -> impl Iterator { + pub fn get_children_from_node(&self, is_reversed: bool) -> impl Iterator { // we box to avoid the clippy lint warning - let out: Box> = if is_reversed { - Box::new(self.children.clone().into_iter().rev()) + let out: Box> = if is_reversed { + Box::new(self.children.iter().rev()) } else { - Box::new(self.children.clone().into_iter()) + Box::new(self.children.iter()) }; out } diff --git a/src/filter.rs b/src/filter.rs index 0c801e6..59ee725 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -3,6 +3,8 @@ use crate::node::Node; use std::collections::BinaryHeap; use std::collections::HashMap; use std::collections::HashSet; +use std::ffi::OsStr; +use std::path::Path; use std::path::PathBuf; pub fn get_biggest( @@ -21,14 +23,14 @@ pub fn get_biggest( let root = get_new_root(top_level_nodes); let mut allowed_nodes = HashSet::new(); - allowed_nodes.insert(&root.name); + allowed_nodes.insert(root.name.as_path()); heap = add_children(using_a_filter, &root, depth, heap); for _ in number_top_level_nodes..n { let line = heap.pop(); match line { Some(line) => { - allowed_nodes.insert(&line.name); + allowed_nodes.insert(line.name.as_path()); heap = add_children(using_a_filter, line, depth, heap); } None => break, @@ -37,33 +39,59 @@ pub fn get_biggest( recursive_rebuilder(&allowed_nodes, &root) } -pub fn get_all_file_types(top_level_nodes: Vec, n: usize) -> Option { - let mut map: HashMap = HashMap::new(); - build_by_all_file_types(top_level_nodes, &mut map); - let mut by_types: Vec = map.into_iter().map(|(_k, v)| v).collect(); - by_types.sort(); - by_types.reverse(); +#[derive(PartialEq, Eq, PartialOrd, Ord)] +struct ExtensionNode<'a> { + size: u64, + extension: Option<&'a OsStr>, +} - let displayed = if by_types.len() <= n { - by_types - } else { - let (displayed, rest) = by_types.split_at(if n > 1 { n - 1 } else { 1 }); - let remaining = DisplayNode { - name: PathBuf::from("(others)"), - size: rest.iter().map(|a| a.size).sum(), - children: vec![], - }; +pub fn get_all_file_types(top_level_nodes: &[Node], n: usize) -> Option { + let ext_nodes = { + let mut extension_cumulative_sizes = HashMap::new(); + build_by_all_file_types(top_level_nodes, &mut extension_cumulative_sizes); - let mut displayed = displayed.to_vec(); - displayed.push(remaining); - displayed + let mut extension_cumulative_sizes: Vec> = extension_cumulative_sizes + .iter() + .map(|(&extension, &size)| ExtensionNode { extension, size }) + .collect(); + + extension_cumulative_sizes.sort_by(|lhs, rhs| lhs.cmp(rhs).reverse()); + + extension_cumulative_sizes }; + let mut ext_nodes_iter = ext_nodes.iter(); + + // First, collect the first N - 1 nodes... + let mut displayed: Vec = ext_nodes_iter + .by_ref() + .take(if n > 1 { n - 1 } else { 1 }) + .map(|node| DisplayNode { + name: PathBuf::from( + node.extension + .map(|ext| format!(".{}", ext.to_string_lossy())) + .unwrap_or_else(|| "(no extension)".to_owned()), + ), + size: node.size, + children: vec![], + }) + .collect(); + + // ...then, aggregate the remaining nodes (if any) into a single "(others)" node + if ext_nodes_iter.len() > 0 { + displayed.push(DisplayNode { + name: PathBuf::from("(others)"), + size: ext_nodes_iter.map(|node| node.size).sum(), + children: vec![], + }); + } + let result = DisplayNode { name: PathBuf::from("(total)"), - size: displayed.iter().map(|a| a.size).sum(), + size: displayed.iter().map(|node| node.size).sum(), children: displayed, }; + Some(result) } @@ -74,44 +102,35 @@ fn add_children<'a>( mut heap: BinaryHeap<&'a Node>, ) -> BinaryHeap<&'a Node> { if depth > file_or_folder.depth { - if using_a_filter { - file_or_folder.children.iter().for_each(|c| { - if c.name.is_file() || c.size > 0 { - heap.push(c) - } - }); - } else { - file_or_folder.children.iter().for_each(|c| heap.push(c)); - } + heap.extend( + file_or_folder + .children + .iter() + .filter(|c| !using_a_filter || c.name.is_file() || c.size > 0), + ) } heap } -fn build_by_all_file_types(top_level_nodes: Vec, counter: &mut HashMap) { +fn build_by_all_file_types<'a>( + top_level_nodes: &'a [Node], + counter: &mut HashMap, u64>, +) { for node in top_level_nodes { if node.name.is_file() { let ext = node.name.extension(); - let key: String = match ext { - Some(e) => ".".to_string() + &e.to_string_lossy(), - None => "(no extension)".into(), - }; - let mut display_node = counter.entry(key.clone()).or_insert(DisplayNode { - name: PathBuf::from(key), - size: 0, - children: vec![], - }); - display_node.size += node.size; + let cumulative_size = counter.entry(ext).or_default(); + *cumulative_size += node.size; } - build_by_all_file_types(node.children, counter) + build_by_all_file_types(&node.children, counter) } } fn get_new_root(top_level_nodes: Vec) -> Node { if top_level_nodes.len() > 1 { - let total_size = top_level_nodes.iter().map(|node| node.size).sum(); Node { name: PathBuf::from("(total)"), - size: total_size, + size: top_level_nodes.iter().map(|node| node.size).sum(), children: top_level_nodes, inode_device: None, depth: 0, @@ -121,27 +140,19 @@ fn get_new_root(top_level_nodes: Vec) -> Node { } } -fn recursive_rebuilder<'a>( - allowed_nodes: &'a HashSet<&PathBuf>, - current: &Node, -) -> Option { +fn recursive_rebuilder(allowed_nodes: &HashSet<&Path>, current: &Node) -> Option { let mut new_children: Vec<_> = current .children .iter() - .filter_map(|c| { - if allowed_nodes.contains(&c.name) { - recursive_rebuilder(allowed_nodes, c) - } else { - None - } - }) + .filter(|c| allowed_nodes.contains(c.name.as_path())) + .filter_map(|c| recursive_rebuilder(allowed_nodes, c)) .collect(); - new_children.sort(); - new_children.reverse(); - let newnode = DisplayNode { + + new_children.sort_by(|lhs, rhs| lhs.cmp(rhs).reverse()); + + Some(DisplayNode { name: current.name.clone(), size: current.size, children: new_children, - }; - Some(newnode) + }) } diff --git a/src/main.rs b/src/main.rs index c11106c..0021b08 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,7 +6,7 @@ extern crate unicode_width; use std::collections::HashSet; use std::process; -use self::display::draw_it; +use self::display::{draw_it, ColorState}; use clap::{crate_version, Arg}; use clap::{Command, Values}; use dir_walker::{walk_it, WalkData}; @@ -29,73 +29,72 @@ mod utils; static DEFAULT_NUMBER_OF_LINES: usize = 30; static DEFAULT_TERMINAL_WIDTH: usize = 80; -#[cfg(windows)] -fn init_color(no_color: bool) -> bool { - // If no color is already set do not print a warning message - if no_color { - true - } else { +/// `ansi_term::enable_ansi_support` only exists on Windows; this wrapper +/// function makes it available on all platforms +#[inline] +fn enable_ansi_support() -> Result<(), i32> { + #[cfg(windows)] + { + ansi_term::enable_ansi_support() + } + + #[cfg(not(windows))] + { + Ok(()) + } +} + +fn init_color(color: ColorState) -> ColorState { + match color { + // If no color is already set do not print a warning message + ColorState::Disabled => ColorState::Disabled, + // Required for windows 10 // Fails to resolve for windows 8 so disable color - match ansi_term::enable_ansi_support() { - Ok(_) => no_color, + ColorState::Enabled => match enable_ansi_support() { + Ok(()) => ColorState::Enabled, Err(_) => { eprintln!( "This version of Windows does not support ANSI colors, setting no_color flag" ); - true + ColorState::Disabled } - } + }, } } -#[cfg(not(windows))] -fn init_color(no_color: bool) -> bool { - no_color -} - fn get_height_of_terminal() -> usize { - // Windows CI runners detect a terminal height of 0 - if let Some((Width(_w), Height(h))) = terminal_size() { - max(h as usize, DEFAULT_NUMBER_OF_LINES) - 10 - } else { - DEFAULT_NUMBER_OF_LINES - 10 - } + // Simplify once https://github.com/eminence/terminal-size/pull/41 is + // merged + terminal_size() + // Windows CI runners detect a terminal height of 0 + .map(|(_, Height(h))| max(h as usize, DEFAULT_NUMBER_OF_LINES)) + .unwrap_or(DEFAULT_NUMBER_OF_LINES) + - 10 } -#[cfg(windows)] fn get_width_of_terminal() -> usize { - // Windows CI runners detect a very low terminal width - if let Some((Width(w), Height(_h))) = terminal_size() { - max(w as usize, DEFAULT_TERMINAL_WIDTH) - } else { - DEFAULT_TERMINAL_WIDTH - } -} - -#[cfg(not(windows))] -fn get_width_of_terminal() -> usize { - if let Some((Width(w), Height(_h))) = terminal_size() { - w as usize - } else { - DEFAULT_TERMINAL_WIDTH - } + // Simplify once https://github.com/eminence/terminal-size/pull/41 is + // merged + terminal_size() + .map(|(Width(w), _)| match cfg!(windows) { + // Windows CI runners detect a very low terminal width + true => max(w as usize, DEFAULT_TERMINAL_WIDTH), + false => w as usize, + }) + .unwrap_or(DEFAULT_TERMINAL_WIDTH) } fn get_regex_value(maybe_value: Option) -> Vec { - let mut result = vec![]; - if let Some(v) = maybe_value { - for reg in v { - match Regex::new(reg) { - Ok(r) => result.push(r), - Err(e) => { - eprintln!("Ignoring bad value for regex {:?}", e); - process::exit(1); - } - } - } - } - result + maybe_value + .unwrap_or_default() + .map(|reg| { + Regex::new(reg).unwrap_or_else(|err| { + eprintln!("Ignoring bad value for regex {:?}", err); + process::exit(1) + }) + }) + .collect() } fn main() { @@ -235,18 +234,15 @@ fn main() { let filter_regexs = get_regex_value(options.values_of("filter")); let invert_filter_regexs = get_regex_value(options.values_of("invert_filter")); - let terminal_width = match options.value_of_t("width") { - Ok(v) => v, - Err(_) => get_width_of_terminal(), - }; + let terminal_width = options + .value_of_t("width") + .unwrap_or_else(|_| get_width_of_terminal()); + + let depth = options.value_of_t("depth").unwrap_or_else(|_| { + eprintln!("Ignoring bad value for depth"); + usize::MAX + }); - let depth = match options.value_of_t("depth") { - Ok(v) => v, - Err(_) => { - eprintln!("Ignoring bad value for depth"); - usize::MAX - } - }; // If depth is set we set the default number_of_lines to be max // instead of screen height let default_height = if depth != usize::MAX { @@ -255,40 +251,36 @@ fn main() { get_height_of_terminal() }; - let number_of_lines = match options.value_of("number_of_lines") { - Some(v) => match v.parse::() { - Ok(num_lines) => num_lines, - Err(_) => { - eprintln!("Ignoring bad value for number_of_lines"); - default_height - } - }, - None => default_height, - }; + let number_of_lines = options + .value_of("number_of_lines") + .and_then(|v| { + v.parse() + .map_err(|_| eprintln!("Ignoring bad value for number_of_lines")) + .ok() + }) + .unwrap_or(default_height); - let no_colors = init_color(options.is_present("no_colors")); + let colors = init_color(match options.is_present("no_colors") { + false => ColorState::Enabled, + true => ColorState::Disabled, + }); let use_apparent_size = options.is_present("display_apparent_size"); - let ignore_directories: Vec = options + let ignore_directories = options .values_of("ignore_directory") - .map(|i| i.map(PathBuf::from).collect()) - .unwrap_or_default(); + .unwrap_or_default() + .map(PathBuf::from); let by_filecount = options.is_present("by_filecount"); let ignore_hidden = options.is_present("ignore_hidden"); let limit_filesystem = options.is_present("limit_filesystem"); let simplified_dirs = simplify_dir_names(target_dirs); - let allowed_filesystems = { - if limit_filesystem { - get_filesystem_devices(simplified_dirs.iter()) - } else { - HashSet::new() - } - }; + let allowed_filesystems = limit_filesystem + .then(|| get_filesystem_devices(simplified_dirs.iter())) + .unwrap_or_default(); let ignored_full_path: HashSet = ignore_directories - .into_iter() - .flat_map(|x| simplified_dirs.iter().map(move |d| d.join(x.clone()))) + .flat_map(|x| simplified_dirs.iter().map(move |d| d.join(&x))) .collect(); let walk_data = WalkData { @@ -308,34 +300,31 @@ fn main() { let (top_level_nodes, has_errors) = walk_it(simplified_dirs, walk_data); - let tree = { - match (depth, summarize_file_types) { - (_, true) => get_all_file_types(top_level_nodes, number_of_lines), - (depth, _) => get_biggest( - top_level_nodes, - number_of_lines, - depth, - options.values_of("filter").is_some() - || options.value_of("invert_filter").is_some(), - ), - } + let tree = match summarize_file_types { + true => get_all_file_types(&top_level_nodes, number_of_lines), + false => get_biggest( + top_level_nodes, + number_of_lines, + depth, + options.values_of("filter").is_some() || options.value_of("invert_filter").is_some(), + ), }; if has_errors { eprintln!("Did not have permissions for all directories"); } - match tree { - None => {} - Some(root_node) => draw_it( + + if let Some(root_node) = tree { + draw_it( options.is_present("display_full_paths"), !options.is_present("reverse"), - no_colors, + colors, options.is_present("no_bars"), terminal_width, by_filecount, - root_node, + &root_node, options.is_present("iso"), options.is_present("skip_total"), - ), + ) } } diff --git a/src/node.rs b/src/node.rs index 3e1b3fe..78ecbf0 100644 --- a/src/node.rs +++ b/src/node.rs @@ -27,36 +27,33 @@ pub fn build_node( by_filecount: bool, depth: usize, ) -> Option { - match get_metadata(&dir, use_apparent_size) { - Some(data) => { - let inode_device = if is_symlink && !use_apparent_size { - None - } else { - data.1 - }; + get_metadata(&dir, use_apparent_size).map(|data| { + let inode_device = if is_symlink && !use_apparent_size { + None + } else { + data.1 + }; - let size = if is_filtered_out_due_to_regex(filter_regex, &dir) - || is_filtered_out_due_to_invert_regex(invert_filter_regex, &dir) - || (is_symlink && !use_apparent_size) - || by_filecount && !is_file - { - 0 - } else if by_filecount { - 1 - } else { - data.0 - }; + let size = if is_filtered_out_due_to_regex(filter_regex, &dir) + || is_filtered_out_due_to_invert_regex(invert_filter_regex, &dir) + || (is_symlink && !use_apparent_size) + || by_filecount && !is_file + { + 0 + } else if by_filecount { + 1 + } else { + data.0 + }; - Some(Node { - name: dir, - size, - children, - inode_device, - depth, - }) + Node { + name: dir, + size, + children, + inode_device, + depth, } - None => None, - } + }) } impl PartialEq for Node { @@ -67,11 +64,10 @@ impl PartialEq for 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) - } + self.size + .cmp(&other.size) + .then_with(|| self.name.cmp(&other.name)) + .then_with(|| self.children.cmp(&other.children)) } } diff --git a/src/platform.rs b/src/platform.rs index 151863b..77c578a 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -5,7 +5,7 @@ use std::path::Path; #[cfg(target_family = "unix")] fn get_block_size() -> u64 { - // All os specific implementations of MetatdataExt seem to define a block as 512 bytes + // All os specific implementations of MetadataExt seem to define a block as 512 bytes // https://doc.rust-lang.org/std/os/linux/fs/trait.MetadataExt.html#tymethod.st_blocks 512 } @@ -105,12 +105,12 @@ pub fn get_metadata(d: &Path, _use_apparent_size: bool) -> Option<(u64, Option<( use std::os::windows::fs::MetadataExt; match d.metadata() { Ok(ref md) => { - const FILE_ATTRIBUTE_ARCHIVE: u32 = 0x20u32; - const FILE_ATTRIBUTE_READONLY: u32 = 0x1u32; - const FILE_ATTRIBUTE_HIDDEN: u32 = 0x2u32; - const FILE_ATTRIBUTE_SYSTEM: u32 = 0x4u32; - const FILE_ATTRIBUTE_NORMAL: u32 = 0x80u32; - const FILE_ATTRIBUTE_DIRECTORY: u32 = 0x10u32; + const FILE_ATTRIBUTE_ARCHIVE: u32 = 0x20; + const FILE_ATTRIBUTE_READONLY: u32 = 0x01; + const FILE_ATTRIBUTE_HIDDEN: u32 = 0x02; + const FILE_ATTRIBUTE_SYSTEM: u32 = 0x04; + const FILE_ATTRIBUTE_NORMAL: u32 = 0x80; + const FILE_ATTRIBUTE_DIRECTORY: u32 = 0x10; let attr_filtered = md.file_attributes() & !(FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_SYSTEM); diff --git a/src/utils.rs b/src/utils.rs index b66e165..30ea705 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -7,6 +7,7 @@ use regex::Regex; pub fn simplify_dir_names>(filenames: Vec

) -> HashSet { let mut top_level_names: HashSet = HashSet::with_capacity(filenames.len()); + for t in filenames { let top_level_name = normalize_path(t); let mut can_add = true; @@ -26,6 +27,7 @@ pub fn simplify_dir_names>(filenames: Vec

) -> HashSet top_level_names.insert(top_level_name); } } + top_level_names } @@ -33,14 +35,9 @@ pub fn get_filesystem_devices<'a, P: IntoIterator>(paths: P) // Gets the device ids for the filesystems which are used by the argument paths paths .into_iter() - .filter_map(|p| { - let meta = get_metadata(p, false); - - if let Some((_size, Some((_id, dev)))) = meta { - Some(dev) - } else { - None - } + .filter_map(|p| match get_metadata(p, false) { + Some((_size, Some((_id, dev)))) => Some(dev), + _ => None, }) .collect() } @@ -52,7 +49,7 @@ pub fn normalize_path>(path: P) -> PathBuf { // 3. removing trailing extra separators and '.' ("current directory") path segments // * `Path.components()` does all the above work; ref: // 4. changing to os preferred separator (automatically done by recollecting components back into a PathBuf) - path.as_ref().components().collect::() + path.as_ref().components().collect() } pub fn is_filtered_out_due_to_regex(filter_regex: &[Regex], dir: &Path) -> bool { diff --git a/tests/test_exact_output.rs b/tests/test_exact_output.rs index dbba837..465171e 100644 --- a/tests/test_exact_output.rs +++ b/tests/test_exact_output.rs @@ -20,14 +20,11 @@ fn copy_test_data(dir: &str) { // First remove the existing directory - just incase it is there and has incorrect data let last_slash = dir.rfind('/').unwrap(); let last_part_of_dir = dir.chars().skip(last_slash).collect::(); - match Command::new("rm") + let _ = Command::new("rm") .arg("-rf") .arg("/tmp/".to_owned() + &*last_part_of_dir) - .ok() - { - Ok(_) => {} - Err(_) => {} - }; + .ok(); + match Command::new("cp").arg("-r").arg(dir).arg("/tmp/").ok() { Ok(_) => {} Err(err) => { @@ -48,14 +45,14 @@ fn exact_output_test>(valid_outputs: Vec, command_args: initialize(); let mut a = &mut Command::cargo_bin("dust").unwrap(); + for p in command_args { a = a.arg(p); } - let output: String = str::from_utf8(&a.unwrap().stdout).unwrap().into(); - assert!(valid_outputs - .iter() - .fold(false, |sum, i| sum || output.contains(i))); + let output = str::from_utf8(&a.unwrap().stdout).unwrap().to_owned(); + + assert!(valid_outputs.iter().any(|i| output.contains(i))); } // "windows" result data can vary by host (size seems to be variable by one byte); fix code vs test and re-enable @@ -129,7 +126,7 @@ fn main_output_long_paths() -> Vec { vec![mac_and_some_linux, ubuntu] } -// Check against directories and files whos names are substrings of each other +// Check against directories and files whose names are substrings of each other #[cfg_attr(target_os = "windows", ignore)] #[test] pub fn test_substring_of_names_and_long_names() { diff --git a/tests/test_flags.rs b/tests/test_flags.rs index c9b7e93..81f1119 100644 --- a/tests/test_flags.rs +++ b/tests/test_flags.rs @@ -67,7 +67,7 @@ pub fn test_d_flag_works_and_still_recurses_down() { assert!(output.contains("4 ┌─┴ test_dir2")); } -// Check against directories and files whos names are substrings of each other +// Check against directories and files whose names are substrings of each other #[test] pub fn test_ignore_dir() { let output = build_command(vec!["-c", "-X", "dir_substring", "tests/test_dir2/"]); diff --git a/tests/tests_symlinks.rs b/tests/tests_symlinks.rs index 92aca17..08b96f6 100644 --- a/tests/tests_symlinks.rs +++ b/tests/tests_symlinks.rs @@ -39,9 +39,12 @@ pub fn test_soft_sym_link() { let a = format!("─┴ {}", dir_s); let mut cmd = Command::cargo_bin("dust").unwrap(); - // Mac test runners create long filenames in tmp directories let output = cmd - .args(["-p", "-c", "-s", "-w 999", dir_s]) + .arg("-p") + .arg("-c") + .arg("-s") + .args(["-w", "999"]) + .arg(dir_s) .unwrap() .stdout; @@ -72,8 +75,13 @@ pub fn test_hard_sym_link() { let dirs_output = format!("─┴ {}", dir_s); let mut cmd = Command::cargo_bin("dust").unwrap(); - // Mac test runners create long filenames in tmp directories - let output = cmd.args(["-p", "-c", "-w 999", dir_s]).unwrap().stdout; + let output = cmd + .arg("-p") + .arg("-c") + .args(["-w", "999"]) + .arg(dir_s) + .unwrap() + .stdout; // The link should not appear in the output because multiple inodes are now ordered // then filtered.