From 6551cbe50e8921eb59df5b54249bd3620b67f247 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Mon, 3 May 2021 21:07:47 +0530 Subject: [PATCH 1/4] ls: Implement total size feature - Implement total size reporting that was missing - Fix minor formatting / readability nits --- src/uu/ls/src/ls.rs | 72 ++++++++++++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 21 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 0e2754f0703..7c488f77b52 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_file_size(md.len(), config).len(), ) } else { (0, 0) @@ -1344,14 +1345,23 @@ 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| display_block_size(md, config)); + } + + #[cfg(unix)] + if total_size > 0 { + let _ = writeln!(out, "total {}", display_file_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 +1406,26 @@ 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))] + // unsupported for windows at the moment + 0 +} + fn display_grid( names: impl Iterator, width: u16, @@ -1471,7 +1501,7 @@ fn display_item_long( let _ = writeln!( out, " {} {} {}", - pad_left(display_file_size(&md, config), max_size), + pad_left(display_file_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 +1656,13 @@ fn format_prefixed(prefixed: NumberPrefix) -> String { } } -fn display_file_size(metadata: &Metadata, config: &Config) -> String { +fn display_file_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(), } } From d3aaf9b342608caea5c2e9c711abeef6f696f837 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Mon, 3 May 2021 23:29:07 +0530 Subject: [PATCH 2/4] tests: Add tests for ls total sizes feature --- tests/by-util/test_ls.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 110764aa5fc..c6d9a89359d 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -308,6 +308,31 @@ fn test_ls_long() { } } +#[cfg(unix)] +#[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"); + + for arg in &["-l", "--long", "--format=long", "--format=verbose"] { + let result = scene.ucmd().arg(arg).succeeds(); + result.stdout_contains("total 8"); + + for arg2 in &["-h", "--human-readable", "--si"] { + let result = scene.ucmd().arg(arg).arg(arg2).succeeds(); + result.stdout_contains(if *arg2 == "--si" { + "total 8.2k" + } else { + "total 8.0K" + }); + } + } +} + #[test] fn test_ls_long_formats() { let scene = TestScenario::new(util_name!()); From d18109cc14594626f23a63bfef74430366bc8147 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Tue, 4 May 2021 21:10:47 +0530 Subject: [PATCH 3/4] ls: Fix MSRV build errors due to unsupported attributes for if blocks --- Cargo.lock | 2 ++ src/uu/ls/src/ls.rs | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8d740bb9ab9..35a1d83f225 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1698,6 +1698,7 @@ dependencies = [ name = "uu_basename" version = "0.0.6" dependencies = [ + "clap", "uucore", "uucore_procs", ] @@ -2653,6 +2654,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 7c488f77b52..2c5cf28929b 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1356,8 +1356,10 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter 0 { - let _ = writeln!(out, "total {}", display_file_size(total_size, config)); + { + if total_size > 0 { + let _ = writeln!(out, "total {}", display_file_size(total_size, config)); + } } for item in items { From 1fd2734d39fa40b76a13444aee2a6783eefc2ad2 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Wed, 5 May 2021 23:02:27 +0530 Subject: [PATCH 4/4] 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 --- src/uu/ls/src/ls.rs | 24 ++++++++++++------------ tests/by-util/test_ls.rs | 28 ++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 2c5cf28929b..f24bf513e1b 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1332,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.len(), config).len(), + display_size(md.len(), config).len(), ) } else { (0, 0) @@ -1352,14 +1352,11 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter 0 { - let _ = writeln!(out, "total {}", display_file_size(total_size, config)); - } + if total_size > 0 { + let _ = writeln!(out, "total {}", display_size(total_size, config)); } for item in items { @@ -1408,7 +1405,7 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter u64 { +fn get_block_size(md: &Metadata, config: &Config) -> u64 { /* GNU ls will display sizes in terms of block size md.len() will differ from this value when the file has some holes */ @@ -1424,8 +1421,11 @@ fn display_block_size(md: &Metadata, config: &Config) -> u64 { } #[cfg(not(unix))] - // unsupported for windows at the moment - 0 + { + let _ = config; + // no way to get block size for windows, fall-back to file size + md.len() + } } fn display_grid( @@ -1503,7 +1503,7 @@ fn display_item_long( let _ = writeln!( out, " {} {} {}", - pad_left(display_file_size(md.len(), 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 @@ -1658,7 +1658,7 @@ fn format_prefixed(prefixed: NumberPrefix) -> String { } } -fn display_file_size(len: u64, 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 { diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index c6d9a89359d..0985ba719da 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,7 +309,6 @@ fn test_ls_long() { } } -#[cfg(unix)] #[test] fn test_ls_long_total_size() { let scene = TestScenario::new(util_name!()); @@ -318,16 +318,36 @@ fn test_ls_long_total_size() { 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("total 8"); + 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" { - "total 8.2k" + expected_prints["long_si"] } else { - "total 8.0K" + expected_prints["long_human_readable"] }); } }