From 3f6fe7f3886b8dddbda57f51703c97092413051e Mon Sep 17 00:00:00 2001 From: Abhishek C Sharma Date: Thu, 10 Feb 2022 12:35:20 -0800 Subject: [PATCH] ls: add new optional arguments to --classify flag (#3041) * ls: add new optional arguments to --classify flag The --classify flag in ls now takes an option when argument that may have the values always, auto and none. Modified clap argument to allow an optional parameter and changed the classify flag value parsing logic to account for this change. * ls: add test for indicator-style, ind and classify with value none * ls: require option paramter to --classify to use a = to specify flag value * ls: account for all the undocumented possible values for the --classify flag Added the other values for the --classify flag along with modifications to tests. Also documented the inconsistency between GNU coreutils because we accept the flag value even for the short version of the flag. --- src/uu/ls/src/ls.rs | 36 +++++++++++++++++++++++++++++++++--- tests/by-util/test_ls.rs | 21 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7fdec53f0..d82915c85 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -583,8 +583,19 @@ impl Config { "slash" => IndicatorStyle::Slash, &_ => IndicatorStyle::None, } - } else if options.is_present(options::indicator_style::CLASSIFY) { - IndicatorStyle::Classify + } else if let Some(field) = options.value_of(options::indicator_style::CLASSIFY) { + match field { + "never" | "no" | "none" => IndicatorStyle::None, + "always" | "yes" | "force" => IndicatorStyle::Classify, + "auto" | "tty" | "if-tty" => { + if atty::is(atty::Stream::Stdout) { + IndicatorStyle::Classify + } else { + IndicatorStyle::None + } + } + &_ => IndicatorStyle::None, + } } else if options.is_present(options::indicator_style::SLASH) { IndicatorStyle::Slash } else if options.is_present(options::indicator_style::FILE_TYPE) { @@ -1202,6 +1213,11 @@ only ignore '.' and '..'.", ]), ) .arg( + // The --classify flag can take an optional when argument to + // control its behavior from version 9 of GNU coreutils. + // There is currently an inconsistency where GNU coreutils allows only + // the long form of the flag to take the argument while we allow it + // for both the long and short form of the flag. Arg::new(options::indicator_style::CLASSIFY) .short('F') .long(options::indicator_style::CLASSIFY) @@ -1209,8 +1225,22 @@ only ignore '.' and '..'.", "Append a character to each file name indicating the file type. Also, for \ regular files that are executable, append '*'. The file type indicators are \ '/' for directories, '@' for symbolic links, '|' for FIFOs, '=' for sockets, \ - '>' for doors, and nothing for regular files.", + '>' for doors, and nothing for regular files. when may be omitted, or one of:\n\ + \tnone - Do not classify. This is the default.\n\ + \tauto - Only classify if standard output is a terminal.\n\ + \talways - Always classify.\n\ + Specifying --classify and no when is equivalent to --classify=always. This will not follow\ + symbolic links listed on the command line unless the --dereference-command-line (-H),\ + --dereference (-L), or --dereference-command-line-symlink-to-dir options are specified.", ) + .takes_value(true) + .value_name("when") + .possible_values(&[ + "always", "yes", "force", "auto", "tty", "if-tty", "never", "no", "none", + ]) + .default_missing_value("always") + .require_equals(true) + .min_values(0) .overrides_with_all(&[ options::indicator_style::FILE_TYPE, options::indicator_style::SLASH, diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index f61611390..19947c05a 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -1555,6 +1555,9 @@ fn test_ls_indicator_style() { "--indicator-style=slash", "--ind=slash", "--classify", + "--classify=always", + "--classify=yes", + "--classify=force", "--class", "--file-type", "--file", @@ -1564,6 +1567,24 @@ fn test_ls_indicator_style() { scene.ucmd().arg(opt).succeeds().stdout_contains(&"/"); } + // Classify, Indicator options should not contain any indicators when value is none. + for opt in [ + "--indicator-style=none", + "--ind=none", + "--classify=none", + "--classify=never", + "--classify=no", + ] { + // Verify that there are no indicators for any of the file types. + scene + .ucmd() + .arg(opt) + .succeeds() + .stdout_does_not_contain(&"/") + .stdout_does_not_contain(&"@") + .stdout_does_not_contain(&"|"); + } + // Classify and File-Type all contain indicators for pipes and links. let options = vec!["classify", "file-type"]; for opt in options {