diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 3d957474c..bf449d96b 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1394,14 +1394,17 @@ fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { } } -fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize) { +fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize, usize, usize) { + // TODO: Cache/memoize the display_* results so we don't have to recalculate them. if let Some(md) = entry.md() { ( display_symlink_count(md).len(), + display_uname(md, config).len(), + display_group(md, config).len(), display_size_or_rdev(md, config).len(), ) } else { - (0, 0) + (0, 0, 0, 0) } } @@ -1409,15 +1412,28 @@ fn pad_left(string: String, count: usize) -> String { format!("{:>width$}", string, width = count) } +fn pad_right(string: String, count: usize) -> String { + format!("{:) { if config.format == Format::Long { - let (mut max_links, mut max_width) = (1, 1); + let ( + mut longest_link_count_len, + mut longest_uname_len, + mut longest_group_len, + mut longest_size_len, + ) = (1, 1, 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); + let (link_count_len, uname_len, group_len, size_len) = + display_dir_entry_size(item, config); + longest_link_count_len = link_count_len.max(longest_link_count_len); + longest_size_len = size_len.max(longest_size_len); + longest_uname_len = uname_len.max(longest_uname_len); + longest_group_len = group_len.max(longest_group_len); + longest_size_len = size_len.max(longest_size_len); total_size += item.md().map_or(0, |md| get_block_size(md, config)); } @@ -1426,7 +1442,15 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter, ) { @@ -1557,27 +1619,39 @@ fn display_item_long( out, "{} {}", display_permissions(md, true), - pad_left(display_symlink_count(md), max_links), + pad_left(display_symlink_count(md), longest_link_count_len), ); if config.long.owner { - let _ = write!(out, " {}", display_uname(md, config)); + let _ = write!( + out, + " {}", + pad_right(display_uname(md, config), longest_uname_len) + ); } if config.long.group { - let _ = write!(out, " {}", display_group(md, config)); + let _ = write!( + out, + " {}", + pad_right(display_group(md, config), longest_group_len) + ); } // Author is only different from owner on GNU/Hurd, so we reuse // the owner, since GNU/Hurd is not currently supported by Rust. if config.long.author { - let _ = write!(out, " {}", display_uname(md, config)); + let _ = write!( + out, + " {}", + pad_right(display_uname(md, config), longest_uname_len) + ); } let _ = writeln!( out, " {} {} {}", - pad_left(display_size_or_rdev(md, config), max_size), + pad_left(display_size_or_rdev(md, config), longest_size_len), 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 diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index a3372050a..546fb86a8 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -333,6 +333,61 @@ fn test_ls_long() { } } +#[test] +fn test_ls_long_format() { + #[cfg(not(windows))] + let last; + #[cfg(not(windows))] + { + let _guard = UMASK_MUTEX.lock(); + last = unsafe { umask(0) }; + + unsafe { + umask(0o002); + } + } + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir(&at.plus_as_string("test-long-dir")); + at.touch(&at.plus_as_string("test-long-dir/test-long-file")); + at.mkdir(&at.plus_as_string("test-long-dir/test-long-dir")); + + for arg in &["-l", "--long", "--format=long", "--format=verbose"] { + let result = scene.ucmd().arg(arg).arg("test-long-dir").succeeds(); + // Assuming sane username do not have spaces within them. + // A line of the output should be: + // One of the characters -bcCdDlMnpPsStTx? + // rwx, with - for missing permissions, thrice. + // A number, preceded by column whitespace, and followed by a single space. + // A username, currently [^ ], followed by column whitespace, twice (or thrice for Hurd). + // A number, followed by a single space. + // A month, followed by a single space. + // A day, preceded by column whitespace, and followed by a single space. + // Either a year or a time, currently [0-9:]+, preceded by column whitespace, + // and followed by a single space. + // Whatever comes after is irrelevant to this specific test. + #[cfg(not(windows))] + result.stdout_matches(&Regex::new( + r"\n[-bcCdDlMnpPsStTx?]([r-][w-][xt-]){3} +\d+ [^ ]+ +[^ ]+( +[^ ]+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ " + ).unwrap()); + } + + let result = scene.ucmd().arg("-lan").arg("test-long-dir").succeeds(); + // This checks for the line with the .. entry. The uname and group should be digits. + #[cfg(not(windows))] + result.stdout_matches(&Regex::new( + r"\nd([r-][w-][xt-]){3} +\d+ \d+ +\d+( +\d+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ \.\." + ).unwrap()); + + #[cfg(not(windows))] + { + unsafe { + umask(last); + } + } +} + #[test] fn test_ls_long_total_size() { let scene = TestScenario::new(util_name!());