From 7d2b051866f77000d253de324ad69e3fbe31ce9d Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Thu, 6 May 2021 02:33:25 +0530 Subject: [PATCH] Implement Total size feature (#2170) * ls: Implement total size feature - Implement total size reporting that was missing - Fix minor formatting / readability nits * tests: Add tests for ls total sizes feature * ls: Fix MSRV build errors due to unsupported attributes for if blocks * ls: Add windows support for total sizes feature - Add windows support (defaults to file size as block sizes related infromation is not avialable on windows) - Renamed some functions --- Cargo.lock | 2 ++ src/uu/ls/src/ls.rs | 74 ++++++++++++++++++++++++++++------------ tests/by-util/test_ls.rs | 45 ++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 62fa80c2d..3cd0c7cda 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1692,6 +1692,7 @@ dependencies = [ name = "uu_basename" version = "0.0.6" dependencies = [ + "clap", "uucore", "uucore_procs", ] @@ -2645,6 +2646,7 @@ dependencies = [ name = "uu_who" version = "0.0.6" dependencies = [ + "clap", "uucore", "uucore_procs", ] diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 0e2754f07..f24bf513e 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1179,31 +1179,32 @@ impl PathData { } fn list(locs: Vec, config: Config) -> i32 { - let number_of_locs = locs.len(); - let mut files = Vec::::new(); let mut dirs = Vec::::new(); let mut has_failed = false; let mut out = BufWriter::new(stdout()); - for loc in locs { + for loc in &locs { let p = PathBuf::from(&loc); if !p.exists() { show_error!("'{}': {}", &loc, "No such file or directory"); - // We found an error, the return code of ls should not be 0 - // And no need to continue the execution + /* + We found an error, the return code of ls should not be 0 + And no need to continue the execution + */ has_failed = true; continue; } let path_data = PathData::new(p, None, None, &config, true); - let show_dir_contents = if let Some(ft) = path_data.file_type() { - !config.directory && ft.is_dir() - } else { - has_failed = true; - false + let show_dir_contents = match path_data.file_type() { + Some(ft) => !config.directory && ft.is_dir(), + None => { + has_failed = true; + false + } }; if show_dir_contents { @@ -1217,7 +1218,7 @@ fn list(locs: Vec, config: Config) -> i32 { sort_entries(&mut dirs, &config); for dir in dirs { - if number_of_locs > 1 { + if locs.len() > 1 { let _ = writeln!(out, "\n{}:", dir.p_buf.display()); } enter_directory(&dir, &config, &mut out); @@ -1331,7 +1332,7 @@ fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize) { if let Some(md) = entry.md() { ( display_symlink_count(&md).len(), - display_file_size(&md, config).len(), + display_size(md.len(), config).len(), ) } else { (0, 0) @@ -1344,14 +1345,22 @@ fn pad_left(string: String, count: usize) -> String { fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter) { if config.format == Format::Long { - let (mut max_links, mut max_size) = (1, 1); + let (mut max_links, mut max_width) = (1, 1); + let mut total_size = 0; + for item in items { - let (links, size) = display_dir_entry_size(item, config); + let (links, width) = display_dir_entry_size(item, config); max_links = links.max(max_links); - max_size = size.max(max_size); + max_width = width.max(max_width); + total_size += item.md().map_or(0, |md| get_block_size(md, config)); } + + if total_size > 0 { + let _ = writeln!(out, "total {}", display_size(total_size, config)); + } + for item in items { - display_item_long(item, max_links, max_size, config, out); + display_item_long(item, max_links, max_width, config, out); } } else { let names = items.iter().filter_map(|i| display_file_name(&i, config)); @@ -1396,6 +1405,29 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter u64 { + /* GNU ls will display sizes in terms of block size + md.len() will differ from this value when the file has some holes + */ + #[cfg(unix)] + { + // hard-coded for now - enabling setting this remains a TODO + let ls_block_size = 1024; + return match config.size_format { + SizeFormat::Binary => md.blocks() * 512, + SizeFormat::Decimal => md.blocks() * 512, + SizeFormat::Bytes => md.blocks() * 512 / ls_block_size, + }; + } + + #[cfg(not(unix))] + { + let _ = config; + // no way to get block size for windows, fall-back to file size + md.len() + } +} + fn display_grid( names: impl Iterator, width: u16, @@ -1471,7 +1503,7 @@ fn display_item_long( let _ = writeln!( out, " {} {} {}", - pad_left(display_file_size(&md, config), max_size), + pad_left(display_size(md.len(), config), max_size), display_date(&md, config), // unwrap is fine because it fails when metadata is not available // but we already know that it is because it's checked at the @@ -1626,13 +1658,13 @@ fn format_prefixed(prefixed: NumberPrefix) -> String { } } -fn display_file_size(metadata: &Metadata, config: &Config) -> String { +fn display_size(len: u64, config: &Config) -> String { // NOTE: The human-readable behaviour deviates from the GNU ls. // The GNU ls uses binary prefixes by default. match config.size_format { - SizeFormat::Binary => format_prefixed(NumberPrefix::binary(metadata.len() as f64)), - SizeFormat::Decimal => format_prefixed(NumberPrefix::decimal(metadata.len() as f64)), - SizeFormat::Bytes => metadata.len().to_string(), + SizeFormat::Binary => format_prefixed(NumberPrefix::binary(len as f64)), + SizeFormat::Decimal => format_prefixed(NumberPrefix::decimal(len as f64)), + SizeFormat::Bytes => len.to_string(), } } diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 110764aa5..0985ba719 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -5,6 +5,7 @@ use crate::common::util::*; extern crate regex; use self::regex::Regex; +use std::collections::HashMap; use std::path::Path; use std::thread::sleep; use std::time::Duration; @@ -308,6 +309,50 @@ fn test_ls_long() { } } +#[test] +fn test_ls_long_total_size() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch(&at.plus_as_string("test-long")); + at.append("test-long", "1"); + at.touch(&at.plus_as_string("test-long2")); + at.append("test-long2", "2"); + + let expected_prints: HashMap<_, _> = if cfg!(unix) { + [ + ("long_vanilla", "total 8"), + ("long_human_readable", "total 8.0K"), + ("long_si", "total 8.2k"), + ] + .iter() + .cloned() + .collect() + } else { + [ + ("long_vanilla", "total 2"), + ("long_human_readable", "total 2"), + ("long_si", "total 2"), + ] + .iter() + .cloned() + .collect() + }; + + for arg in &["-l", "--long", "--format=long", "--format=verbose"] { + let result = scene.ucmd().arg(arg).succeeds(); + result.stdout_contains(expected_prints["long_vanilla"]); + + for arg2 in &["-h", "--human-readable", "--si"] { + let result = scene.ucmd().arg(arg).arg(arg2).succeeds(); + result.stdout_contains(if *arg2 == "--si" { + expected_prints["long_si"] + } else { + expected_prints["long_human_readable"] + }); + } + } +} + #[test] fn test_ls_long_formats() { let scene = TestScenario::new(util_name!());