From aef130dae7840dec3c2234eb9416de34a9893e25 Mon Sep 17 00:00:00 2001
From: Daniel Hofstetter <daniel.hofstetter@42dh.com>
Date: Thu, 20 Jul 2023 15:26:46 +0200
Subject: [PATCH] nl: show error if --join-blank-lines is zero

---
 src/uu/nl/src/helper.rs  | 19 +++++++------------
 src/uu/nl/src/nl.rs      |  3 ++-
 tests/by-util/test_nl.rs | 21 ++++++++++++++++++++-
 3 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/src/uu/nl/src/helper.rs b/src/uu/nl/src/helper.rs
index 2a90cb130f7..29c75af6968 100644
--- a/src/uu/nl/src/helper.rs
+++ b/src/uu/nl/src/helper.rs
@@ -86,23 +86,18 @@ pub fn parse_options(settings: &mut crate::Settings, opts: &clap::ArgMatches) ->
             "Invalid line number field width: ‘0’: Numerical result out of range",
         )),
     }
+    match opts.get_one::<u64>(options::JOIN_BLANK_LINES) {
+        None => {}
+        Some(num) if *num > 0 => settings.join_blank_lines = *num,
+        Some(_) => errs.push(String::from(
+            "Invalid line number of blank lines: ‘0’: Numerical result out of range",
+        )),
+    }
     if let Some(num) = opts.get_one::<i64>(options::LINE_INCREMENT) {
         settings.line_increment = *num;
     }
     if let Some(num) = opts.get_one::<i64>(options::STARTING_LINE_NUMBER) {
         settings.starting_line_number = *num;
     }
-    match opts.get_one::<String>(options::JOIN_BLANK_LINES) {
-        None => {}
-        Some(val) => {
-            let conv: Option<u64> = val.parse().ok();
-            match conv {
-                None => {
-                    errs.push(String::from("Illegal value for -l"));
-                }
-                Some(num) => settings.join_blank_lines = num,
-            }
-        }
-    }
     errs
 }
diff --git a/src/uu/nl/src/nl.rs b/src/uu/nl/src/nl.rs
index 39090aea5ac..bc3817acc78 100644
--- a/src/uu/nl/src/nl.rs
+++ b/src/uu/nl/src/nl.rs
@@ -216,7 +216,8 @@ pub fn uu_app() -> Command {
                 .short('l')
                 .long(options::JOIN_BLANK_LINES)
                 .help("group of NUMBER empty lines counted as one")
-                .value_name("NUMBER"),
+                .value_name("NUMBER")
+                .value_parser(clap::value_parser!(u64)),
         )
         .arg(
             Arg::new(options::NUMBER_FORMAT)
diff --git a/tests/by-util/test_nl.rs b/tests/by-util/test_nl.rs
index bdb76da78f7..c32e90ad59f 100644
--- a/tests/by-util/test_nl.rs
+++ b/tests/by-util/test_nl.rs
@@ -1,4 +1,4 @@
-// spell-checker:ignore iinvalid ninvalid vinvalid winvalid
+// spell-checker:ignore iinvalid linvalid ninvalid vinvalid winvalid
 use crate::common::util::TestScenario;
 
 #[test]
@@ -244,3 +244,22 @@ fn test_invalid_line_increment() {
             .stderr_contains("invalid value 'invalid'");
     }
 }
+
+#[test]
+fn test_join_blank_lines_zero() {
+    for arg in ["-l0", "--join-blank-lines=0"] {
+        new_ucmd!().arg(arg).fails().stderr_contains(
+            "Invalid line number of blank lines: ‘0’: Numerical result out of range",
+        );
+    }
+}
+
+#[test]
+fn test_invalid_join_blank_lines() {
+    for arg in ["-linvalid", "--join-blank-lines=invalid"] {
+        new_ucmd!()
+            .arg(arg)
+            .fails()
+            .stderr_contains("invalid value 'invalid'");
+    }
+}