1
Fork 0
mirror of https://github.com/RGBCube/uutils-coreutils synced 2025-07-28 11:37:44 +00:00

Proper columns for ls -l (#2623)

* Used .as_path() and .as_str() when required:

when the argument required is a Path and not a PathBuf,
or an str and not a Path, respectively.

* Changed display_items to take Vec<PathData>, which is passed, instead of [PathData]

* Added a pad_right function.

* Implemented column-formating to mimic the behavior of GNU coreutils's ls

Added returns in display_dir_entry_size that keep track of uname and
group lengths.
Renamed variables to make more sense.
Changed display_item_long to take all the lengths it needs to render
correctly.
Implemented owner, group, and author padding right to mimic GNU ls.

* Added a todo for future quality-of-life cache addition.

* Documented display_item_long, as a first step in documenting all functions.

* Revert "Used .as_path() and .as_str() when required:"

This reverts commit b88db6a8170f827a7adc58de14acb59f19be2db1.

* Revert "Changed display_items to take Vec<PathData>, which is passed, instead of [PathData]"

This reverts commit 0c690dda8d5eb1b257afb4c74d9c2dfc1f7cc97b.

* Ran cargo fmt to get rid of Style/format `fmt` testing error.

* Added a test for `ls -l` and `ls -lan` line formats.

* Changed uname -> username for cspell. Removed extra blank line for rustfmt.
This commit is contained in:
Mahmoud Soltan 2021-08-30 23:09:16 +02:00 committed by GitHub
parent d247cbd7e5
commit 97a0c06ff4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 143 additions and 14 deletions

View file

@ -1394,14 +1394,17 @@ fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result<Metadata> {
}
}
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!("{:<width$}", string, width = count)
}
fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout>) {
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<Stdout
}
for item in items {
display_item_long(item, max_links, max_width, config, out);
display_item_long(
item,
longest_link_count_len,
longest_uname_len,
longest_group_len,
longest_size_len,
config,
out,
);
}
} else {
let names = items.iter().filter_map(|i| display_file_name(i, config));
@ -1531,10 +1555,48 @@ fn display_grid(
}
}
/// This writes to the BufWriter out a single string of the output of `ls -l`.
///
/// It writes the following keys, in order:
/// * `inode` ([`get_inode`], config-optional)
/// * `permissions` ([`display_permissions`])
/// * `symlink_count` ([`display_symlink_count`])
/// * `owner` ([`display_uname`], config-optional)
/// * `group` ([`display_group`], config-optional)
/// * `author` ([`display_uname`], config-optional)
/// * `size / rdev` ([`display_size_or_rdev`])
/// * `system_time` ([`get_system_time`])
/// * `file_name` ([`display_file_name`])
///
/// This function needs to display information in columns:
/// * permissions and system_time are already guaranteed to be pre-formatted in fixed length.
/// * file_name is the last column and is left-aligned.
/// * Everything else needs to be padded using [`pad_left`].
///
/// That's why we have the parameters:
/// ```txt
/// max_links: usize,
/// longest_uname_len: usize,
/// longest_group_len: usize,
/// max_size: usize,
/// ```
/// that decide the maximum possible character count of each field.
///
/// [`get_inode`]: ls::get_inode
/// [`display_permissions`]: ls::display_permissions
/// [`display_symlink_count`]: ls::display_symlink_count
/// [`display_uname`]: ls::display_uname
/// [`display_group`]: ls::display_group
/// [`display_size_or_rdev`]: ls::display_size_or_rdev
/// [`get_system_time`]: ls::get_system_time
/// [`display_file_name`]: ls::display_file_name
/// [`pad_left`]: ls::pad_left
fn display_item_long(
item: &PathData,
max_links: usize,
max_size: usize,
longest_link_count_len: usize,
longest_uname_len: usize,
longest_group_len: usize,
longest_size_len: usize,
config: &Config,
out: &mut BufWriter<Stdout>,
) {
@ -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

View file

@ -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!());