From 51eb20a15d768b0b24b60a78b98c27602ed5cc50 Mon Sep 17 00:00:00 2001
From: Daniel Hofstetter <daniel.hofstetter@42dh.com>
Date: Thu, 28 Sep 2023 10:50:56 +0200
Subject: [PATCH] fmt: use clap's value parser for goal & width

---
 src/uu/fmt/src/fmt.rs     | 30 ++++++++----------------------
 tests/by-util/test_fmt.rs | 22 ++++++++++++++++++++++
 2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/src/uu/fmt/src/fmt.rs b/src/uu/fmt/src/fmt.rs
index c5eac70730..c30d923b76 100644
--- a/src/uu/fmt/src/fmt.rs
+++ b/src/uu/fmt/src/fmt.rs
@@ -131,16 +131,8 @@ fn parse_arguments(args: impl uucore::Args) -> UResult<(Vec<String>, FmtOptions)
         fmt_opts.use_anti_prefix = true;
     };
 
-    if let Some(s) = matches.get_one::<String>(OPT_WIDTH) {
-        fmt_opts.width = match s.parse::<usize>() {
-            Ok(t) => t,
-            Err(e) => {
-                return Err(USimpleError::new(
-                    1,
-                    format!("Invalid WIDTH specification: {}: {}", s.quote(), e),
-                ));
-            }
-        };
+    if let Some(width) = matches.get_one::<usize>(OPT_WIDTH) {
+        fmt_opts.width = *width;
         if fmt_opts.width > MAX_WIDTH {
             return Err(USimpleError::new(
                 1,
@@ -156,16 +148,8 @@ fn parse_arguments(args: impl uucore::Args) -> UResult<(Vec<String>, FmtOptions)
         );
     };
 
-    if let Some(s) = matches.get_one::<String>(OPT_GOAL) {
-        fmt_opts.goal = match s.parse::<usize>() {
-            Ok(t) => t,
-            Err(e) => {
-                return Err(USimpleError::new(
-                    1,
-                    format!("Invalid GOAL specification: {}: {}", s.quote(), e),
-                ));
-            }
-        };
+    if let Some(goal) = matches.get_one::<usize>(OPT_GOAL) {
+        fmt_opts.goal = *goal;
         if !matches.contains_id(OPT_WIDTH) {
             fmt_opts.width = cmp::max(
                 fmt_opts.goal * 100 / DEFAULT_GOAL_TO_WIDTH_RATIO,
@@ -372,14 +356,16 @@ pub fn uu_app() -> Command {
                 .short('w')
                 .long("width")
                 .help("Fill output lines up to a maximum of WIDTH columns, default 75.")
-                .value_name("WIDTH"),
+                .value_name("WIDTH")
+                .value_parser(clap::value_parser!(usize)),
         )
         .arg(
             Arg::new(OPT_GOAL)
                 .short('g')
                 .long("goal")
                 .help("Goal width, default of 93% of WIDTH. Must be less than WIDTH.")
-                .value_name("GOAL"),
+                .value_name("GOAL")
+                .value_parser(clap::value_parser!(usize)),
         )
         .arg(
             Arg::new(OPT_QUICK)
diff --git a/tests/by-util/test_fmt.rs b/tests/by-util/test_fmt.rs
index 7d23cbd52f..4fd0590801 100644
--- a/tests/by-util/test_fmt.rs
+++ b/tests/by-util/test_fmt.rs
@@ -48,6 +48,17 @@ fn test_fmt_width_too_big() {
     }
 }
 
+#[test]
+fn test_fmt_invalid_width() {
+    for param in ["-w", "--width"] {
+        new_ucmd!()
+            .args(&["one-word-per-line.txt", param, "invalid"])
+            .fails()
+            .code_is(1)
+            .stderr_contains("invalid value 'invalid'");
+    }
+}
+
 #[ignore]
 #[test]
 fn test_fmt_goal() {
@@ -70,6 +81,17 @@ fn test_fmt_goal_too_big() {
     }
 }
 
+#[test]
+fn test_fmt_invalid_goal() {
+    for param in ["-g", "--goal"] {
+        new_ucmd!()
+            .args(&["one-word-per-line.txt", param, "invalid"])
+            .fails()
+            .code_is(1)
+            .stderr_contains("invalid value 'invalid'");
+    }
+}
+
 #[test]
 fn test_fmt_set_goal_not_contain_width() {
     for param in ["-g", "--goal"] {