From 118b802fe8bcc584be5a3fd65cf8e23396de28dc Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Fri, 19 Mar 2021 15:14:25 +0100 Subject: [PATCH 1/2] ls: --si and more compatible size formatting --- src/uu/ls/src/ls.rs | 66 +++++++++++++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 5c4cdaa80..f7075c4df 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -92,13 +92,16 @@ pub mod options { pub static ACCESS: &str = "u"; pub static CHANGE: &str = "c"; } + pub mod size { + pub static HUMAN_READABLE: &str = "human-readable"; + pub static SI: &str = "si"; + } pub static FORMAT: &str = "format"; pub static SORT: &str = "sort"; pub static TIME: &str = "time"; pub static IGNORE_BACKUPS: &str = "ignore-backups"; pub static DIRECTORY: &str = "directory"; pub static CLASSIFY: &str = "classify"; - pub static HUMAN_READABLE: &str = "human-readable"; pub static INODE: &str = "inode"; pub static DEREFERENCE: &str = "dereference"; pub static NUMERIC_UID_GID: &str = "numeric-uid-gid"; @@ -122,9 +125,10 @@ enum Sort { Time, } -enum SizeFormats { +enum SizeFormat { Bytes, - Binary, // Powers of 1024, --human-readable + Binary, // Powers of 1024, --human-readable, -h + Decimal, // Powers of 1000, --si } #[derive(PartialEq, Eq)] @@ -149,7 +153,7 @@ struct Config { dereference: bool, classify: bool, ignore_backups: bool, - size_format: SizeFormats, + size_format: SizeFormat, numeric_uid_gid: bool, directory: bool, time: Time, @@ -231,10 +235,12 @@ impl Config { }, }; - let size_format = if options.is_present(options::HUMAN_READABLE) { - SizeFormats::Binary + let size_format = if options.is_present(options::size::HUMAN_READABLE) { + SizeFormat::Binary + } else if options.is_present(options::size::SI) { + SizeFormat::Decimal } else { - SizeFormats::Bytes + SizeFormat::Bytes }; Config { @@ -413,10 +419,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 { '>' for doors, and nothing for regular files.", )) .arg( - Arg::with_name(options::HUMAN_READABLE) + Arg::with_name(options::size::HUMAN_READABLE) .short("h") - .long(options::HUMAN_READABLE) - .help("Print human readable file sizes (e.g. 1K 234M 56G)."), + .long(options::size::HUMAN_READABLE) + .help("Print human readable file sizes (e.g. 1K 234M 56G).") + .overrides_with(options::size::SI), + ) + .arg( + Arg::with_name(options::size::SI) + .long(options::size::SI) + .help("Print human readable file sizes using powers of 1000 instead of 1024.") ) .arg( Arg::with_name(options::INODE) @@ -787,17 +799,37 @@ fn display_date(metadata: &Metadata, config: &Config) -> String { } } +// There are a few peculiarities to how GNU formats the sizes: +// 1. One decimal place is given if and only if the size is smaller than 10 +// 2. It rounds sizes up. +// 3. The human-readable format uses powers for 1024, but does not display the "i" +// that is commonly used to denote Kibi, Mebi, etc. +// 4. Kibi and Kilo are denoted differently ("k" and "K", respectively) +fn format_prefixed(prefixed: NumberPrefix) -> String { + match prefixed { + NumberPrefix::Standalone(bytes) => bytes.to_string(), + NumberPrefix::Prefixed(prefix, bytes) => { + // Remove the "i" from "Ki", "Mi", etc. if present + let prefix_str = prefix.symbol().trim_end_matches("i"); + + // Check whether we get more than 10 if we round up to the first decimal + // because we want do display 9.81 as "9.9", not as "10". + if (10.0 * bytes).ceil() >= 100.0 { + format!("{:.0}{}", bytes.ceil(), prefix_str) + } else { + format!("{:.1}{}", (10.0 * bytes).ceil() / 10.0, prefix_str) + } + } + } +} + fn display_file_size(metadata: &Metadata, 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 { - SizeFormats::Binary => match NumberPrefix::decimal(metadata.len() as f64) { - NumberPrefix::Standalone(bytes) => bytes.to_string(), - NumberPrefix::Prefixed(prefix, bytes) => { - format!("{:.2}{}", bytes, prefix).to_uppercase() - } - }, - SizeFormats::Bytes => metadata.len().to_string(), + 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(), } } From 39b07f670f8b35599c4a62d978715631ae19dea5 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Fri, 19 Mar 2021 15:15:24 +0100 Subject: [PATCH 2/2] tests/ls: adapt tests to --si and new size formats --- tests/by-util/test_ls.rs | 82 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 74 insertions(+), 8 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index d01aaffd9..e0063aa1a 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -428,28 +428,94 @@ fn test_ls_ls_color() { #[cfg(not(any(target_vendor = "apple", target_os = "windows")))] // Truncate not available on mac or win #[test] -fn test_ls_human() { +fn test_ls_human_si() { let scene = TestScenario::new(util_name!()); - let file = "test_human"; - let result = scene.cmd("truncate").arg("-s").arg("+1000").arg(file).run(); + let file1 = "test_human-1"; + let result = scene + .cmd("truncate") + .arg("-s") + .arg("+1000") + .arg(file1) + .run(); println!("stderr = {:?}", result.stderr); println!("stdout = {:?}", result.stdout); - let result = scene.ucmd().arg("-hl").arg(file).run(); + + let result = scene.ucmd().arg("-hl").arg(file1).run(); println!("stderr = {:?}", result.stderr); println!("stdout = {:?}", result.stdout); assert!(result.success); - assert!(result.stdout.contains("1.00K")); + assert!(result.stdout.contains(" 1000 ")); + + let result = scene.ucmd().arg("-l").arg("--si").arg(file1).run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + assert!(result.stdout.contains(" 1.0k ")); + scene .cmd("truncate") .arg("-s") .arg("+1000k") - .arg(file) + .arg(file1) .run(); - let result = scene.ucmd().arg("-hl").arg(file).run(); + + let result = scene.ucmd().arg("-hl").arg(file1).run(); println!("stderr = {:?}", result.stderr); println!("stdout = {:?}", result.stdout); assert!(result.success); - assert!(result.stdout.contains("1.02M")); + assert!(result.stdout.contains(" 1001K ")); + + let result = scene.ucmd().arg("-l").arg("--si").arg(file1).run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + assert!(result.stdout.contains(" 1.1M ")); + + let file2 = "test-human-2"; + let result = scene + .cmd("truncate") + .arg("-s") + .arg("+12300k") + .arg(file2) + .run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + let result = scene.ucmd().arg("-hl").arg(file2).run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + // GNU rounds up, so we must too. + assert!(result.stdout.contains(" 13M ")); + + let result = scene.ucmd().arg("-l").arg("--si").arg(file2).run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + // GNU rounds up, so we must too. + assert!(result.stdout.contains(" 13M ")); + + let file3 = "test-human-3"; + let result = scene + .cmd("truncate") + .arg("-s") + .arg("+9999") + .arg(file3) + .run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + + let result = scene.ucmd().arg("-hl").arg(file3).run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + assert!(result.stdout.contains(" 9.8K ")); + + let result = scene.ucmd().arg("-l").arg("--si").arg(file3).run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + assert!(result.stdout.contains(" 10k ")); } #[cfg(windows)]