ls: compute the correct exit code

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(
@@ -965,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();
 
@@ -1150,7 +1156,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)?;
 

tests/by-util/test_ls.rs

@@ -44,11 +44,89 @@ 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_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();
 }
 
+#[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();