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

@@ -583,8 +583,19 @@ impl Config {
                 "slash" => IndicatorStyle::Slash,
                 &_ => IndicatorStyle::None,
             }
-        } else if options.is_present(options::indicator_style::CLASSIFY) {
+        } else if let Some(field) = options.value_of(options::indicator_style::CLASSIFY) {
-            IndicatorStyle::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,

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 {