From 5fb091a4fb14eed29894de19ab62212941945bb6 Mon Sep 17 00:00:00 2001
From: Joining7943 <111500881+Joining7943@users.noreply.github.com>
Date: Wed, 1 Mar 2023 07:35:53 +0100
Subject: [PATCH] sleep: Use fundu error types to improve error messages

---
 src/uu/sleep/src/sleep.rs   | 26 +++++++++++++++++++++-----
 tests/by-util/test_sleep.rs | 16 +++++++++-------
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/src/uu/sleep/src/sleep.rs b/src/uu/sleep/src/sleep.rs
index af6a0f22e..009986095 100644
--- a/src/uu/sleep/src/sleep.rs
+++ b/src/uu/sleep/src/sleep.rs
@@ -14,7 +14,7 @@ use uucore::{
 };
 
 use clap::{crate_version, Arg, ArgAction, Command};
-use fundu::{self, DurationParser};
+use fundu::{self, DurationParser, ParseError};
 
 static ABOUT: &str = help_about!("sleep.md");
 const USAGE: &str = help_usage!("sleep.md");
@@ -68,12 +68,28 @@ fn sleep(args: &[&str]) -> UResult<()> {
 
     let sleep_dur = args
         .iter()
-        .filter_map(|input| {
-            parser.parse(input.trim()).ok().or_else(|| {
+        .filter_map(|input| match parser.parse(input.trim()) {
+            Ok(duration) => Some(duration),
+            Err(error) => {
                 arg_error = true;
-                show_error!("invalid time interval '{input}'");
+
+                let reason = match error {
+                    ParseError::Empty if input.is_empty() => "Input was empty".to_string(),
+                    ParseError::Empty => "Found only whitespace in input".to_string(),
+                    ParseError::Syntax(pos, description)
+                    | ParseError::TimeUnit(pos, description) => {
+                        format!("{description} at position {}", pos.saturating_add(1))
+                    }
+                    ParseError::NegativeExponentOverflow | ParseError::PositiveExponentOverflow => {
+                        "Exponent was out of bounds".to_string()
+                    }
+                    ParseError::NegativeNumber => "Number was negative".to_string(),
+                    error => error.to_string(),
+                };
+                show_error!("invalid time interval '{input}': {reason}");
+
                 None
-            })
+            }
         })
         .fold(Duration::ZERO, |acc, n| acc.saturating_add(n));
 
diff --git a/tests/by-util/test_sleep.rs b/tests/by-util/test_sleep.rs
index c525cc0d2..8bb4b11ea 100644
--- a/tests/by-util/test_sleep.rs
+++ b/tests/by-util/test_sleep.rs
@@ -10,11 +10,11 @@ fn test_invalid_time_interval() {
     new_ucmd!()
         .arg("xyz")
         .fails()
-        .usage_error("invalid time interval 'xyz'");
+        .usage_error("invalid time interval 'xyz': Invalid character: 'x' at position 1");
     new_ucmd!()
         .args(&["--", "-1"])
         .fails()
-        .usage_error("invalid time interval '-1'");
+        .usage_error("invalid time interval '-1': Number was negative");
 }
 
 #[test]
@@ -204,14 +204,16 @@ fn test_sleep_when_input_has_only_whitespace_then_error(#[case] input: &str) {
         .arg(input)
         .timeout(Duration::from_secs(10))
         .fails()
-        .usage_error(format!("invalid time interval '{input}'"));
+        .usage_error(format!(
+            "invalid time interval '{input}': Found only whitespace in input"
+        ));
 }
 
 #[test]
 fn test_sleep_when_multiple_input_some_with_error_then_shows_all_errors() {
-    let expected = "invalid time interval 'abc'\n\
-        sleep: invalid time interval '1years'\n\
-        sleep: invalid time interval ' '";
+    let expected = "invalid time interval 'abc': Invalid character: 'a' at position 1\n\
+        sleep: invalid time interval '1years': Invalid time unit: 'years' at position 2\n\
+        sleep: invalid time interval ' ': Found only whitespace in input";
 
     // Even if one of the arguments is valid, but the rest isn't, we should still fail and exit early.
     // So, the timeout of 10 seconds ensures we haven't executed `thread::sleep` with the only valid
@@ -228,5 +230,5 @@ fn test_negative_interval() {
     new_ucmd!()
         .args(&["--", "-1"])
         .fails()
-        .usage_error("invalid time interval '-1'");
+        .usage_error("invalid time interval '-1': Number was negative");
 }