From 955c547adffda3546ab6b9fb58d7d508f439eb3f Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Fri, 26 Mar 2021 19:12:01 +0100 Subject: [PATCH] ls: overrideable `-n` option (#1917) --- src/uu/ls/src/ls.rs | 87 +++++++++++++++++++--------------------- tests/by-util/test_ls.rs | 61 ++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 45 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 8714a0fa1..201ddc7a6 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -84,6 +84,7 @@ pub mod options { pub static COMMAS: &str = "m"; pub static LONG_NO_OWNER: &str = "g"; pub static LONG_NO_GROUP: &str = "o"; + pub static LONG_NUMERIC_UID_GID: &str = "numeric-uid-gid"; } pub mod files { pub static ALL: &str = "all"; @@ -114,7 +115,6 @@ pub mod options { pub static CLASSIFY: &str = "classify"; pub static INODE: &str = "inode"; pub static DEREFERENCE: &str = "dereference"; - pub static NUMERIC_UID_GID: &str = "numeric-uid-gid"; pub static REVERSE: &str = "reverse"; pub static RECURSIVE: &str = "recursive"; pub static COLOR: &str = "color"; @@ -167,7 +167,6 @@ struct Config { classify: bool, ignore_backups: bool, size_format: SizeFormat, - numeric_uid_gid: bool, directory: bool, time: Time, #[cfg(unix)] @@ -183,6 +182,8 @@ struct LongFormat { author: bool, group: bool, owner: bool, + #[cfg(unix)] + numeric_uid_gid: bool, } impl Config { @@ -210,7 +211,7 @@ impl Config { (Format::Columns, options::format::COLUMNS) }; - // The -o and -g options are tricky. They cannot override with each + // The -o, -n and -g options are tricky. They cannot override with each // other because it's possible to combine them. For example, the option // -og should hide both owner and group. Furthermore, they are not // reset if -l or --format=long is used. So these should just show the @@ -223,42 +224,26 @@ impl Config { // which always applies. // // The idea here is to not let these options override with the other - // options, but manually check the last index they occur. If this index - // is larger than the index for the other format options, we apply the - // long format. - match options.indices_of(opt).map(|x| x.max().unwrap()) { - None => { - if options.is_present(options::format::LONG_NO_GROUP) - || options.is_present(options::format::LONG_NO_OWNER) - { - format = Format::Long; - } else if options.is_present(options::format::ONELINE) { - format = Format::OneLine; - } - } - Some(mut idx) => { - if let Some(indices) = options.indices_of(options::format::LONG_NO_OWNER) { - let i = indices.max().unwrap(); - if i > idx { - format = Format::Long; - idx = i; - } - } - if let Some(indices) = options.indices_of(options::format::LONG_NO_GROUP) { - let i = indices.max().unwrap(); - if i > idx { - format = Format::Long; - idx = i; - } - } - if let Some(indices) = options.indices_of(options::format::ONELINE) { - let i = indices.max().unwrap(); - if i > idx && format != Format::Long { + // options, but manually whether they have an index that's greater than + // the other format options. If so, we set the appropriate format. + if format != Format::Long { + let idx = options.indices_of(opt).map(|x| x.max().unwrap()).unwrap_or(0); + if [options::format::LONG_NO_OWNER, options::format::LONG_NO_GROUP, options::format::LONG_NUMERIC_UID_GID] + .iter() + .flat_map(|opt| options.indices_of(opt)) + .flatten() + .any(|i| i >= idx) + { + format = Format::Long; + } else { + if let Some(mut indices) = options.indices_of(options::format::ONELINE) { + if indices.any(|i| i > idx) { format = Format::OneLine; } } } } + let files = if options.is_present(options::files::ALL) { Files::All @@ -328,10 +313,14 @@ impl Config { let group = !options.is_present(options::NO_GROUP) && !options.is_present(options::format::LONG_NO_GROUP); let owner = !options.is_present(options::format::LONG_NO_OWNER); + #[cfg(unix)] + let numeric_uid_gid = options.is_present(options::format::LONG_NUMERIC_UID_GID); LongFormat { author, group, owner, + #[cfg(unix)] + numeric_uid_gid, } }; @@ -355,7 +344,6 @@ impl Config { classify: options.is_present(options::CLASSIFY), ignore_backups: options.is_present(options::IGNORE_BACKUPS), size_format, - numeric_uid_gid: options.is_present(options::NUMERIC_UID_GID), directory: options.is_present(options::DIRECTORY), time, #[cfg(unix)] @@ -444,22 +432,36 @@ pub fn uumain(args: impl uucore::Args) -> i32 { options::format::COLUMNS, ]), ) - // The next three arguments do not override with the other format + // The next four arguments do not override with the other format // options, see the comment in Config::from for the reason. + // Ideally, they would use Arg::override_with, with their own name + // but that doesn't seem to work in all cases. Example: + // ls -1g1 + // even though `ls -11` and `ls -1 -g -1` work. .arg( Arg::with_name(options::format::ONELINE) .short(options::format::ONELINE) .help("List one file per line.") + .multiple(true) ) .arg( Arg::with_name(options::format::LONG_NO_GROUP) .short(options::format::LONG_NO_GROUP) .help("Long format without group information. Identical to --format=long with --no-group.") + .multiple(true) ) .arg( Arg::with_name(options::format::LONG_NO_OWNER) .short(options::format::LONG_NO_OWNER) .help("Long format without owner information.") + .multiple(true) + ) + .arg( + Arg::with_name(options::format::LONG_NUMERIC_UID_GID) + .short("n") + .long(options::format::LONG_NUMERIC_UID_GID) + .help("-l with numeric UIDs and GIDs.") + .multiple(true) ) // Time arguments @@ -657,12 +659,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { file the link references rather than the link itself.", ), ) - .arg( - Arg::with_name(options::NUMERIC_UID_GID) - .short("n") - .long(options::NUMERIC_UID_GID) - .help("-l with numeric UIDs and GIDs."), - ) + .arg( Arg::with_name(options::REVERSE) .short("r") @@ -852,7 +849,7 @@ fn pad_left(string: String, count: usize) -> String { } fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config) { - if config.format == Format::Long || config.numeric_uid_gid { + if config.format == Format::Long { let (mut max_links, mut max_size) = (1, 1); for item in items { let (links, size) = display_dir_entry_size(item, config); @@ -994,7 +991,7 @@ use uucore::entries; #[cfg(unix)] fn display_uname(metadata: &Metadata, config: &Config) -> String { - if config.numeric_uid_gid { + if config.long.numeric_uid_gid { metadata.uid().to_string() } else { entries::uid2usr(metadata.uid()).unwrap_or_else(|_| metadata.uid().to_string()) @@ -1003,7 +1000,7 @@ fn display_uname(metadata: &Metadata, config: &Config) -> String { #[cfg(unix)] fn display_group(metadata: &Metadata, config: &Config) -> String { - if config.numeric_uid_gid { + if config.long.numeric_uid_gid { metadata.gid().to_string() } else { entries::gid2grp(metadata.gid()).unwrap_or_else(|_| metadata.gid().to_string()) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 091d47234..7d5a3da7b 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -297,15 +297,24 @@ fn test_ls_long_formats() { // Regex for three names, so all of author, group and owner let re_three = Regex::new(r"[xrw-]{9} \d ([-0-9_a-z]+ ){3}0").unwrap(); + #[cfg(unix)] + let re_three_num = Regex::new(r"[xrw-]{9} \d (\d+ ){3}0").unwrap(); + // Regex for two names, either: // - group and owner // - author and owner // - author and group let re_two = Regex::new(r"[xrw-]{9} \d ([-0-9_a-z]+ ){2}0").unwrap(); + #[cfg(unix)] + let re_two_num = Regex::new(r"[xrw-]{9} \d (\d+ ){2}0").unwrap(); + // Regex for one name: author, group or owner let re_one = Regex::new(r"[xrw-]{9} \d [-0-9_a-z]+ 0").unwrap(); + #[cfg(unix)] + let re_one_num = Regex::new(r"[xrw-]{9} \d \d+ 0").unwrap(); + // Regex for no names let re_zero = Regex::new(r"[xrw-]{9} \d 0").unwrap(); @@ -329,6 +338,19 @@ fn test_ls_long_formats() { println!("stdout = {:?}", result.stdout); assert!(re_three.is_match(&result.stdout)); + #[cfg(unix)] + { + let result = scene + .ucmd() + .arg("-n") + .arg("--author") + .arg("test-long-formats") + .succeeds(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(re_three_num.is_match(&result.stdout)); + } + for arg in &[ "-l", // only group and owner "-g --author", // only author and group @@ -344,6 +366,19 @@ fn test_ls_long_formats() { println!("stderr = {:?}", result.stderr); println!("stdout = {:?}", result.stdout); assert!(re_two.is_match(&result.stdout)); + + #[cfg(unix)] + { + let result = scene + .ucmd() + .arg("-n") + .args(&arg.split(" ").collect::>()) + .arg("test-long-formats") + .succeeds(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(re_two_num.is_match(&result.stdout)); + } } for arg in &[ @@ -364,6 +399,19 @@ fn test_ls_long_formats() { println!("stderr = {:?}", result.stderr); println!("stdout = {:?}", result.stdout); assert!(re_one.is_match(&result.stdout)); + + #[cfg(unix)] + { + let result = scene + .ucmd() + .arg("-n") + .args(&arg.split(" ").collect::>()) + .arg("test-long-formats") + .succeeds(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(re_one_num.is_match(&result.stdout)); + } } for arg in &[ @@ -387,6 +435,19 @@ fn test_ls_long_formats() { println!("stderr = {:?}", result.stderr); println!("stdout = {:?}", result.stdout); assert!(re_zero.is_match(&result.stdout)); + + #[cfg(unix)] + { + let result = scene + .ucmd() + .arg("-n") + .args(&arg.split(" ").collect::>()) + .arg("test-long-formats") + .succeeds(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(re_zero.is_match(&result.stdout)); + } } }