From 0f1c71ca5929c5c61be3e396f5c2dc68b70dde51 Mon Sep 17 00:00:00 2001 From: Anirudh Sanjeev Date: Thu, 8 Jul 2021 22:45:27 -0400 Subject: [PATCH 1/5] Implement "ls -s" and "ls -ls" --- src/uu/ls/src/ls.rs | 74 ++++++++++++++++++++++++++++++++------------- 1 file changed, 53 insertions(+), 21 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index a2c6f3481cd..b35fb135d14 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -119,6 +119,7 @@ pub mod options { pub static IGNORE_BACKUPS: &str = "ignore-backups"; pub static DIRECTORY: &str = "directory"; pub static INODE: &str = "inode"; + pub static BLOCKS: &str = "blocks"; pub static REVERSE: &str = "reverse"; pub static RECURSIVE: &str = "recursive"; pub static COLOR: &str = "color"; @@ -230,6 +231,7 @@ struct Config { time: Time, #[cfg(unix)] inode: bool, + blocks: bool, color: Option, long: LongFormat, width: Option, @@ -571,6 +573,7 @@ impl Config { color, #[cfg(unix)] inode: options.is_present(options::INODE), + blocks: options.is_present(options::BLOCKS), long, width, quoting_style, @@ -1038,6 +1041,12 @@ only ignore '.' and '..'.", .long(options::INODE) .help("print the index number of each file"), ) + .arg( + Arg::with_name(options::BLOCKS) + .short("s") + .long(options::BLOCKS) + .help("print the number of file system blocks of each file"), + ) .arg( Arg::with_name(options::REVERSE) .short("r") @@ -1377,15 +1386,25 @@ fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { } } -fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize) { - if let Some(md) = entry.md() { - ( - display_symlink_count(md).len(), - display_size_or_rdev(md, config).len(), - ) - } else { - (0, 0) + +// Get maximum string sizes for: +// - number of links of each items +// - size +// - blocks +fn get_max_sizes (items: &[PathData], config: &Config) -> (usize, usize, usize, u64) { + let (mut max_links, mut max_width, mut max_blocks) = (1, 1, 1); + let mut total_size = 0; + + for item in items { + if let Some(md) = item.md() { + max_links = display_symlink_count(md).len().max(max_links); + max_width = display_size_or_rdev(md, config).len().max(max_width); + max_blocks = md.blocks().to_string().len().max(max_blocks); + total_size += get_block_size(md, config); + } } + + return (max_links, max_width, max_blocks, total_size); } fn pad_left(string: String, count: usize) -> String { @@ -1393,26 +1412,18 @@ fn pad_left(string: String, count: usize) -> String { } fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter) { + let (max_links, max_width, max_blocks, total_size) = get_max_sizes(items, config); if config.format == Format::Long { - let (mut max_links, mut max_width) = (1, 1); - let mut total_size = 0; - - for item in items { - let (links, width) = display_dir_entry_size(item, config); - max_links = links.max(max_links); - 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_width, config, out); + display_item_long(item, max_links, max_width, max_blocks, config, out); } } else { - let names = items.iter().filter_map(|i| display_file_name(i, config)); + let names = items.iter().filter_map(|i| display_file_name(i, config, max_blocks)); match (&config.format, config.width) { (Format::Columns, Some(width)) => { @@ -1507,6 +1518,7 @@ fn display_item_long( item: &PathData, max_links: usize, max_size: usize, + max_blocks: usize, config: &Config, out: &mut BufWriter, ) { @@ -1525,6 +1537,14 @@ fn display_item_long( } } + if config.blocks { + let _ = write!( + out, + "{} ", + pad_left(md.blocks().to_string(), max_blocks) + ); + } + let _ = write!( out, "{} {}", @@ -1554,7 +1574,7 @@ fn display_item_long( // unwrap is fine because it fails when metadata is not available // but we already know that it is because it's checked at the // start of the function. - display_file_name(item, config).unwrap().contents, + display_file_name(item, config, 1).unwrap().contents, ); } @@ -1563,6 +1583,7 @@ fn get_inode(metadata: &Metadata) -> String { format!("{:8}", metadata.ino()) } + // Currently getpwuid is `linux` target only. If it's broken out into // a posix-compliant attribute this can be updated... #[cfg(unix)] @@ -1765,7 +1786,7 @@ fn classify_file(path: &PathData) -> Option { } } -fn display_file_name(path: &PathData, config: &Config) -> Option { +fn display_file_name(path: &PathData, config: &Config, max_blocks: usize) -> Option { let mut name = escape_name(&path.display_name, &config.quoting_style); #[cfg(unix)] @@ -1815,6 +1836,17 @@ fn display_file_name(path: &PathData, config: &Config) -> Option { } } + if config.format != Format::Long && config.blocks { + if let Some(md) = path.md() { + width += max_blocks + 1; + name.insert_str(0, &format!("{} ", pad_left(md.blocks().to_string(), max_blocks))); + } else { + width += max_blocks + 1; + name.insert_str(0, &format!("{} ", pad_left(String::from(""), max_blocks))); + } + } + + if config.format == Format::Long && path.file_type()?.is_symlink() { if let Ok(target) = path.p_buf.read_link() { name.push_str(" -> "); From b0c50067863c3338f0a2a41f31fb4142d0ea066e Mon Sep 17 00:00:00 2001 From: Anirudh Sanjeev Date: Fri, 9 Jul 2021 13:19:39 -0400 Subject: [PATCH 2/5] Add support for windows and tests --- src/uu/ls/src/ls.rs | 19 +++++++++++++++--- tests/by-util/test_ls.rs | 43 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index b35fb135d14..e0d89d70364 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1386,6 +1386,19 @@ fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { } } +fn get_display_size(md: &Metadata) -> u64 { + #[cfg(windows)] { + if md.is_dir() { + return 0 + } + return md.len() as u64; + } + + #[cfg(unix)] { + return md.blocks() as u64; + } +} + // Get maximum string sizes for: // - number of links of each items @@ -1399,7 +1412,7 @@ fn get_max_sizes (items: &[PathData], config: &Config) -> (usize, usize, usize, if let Some(md) = item.md() { max_links = display_symlink_count(md).len().max(max_links); max_width = display_size_or_rdev(md, config).len().max(max_width); - max_blocks = md.blocks().to_string().len().max(max_blocks); + max_blocks = get_display_size(md).to_string().len().max(max_blocks); total_size += get_block_size(md, config); } } @@ -1541,7 +1554,7 @@ fn display_item_long( let _ = write!( out, "{} ", - pad_left(md.blocks().to_string(), max_blocks) + pad_left(get_display_size(md).to_string(), max_blocks) ); } @@ -1839,7 +1852,7 @@ fn display_file_name(path: &PathData, config: &Config, max_blocks: usize) -> Opt if config.format != Format::Long && config.blocks { if let Some(md) = path.md() { width += max_blocks + 1; - name.insert_str(0, &format!("{} ", pad_left(md.blocks().to_string(), max_blocks))); + name.insert_str(0, &format!("{} ", pad_left(get_display_size(md).to_string(), max_blocks))); } else { width += max_blocks + 1; name.insert_str(0, &format!("{} ", pad_left(String::from(""), max_blocks))); diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 44d14c30416..0e7f4adc844 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2046,3 +2046,46 @@ fn test_ls_dangling_symlinks() { .succeeds() // this should fail, though at the moment, ls lacks a way to propagate errors encountered during display .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); } + +#[test] +fn test_ls_block_size_display_short() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.mkdir("dir-test"); + at.touch("file-1"); + at.append("file-1", &"x".repeat(1000)); + at.touch("file-2"); + at.append("file-2", &"x".repeat(100000)); + + let result = scene.ucmd().arg("-s").succeeds(); + #[cfg(not(windows))] + result.stdout_only(" 0 dir-test\n 8 file-1\n200 file-2\n"); + #[cfg(windows)] + result.stdout_only(" 0 dir-test\n 1000 file-1\n100000 file-2\n"); +} + + +#[test] +fn test_ls_block_size_display_long() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.mkdir("dir-test"); + at.touch("file-1"); + at.append("file-1", &"x".repeat(1000)); + at.touch("file-2"); + at.append("file-2", &"x".repeat(100000)); + + let result = scene.ucmd().arg("-ls").succeeds(); + #[cfg(not(windows))] { + result.stdout_contains("\n 0 "); + result.stdout_contains("\n 8 "); // for file-1 + result.stdout_contains("\n200 "); // for file- + } + #[cfg(windows)] { + result.stdout_contains("\n 0 "); + result.stdout_contains("\n 1000 "); // for file-1 + result.stdout_contains("\n100000 "); // for file-2 + } +} From b0936cc1817cf5504f914829b182288cd4fed1e1 Mon Sep 17 00:00:00 2001 From: Anirudh Sanjeev Date: Mon, 12 Jul 2021 20:32:41 -0400 Subject: [PATCH 3/5] Formatting and test fixes --- src/uu/ls/src/ls.rs | 30 ++++++++++++++++-------------- tests/by-util/test_ls.rs | 21 ++++++++++++++++----- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index e0d89d70364..6a9feb6d6c1 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1387,26 +1387,27 @@ fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { } fn get_display_size(md: &Metadata) -> u64 { - #[cfg(windows)] { + #[cfg(windows)] + { if md.is_dir() { - return 0 + return 0; } return md.len() as u64; } - #[cfg(unix)] { + #[cfg(unix)] + { return md.blocks() as u64; } } - // Get maximum string sizes for: // - number of links of each items // - size // - blocks -fn get_max_sizes (items: &[PathData], config: &Config) -> (usize, usize, usize, u64) { +fn get_max_sizes(items: &[PathData], config: &Config) -> (usize, usize, usize, u64) { let (mut max_links, mut max_width, mut max_blocks) = (1, 1, 1); - let mut total_size = 0; + let mut total_size = 0; for item in items { if let Some(md) = item.md() { @@ -1420,14 +1421,13 @@ fn get_max_sizes (items: &[PathData], config: &Config) -> (usize, usize, usize, return (max_links, max_width, max_blocks, total_size); } -fn pad_left(string: String, count: usize) -> String { +fn pad_left(string: impl Display, count: usize) -> String { format!("{:>width$}", string, width = count) } fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter) { let (max_links, max_width, max_blocks, total_size) = get_max_sizes(items, config); if config.format == Format::Long { - if total_size > 0 { let _ = writeln!(out, "total {}", display_size(total_size, config)); } @@ -1436,7 +1436,9 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter { @@ -1596,7 +1598,6 @@ fn get_inode(metadata: &Metadata) -> String { format!("{:8}", metadata.ino()) } - // Currently getpwuid is `linux` target only. If it's broken out into // a posix-compliant attribute this can be updated... #[cfg(unix)] @@ -1850,16 +1851,17 @@ fn display_file_name(path: &PathData, config: &Config, max_blocks: usize) -> Opt } if config.format != Format::Long && config.blocks { + width += max_blocks + 1; if let Some(md) = path.md() { - width += max_blocks + 1; - name.insert_str(0, &format!("{} ", pad_left(get_display_size(md).to_string(), max_blocks))); + name.insert_str( + 0, + &format!("{} ", pad_left(get_display_size(md), max_blocks)), + ); } else { - width += max_blocks + 1; name.insert_str(0, &format!("{} ", pad_left(String::from(""), max_blocks))); } } - if config.format == Format::Long && path.file_type()?.is_symlink() { if let Ok(target) = path.p_buf.read_link() { name.push_str(" -> "); diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 0e7f4adc844..8604b913439 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2059,13 +2059,15 @@ fn test_ls_block_size_display_short() { at.append("file-2", &"x".repeat(100000)); let result = scene.ucmd().arg("-s").succeeds(); - #[cfg(not(windows))] + // Block sizes vary by OS. Only values for the following OS'es are checked + #[cfg(target_os = "macos")] result.stdout_only(" 0 dir-test\n 8 file-1\n200 file-2\n"); - #[cfg(windows)] + #[cfg(any(target_os = "linux", target_os="freebsd"))] + result.stdout_only(" 8 dir-test\n 8 file-1\n200 file-2\n"); + #[cfg(target_os = "windows")] result.stdout_only(" 0 dir-test\n 1000 file-1\n100000 file-2\n"); } - #[test] fn test_ls_block_size_display_long() { let scene = TestScenario::new(util_name!()); @@ -2078,12 +2080,21 @@ fn test_ls_block_size_display_long() { at.append("file-2", &"x".repeat(100000)); let result = scene.ucmd().arg("-ls").succeeds(); - #[cfg(not(windows))] { + // Block sizes vary by OS. Only values for the following OS'es are checked + #[cfg(target_os = "macos")] + { result.stdout_contains("\n 0 "); result.stdout_contains("\n 8 "); // for file-1 result.stdout_contains("\n200 "); // for file- } - #[cfg(windows)] { + #[cfg(any(target_os = "linux", target_os="freebsd"))] + { + result.stdout_contains("\n 8 "); // for directory + result.stdout_contains("\n 8 "); // for file-1 + result.stdout_contains("\n200 "); // for file-2 + } + #[cfg(target_os = "windows")] + { result.stdout_contains("\n 0 "); result.stdout_contains("\n 1000 "); // for file-1 result.stdout_contains("\n100000 "); // for file-2 From 080cae1e2e0da98fc7d7f1a94a654b32bfbb8199 Mon Sep 17 00:00:00 2001 From: Anirudh Sanjeev Date: Mon, 12 Jul 2021 20:45:02 -0400 Subject: [PATCH 4/5] Force -w=0 for ls -s unit test --- tests/by-util/test_ls.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 8604b913439..e5869a149e4 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2058,7 +2058,8 @@ fn test_ls_block_size_display_short() { at.touch("file-2"); at.append("file-2", &"x".repeat(100000)); - let result = scene.ucmd().arg("-s").succeeds(); + let result = scene.ucmd().arg("-s").arg("-w=0").succeeds(); + // Block sizes vary by OS. Only values for the following OS'es are checked #[cfg(target_os = "macos")] result.stdout_only(" 0 dir-test\n 8 file-1\n200 file-2\n"); From e14e8857ad523c2330b871273d8ea272524768ae Mon Sep 17 00:00:00 2001 From: Anirudh Sanjeev Date: Tue, 13 Jul 2021 11:49:36 -0400 Subject: [PATCH 5/5] Style and clippy fixes --- src/uu/ls/src/ls.rs | 6 +++--- tests/by-util/test_ls.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 6a9feb6d6c1..c78afd3c85c 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1392,12 +1392,12 @@ fn get_display_size(md: &Metadata) -> u64 { if md.is_dir() { return 0; } - return md.len() as u64; + md.len() as u64 } #[cfg(unix)] { - return md.blocks() as u64; + md.blocks() as u64 } } @@ -1418,7 +1418,7 @@ fn get_max_sizes(items: &[PathData], config: &Config) -> (usize, usize, usize, u } } - return (max_links, max_width, max_blocks, total_size); + (max_links, max_width, max_blocks, total_size) } fn pad_left(string: impl Display, count: usize) -> String { diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index e5869a149e4..32ab099f4e8 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2063,7 +2063,7 @@ fn test_ls_block_size_display_short() { // Block sizes vary by OS. Only values for the following OS'es are checked #[cfg(target_os = "macos")] result.stdout_only(" 0 dir-test\n 8 file-1\n200 file-2\n"); - #[cfg(any(target_os = "linux", target_os="freebsd"))] + #[cfg(any(target_os = "linux", target_os = "freebsd"))] result.stdout_only(" 8 dir-test\n 8 file-1\n200 file-2\n"); #[cfg(target_os = "windows")] result.stdout_only(" 0 dir-test\n 1000 file-1\n100000 file-2\n"); @@ -2088,7 +2088,7 @@ fn test_ls_block_size_display_long() { result.stdout_contains("\n 8 "); // for file-1 result.stdout_contains("\n200 "); // for file- } - #[cfg(any(target_os = "linux", target_os="freebsd"))] + #[cfg(any(target_os = "linux", target_os = "freebsd"))] { result.stdout_contains("\n 8 "); // for directory result.stdout_contains("\n 8 "); // for file-1