From 4a1bd78f482f31b183d07ff4d91cfbcd95ebd5cd Mon Sep 17 00:00:00 2001
From: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>
Date: Sun, 31 Mar 2024 21:15:37 +0200
Subject: [PATCH 1/2] ls: compute correct exit code on error

Note in particular that this seems to be the only tool where invalid
stringly-enum values cause a different exit code than invalid arguments.
---
 src/uu/ls/src/ls.rs      | 20 +++++++++++++--
 tests/by-util/test_ls.rs | 53 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs
index 333360f50..209745ade 100644
--- a/src/uu/ls/src/ls.rs
+++ b/src/uu/ls/src/ls.rs
@@ -3,7 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm
+// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm stringly
 
 use clap::{
     builder::{NonEmptyStringValueParser, ValueParser},
@@ -36,6 +36,7 @@ use std::{
 };
 use term_grid::{Cell, Direction, Filling, Grid, GridOptions};
 use unicode_width::UnicodeWidthStr;
+use uucore::error::USimpleError;
 #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
 use uucore::fsxattr::has_acl;
 #[cfg(any(
@@ -1150,7 +1151,22 @@ impl Config {
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let command = uu_app();
 
-    let matches = command.try_get_matches_from(args)?;
+    let matches = match command.try_get_matches_from(args) {
+        // clap successfully parsed the arguments:
+        Ok(matches) => matches,
+        // --help, --version, etc.:
+        Err(e) if e.exit_code() == 0 => {
+            return Err(e.into());
+        }
+        // Errors in argument *values* cause exit code 1:
+        Err(e) if e.kind() == clap::error::ErrorKind::InvalidValue => {
+            return Err(USimpleError::new(1, e.to_string()));
+        }
+        // All other argument parsing errors cause exit code 2:
+        Err(e) => {
+            return Err(USimpleError::new(2, e.to_string()));
+        }
+    };
 
     let config = Config::from(&matches)?;
 
diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs
index 3f0154f7a..efe9e1056 100644
--- a/tests/by-util/test_ls.rs
+++ b/tests/by-util/test_ls.rs
@@ -44,11 +44,64 @@ const COMMA_ARGS: &[&str] = &["-m", "--format=commas", "--for=commas"];
 
 const COLUMN_ARGS: &[&str] = &["-C", "--format=columns", "--for=columns"];
 
+#[test]
+fn test_invalid_flag() {
+    new_ucmd!()
+        .arg("--invalid-argument")
+        .fails()
+        .no_stdout()
+        .code_is(2);
+}
+
+#[test]
+fn test_invalid_value_returns_1() {
+    // Invalid values to flags *sometimes* result in error code 1:
+    for flag in [
+        "--classify",
+        "--color",
+        "--format",
+        "--hyperlink",
+        "--indicator-style",
+        "--quoting-style",
+        "--sort",
+        "--time",
+    ] {
+        new_ucmd!()
+            .arg(&format!("{flag}=definitely_invalid_value"))
+            .fails()
+            .no_stdout()
+            .code_is(1);
+    }
+}
+
+#[test]
+fn test_invalid_value_returns_2() {
+    // Invalid values to flags *sometimes* result in error code 2:
+    for flag in ["--block-size", "--width", "--tab-size"] {
+        new_ucmd!()
+            .arg(&format!("{flag}=definitely_invalid_value"))
+            .fails()
+            .no_stdout()
+            .code_is(2);
+    }
+}
+
 #[test]
 fn test_ls_ls() {
     new_ucmd!().succeeds();
 }
 
+#[test]
+fn test_ls_help() {
+    // Because we have to work around a lot of clap's error handling and
+    // modify the exit-code, this is actually non-trivial.
+    new_ucmd!()
+        .arg("--help")
+        .succeeds()
+        .stdout_contains("--version")
+        .no_stderr();
+}
+
 #[test]
 fn test_ls_i() {
     new_ucmd!().arg("-i").succeeds();

From 714b4ff589a4f04bc1702937882b64dc13c76f50 Mon Sep 17 00:00:00 2001
From: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>
Date: Mon, 1 Apr 2024 04:41:24 +0200
Subject: [PATCH 2/2] ls: fix exit code for --time-style when used

---
 src/uu/ls/src/ls.rs      |  7 ++++++-
 tests/by-util/test_ls.rs | 25 +++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs
index 209745ade..ec7fab4e9 100644
--- a/src/uu/ls/src/ls.rs
+++ b/src/uu/ls/src/ls.rs
@@ -966,7 +966,12 @@ impl Config {
 
         let mut quoting_style = extract_quoting_style(options, show_control);
         let indicator_style = extract_indicator_style(options);
-        let time_style = parse_time_style(options)?;
+        // Only parse the value to "--time-style" if it will become relevant.
+        let time_style = if format == Format::Long {
+            parse_time_style(options)?
+        } else {
+            TimeStyle::Iso
+        };
 
         let mut ignore_patterns: Vec<Pattern> = Vec::new();
 
diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs
index efe9e1056..2f48e4460 100644
--- a/tests/by-util/test_ls.rs
+++ b/tests/by-util/test_ls.rs
@@ -86,6 +86,31 @@ fn test_invalid_value_returns_2() {
     }
 }
 
+#[test]
+fn test_invalid_value_time_style() {
+    // This is the only flag which does not raise an error if it is invalid but not actually used:
+    new_ucmd!()
+        .arg("--time-style=definitely_invalid_value")
+        .succeeds()
+        .no_stderr()
+        .code_is(0);
+    // If it is used, error:
+    new_ucmd!()
+        .arg("-g")
+        .arg("--time-style=definitely_invalid_value")
+        .fails()
+        .no_stdout()
+        .code_is(2);
+    // If it only looks temporarily like it might be used, no error:
+    new_ucmd!()
+        .arg("-l")
+        .arg("--time-style=definitely_invalid_value")
+        .arg("--format=single-column")
+        .succeeds()
+        .no_stderr()
+        .code_is(0);
+}
+
 #[test]
 fn test_ls_ls() {
     new_ucmd!().succeeds();