From e80c60b027338760fe632a8f17c73e9da58f61b5 Mon Sep 17 00:00:00 2001 From: "andy.boot" Date: Tue, 2 Jun 2020 17:21:09 +0100 Subject: [PATCH] Do not print filenames that are too long https://github.com/bootandy/dust/issues/92 Not 100% sure if this code is clean yet, may well be a better way to do it Also: Added test directory with incredably long name as test case. Update test_symlinks.py for mac runners. Mac test runners create files with very long names, hence the tests fail periodically unless they look for a truncated name with '..' at the end. --- src/display.rs | 132 ++++++++++++++---- ...when_this_goes_over_80_characters_i_wonder | 0 tests/tests.rs | 28 ++-- tests/tests_symlinks.rs | 39 +++++- 4 files changed, 150 insertions(+), 49 deletions(-) create mode 100644 src/test_dir2/long_dir_name_what_a_very_long_dir_name_what_happens_when_this_goes_over_80_characters_i_wonder diff --git a/src/display.rs b/src/display.rs index 642a978..092787c 100644 --- a/src/display.rs +++ b/src/display.rs @@ -147,12 +147,13 @@ pub fn draw_it( if !permissions { eprintln!("Did not have permissions for all directories"); } + let terminal_width = (get_width_of_terminal() - 16) as usize; + let num_indent_chars = 3; let longest_string_length = root_node .children .iter() - .map(|c| find_longest_dir_name(&c, " ", !use_full_path)) + .map(|c| find_longest_dir_name(&c, num_indent_chars, terminal_width, !use_full_path)) .fold(0, max); - let terminal_width = get_width_of_terminal() - 16; let max_bar_length = if no_percents || longest_string_length >= terminal_width as usize { 0 @@ -160,8 +161,7 @@ pub fn draw_it( terminal_width as usize - longest_string_length }; - // handle usize error also add do not show fancy output option - let bar_text = repeat(BLOCKS[0]).take(max_bar_length).collect::(); + let first_size_bar = repeat(BLOCKS[0]).take(max_bar_length).collect::(); for c in get_children_from_node(root_node, is_reversed) { let display_data = DisplayData { @@ -174,36 +174,36 @@ pub fn draw_it( }; let draw_data = DrawData { indent: "".to_string(), - percent_bar: bar_text.clone(), + percent_bar: first_size_bar.clone(), display_data: &display_data, }; display_node(c, &draw_data, true, true); } } -// We can probably pass depth instead of indent here. -// It is ugly to feed in ' ' instead of the actual tree characters but we don't need them yet. -fn find_longest_dir_name(node: &Node, indent: &str, long_paths: bool) -> usize { +fn find_longest_dir_name(node: &Node, indent: usize, terminal: usize, long_paths: bool) -> usize { let printable_name = get_printable_name(&node.name, long_paths); - let longest = get_unicode_width_of_indent_and_name(indent, &printable_name); + let longest = min( + UnicodeWidthStr::width(&*printable_name) + 1 + indent, + terminal, + ); - // each none root tree drawing is 2 chars - let full_indent: String = indent.to_string() + " "; + // each none root tree drawing is 2 more chars, hence we increment indent by 2 node.children .iter() - .map(|c| find_longest_dir_name(c, &*full_indent, long_paths)) + .map(|c| find_longest_dir_name(c, indent + 2, terminal, long_paths)) .fold(longest, max) } fn display_node(node: Node, draw_data: &DrawData, is_biggest: bool, is_last: bool) { - let indent2 = draw_data.get_new_indent(!node.children.is_empty(), is_last); // hacky way of working out how deep we are in the tree - let level = ((indent2.chars().count() - 1) / 2) - 1; + 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 to_print = format_string( &node, - &*indent2, + &*indent, &*bar_text, is_biggest, draw_data.display_data, @@ -214,7 +214,7 @@ fn display_node(node: Node, draw_data: &DrawData, is_biggest: bool, is_last: boo } let dd = DrawData { - indent: clean_indentation_string(&*indent2), + indent: clean_indentation_string(&*indent), percent_bar: bar_text, display_data: draw_data.display_data, }; @@ -266,9 +266,30 @@ fn get_printable_name>(dir_name: &P, long_paths: bool) -> String printable_name.display().to_string() } -fn get_unicode_width_of_indent_and_name(indent: &str, name: &str) -> usize { +fn pad_or_trim_filename(node: &Node, indent: &str, display_data: &DisplayData) -> String { + let name = get_printable_name(&node.name, display_data.short_paths); let indent_and_name = format!("{} {}", indent, name); - UnicodeWidthStr::width(&*indent_and_name) + let width = UnicodeWidthStr::width(&*indent_and_name); + + // Add spaces after the filename so we can draw the % used bar chart. + let name_and_padding = name + + &(repeat(" ") + .take(display_data.longest_string_length - width) + .collect::()); + + maybe_trim_filename(name_and_padding, display_data) +} + +fn maybe_trim_filename(name_in: String, display_data: &DisplayData) -> String { + if UnicodeWidthStr::width(&*name_in) > display_data.longest_string_length { + let name = name_in + .chars() + .take(display_data.longest_string_length - 2) + .collect::(); + name + ".." + } else { + name_in + } } pub fn format_string( @@ -278,25 +299,18 @@ pub fn format_string( is_biggest: bool, display_data: &DisplayData, ) -> String { - let pretty_size = format!("{:>5}", human_readable_number(node.size)); - - let percent_size_str = format!("{:.0}%", display_data.percent_size(node) * 100.0); - - let name = get_printable_name(&node.name, display_data.short_paths); - let width = get_unicode_width_of_indent_and_name(indent, &name); - let (percents, name_and_padding) = if percent_bar != "" { + let percent_size_str = format!("{:.0}%", display_data.percent_size(node) * 100.0); let percents = format!("│{} │ {:>4}", percent_bar, percent_size_str); - - let name_and_padding = name - + &(repeat(" ") - .take(display_data.longest_string_length - width) - .collect::()); + let name_and_padding = pad_or_trim_filename(node, indent, display_data); (percents, name_and_padding) } else { + let n = get_printable_name(&node.name, display_data.short_paths); + let name = maybe_trim_filename(n, display_data); ("".into(), name) }; + let pretty_size = format!("{:>5}", human_readable_number(node.size)); let pretty_size = if is_biggest && display_data.colors_on { format!("{}", Red.paint(pretty_size)) } else { @@ -316,7 +330,8 @@ pub fn format_string( name_and_padding }; - format!("{} {} {}{}", pretty_size, indent, pretty_name, percents) + let result = format!("{} {} {}{}", pretty_size, indent, pretty_name, percents); + result } fn human_readable_number(size: u64) -> String { @@ -336,6 +351,61 @@ fn human_readable_number(size: u64) -> String { mod tests { #[allow(unused_imports)] use super::*; + #[allow(unused_imports)] + use std::path::PathBuf; + + #[cfg(test)] + fn get_fake_display_data(longest_string_length: usize) -> DisplayData { + DisplayData { + short_paths: true, + is_reversed: false, + colors_on: false, + base_size: 1, + longest_string_length: longest_string_length, + ls_colors: LsColors::from_env().unwrap_or_default(), + } + } + + #[test] + fn test_format_str() { + let n = Node { + name: PathBuf::from("/short"), + size: 2_u64.pow(12), // This is 4.0K + children: vec![], + }; + let indent = "┌─┴"; + let percent_bar = ""; + let is_biggest = false; + + let s = format_string( + &n, + indent, + percent_bar, + is_biggest, + &get_fake_display_data(6), + ); + assert_eq!(s, " 4.0K ┌─┴ short"); + } + + #[test] + fn test_format_str_long_name() { + let name = "very_long_name_longer_than_the_eighty_character_limit_very_long_name_this_bit_will_truncate"; + let n = Node { + name: PathBuf::from(name), + size: 2_u64.pow(12), // This is 4.0K + children: vec![], + }; + let indent = "┌─┴"; + let percent_bar = ""; + let is_biggest = false; + + let dd = get_fake_display_data(64); + let s = format_string(&n, indent, percent_bar, is_biggest, &dd); + assert_eq!( + s, + " 4.0K ┌─┴ very_long_name_longer_than_the_eighty_character_limit_very_lon.." + ); + } #[test] fn test_human_readable_number() { diff --git a/src/test_dir2/long_dir_name_what_a_very_long_dir_name_what_happens_when_this_goes_over_80_characters_i_wonder b/src/test_dir2/long_dir_name_what_a_very_long_dir_name_what_happens_when_this_goes_over_80_characters_i_wonder new file mode 100644 index 0000000..e69de29 diff --git a/tests/tests.rs b/tests/tests.rs index 6270c4a..d6a6e77 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -236,7 +236,7 @@ pub fn test_d_flag_works() { // Check against directories and files whos names are substrings of each other #[cfg_attr(target_os = "windows", ignore)] #[test] -pub fn test_substring_of_names() { +pub fn test_substring_of_names_and_long_names() { initialize(); let mut cmd = Command::cargo_bin("dust").unwrap(); let output = cmd.arg("-c").arg("/tmp/test_dir2").unwrap().stdout; @@ -247,12 +247,13 @@ pub fn test_substring_of_names() { #[cfg(target_os = "linux")] fn no_substring_of_names_output() -> String { " - 4.0K ┌── dir_name_clash│ ████████ │ 17% - 4.0K │ ┌── hello │ ░░░░░░░████████ │ 17% - 8.0K ├─┴ dir_substring │ ███████████████ │ 33% - 4.0K │ ┌── hello │ ░░░░░░░████████ │ 17% - 8.0K ├─┴ dir │ ███████████████ │ 33% - 24K ┌─┴ test_dir2 │████████████████████████████████████████████ │ 100% + 0B ┌── long_dir_name_what_a_very_long_dir_name_what_happens_when_this.. + 4.0K ├── dir_name_clash + 4.0K │ ┌── hello + 8.0K ├─┴ dir_substring + 4.0K │ ┌── hello + 8.0K ├─┴ dir + 24K ┌─┴ test_dir2 " .trim() .into() @@ -261,12 +262,13 @@ fn no_substring_of_names_output() -> String { #[cfg(target_os = "macos")] fn no_substring_of_names_output() -> String { " - 4.0K ┌── hello │ ███████████████ │ 33% - 4.0K ┌─┴ dir_substring │ ███████████████ │ 33% - 4.0K ├── dir_name_clash│ ███████████████ │ 33% - 4.0K │ ┌── hello │ ███████████████ │ 33% - 4.0K ├─┴ dir │ ███████████████ │ 33% - 12K ┌─┴ test_dir2 │████████████████████████████████████████████ │ 100% + 0B ┌── long_dir_name_what_a_very_long_dir_name_what_happens_when_this.. + 4.0K │ ┌── hello + 4.0K ├─┴ dir_substring + 4.0K ├── dir_name_clash + 4.0K │ ┌── hello + 4.0K ├─┴ dir + 12K ┌─┴ test_dir2 " .trim() .into() diff --git a/tests/tests_symlinks.rs b/tests/tests_symlinks.rs index 7eb3561..118f4d3 100644 --- a/tests/tests_symlinks.rs +++ b/tests/tests_symlinks.rs @@ -1,15 +1,42 @@ use assert_cmd::Command; +use std::cmp::max; use std::fs::File; use std::io::Write; use std::panic; use std::path::PathBuf; use std::str; + +use terminal_size::{terminal_size, Height, Width}; +use unicode_width::UnicodeWidthStr; + use tempfile::Builder; use tempfile::TempDir; // File sizes differ on both platform and on the format of the disk. // Windows: `ln` is not usually an available command; creation of symbolic links requires special enhanced permissions +fn get_width_of_terminal() -> u16 { + if let Some((Width(w), Height(_h))) = terminal_size() { + max(w, 80) + } else { + 80 + } +} + +// Mac test runners create tmp files with very long names, hence it may be shortened in the output +fn get_file_name(name: String) -> String { + let terminal_plus_buffer = (get_width_of_terminal() - 16) as usize; + if UnicodeWidthStr::width(&*name) > terminal_plus_buffer { + let trimmed_name = name + .chars() + .take(terminal_plus_buffer - 2) + .collect::(); + trimmed_name + ".." + } else { + name + } +} + fn build_temp_file(dir: &TempDir) -> PathBuf { let file_path = dir.path().join("notes.txt"); let mut file = File::create(&file_path).unwrap(); @@ -34,8 +61,8 @@ pub fn test_soft_sym_link() { .output(); assert!(c.is_ok()); - let c = format!(" ┌── {}", link_name_s); - let b = format!(" ├── {}", file_path_s); + let c = format!(" ┌── {}", get_file_name(link_name_s.into())); + let b = format!(" ├── {}", get_file_name(file_path_s.into())); let a = format!("─┴ {}", dir_s); let mut cmd = Command::cargo_bin("dust").unwrap(); @@ -43,6 +70,7 @@ pub fn test_soft_sym_link() { let output = str::from_utf8(&output).unwrap(); + println!("{:?}", output); assert!(output.contains(a.as_str())); assert!(output.contains(b.as_str())); assert!(output.contains(c.as_str())); @@ -64,8 +92,8 @@ pub fn test_hard_sym_link() { .output(); assert!(c.is_ok()); - let link_output = format!(" ┌── {}", link_name_s); - let file_output = format!(" ┌── {}", file_path_s); + let link_output = format!(" ┌── {}", get_file_name(link_name_s.into())); + let file_output = format!(" ┌── {}", get_file_name(file_path_s.into())); let dirs_output = format!("─┴ {}", dir_s); let mut cmd = Command::cargo_bin("dust").unwrap(); @@ -95,12 +123,13 @@ pub fn test_recursive_sym_link() { assert!(c.is_ok()); let a = format!("─┬ {}", dir_s); - let b = format!(" └── {}", link_name_s); + let b = format!(" └── {}", get_file_name(link_name_s.into())); let mut cmd = Command::cargo_bin("dust").unwrap(); let output = cmd.arg("-p").arg("-c").arg("-r").arg(dir_s).unwrap().stdout; let output = str::from_utf8(&output).unwrap(); + println!("{:?}", output); assert!(output.contains(a.as_str())); assert!(output.contains(b.as_str())); }