Merge pull request #5156 from cakebaker/nl_implement_try_from_for_numbering_style

tertsdiepraam · web-flow · commit f1978955badb · 2023-08-18T23:10:32.000+02:00
nl: implement TryFrom&lt;&amp;str&gt; for NumberingStyle
diff --git a/src/uu/nl/src/helper.rs b/src/uu/nl/src/helper.rs
@@ -2,25 +2,6 @@
 
 use crate::options;
 
-// parse_style parses a style string into a NumberingStyle.
-fn parse_style(chars: &[char]) -> Result<crate::NumberingStyle, String> {
-    if chars.len() == 1 && chars[0] == 'a' {
-        Ok(crate::NumberingStyle::All)
-    } else if chars.len() == 1 && chars[0] == 't' {
-        Ok(crate::NumberingStyle::NonEmpty)
-    } else if chars.len() == 1 && chars[0] == 'n' {
-        Ok(crate::NumberingStyle::None)
-    } else if chars.len() > 1 && chars[0] == 'p' {
-        let s: String = chars[1..].iter().cloned().collect();
-        match regex::Regex::new(&s) {
-            Ok(re) => Ok(crate::NumberingStyle::Regex(Box::new(re))),
-            Err(_) => Err(String::from("Illegal regular expression")),
-        }
-    } else {
-        Err(String::from("Illegal style encountered"))
-    }
-}
-
 // parse_options loads the options into the settings, returning an array of
 // error messages.
 #[allow(clippy::cognitive_complexity)]
@@ -35,47 +16,32 @@ pub fn parse_options(settings: &mut crate::Settings, opts: &clap::ArgMatches) ->
         .get_one::<String>(options::NUMBER_FORMAT)
         .map(Into::into)
         .unwrap_or_default();
-    match opts.get_one::<String>(options::BODY_NUMBERING) {
+    match opts
+        .get_one::<String>(options::HEADER_NUMBERING)
+        .map(String::as_str)
+        .map(TryInto::try_into)
+    {
         None => {}
-        Some(val) => {
-            let chars: Vec<char> = val.chars().collect();
-            match parse_style(&chars) {
-                Ok(s) => {
-                    settings.body_numbering = s;
-                }
-                Err(message) => {
-                    errs.push(message);
-                }
-            }
-        }
+        Some(Ok(style)) => settings.header_numbering = style,
+        Some(Err(message)) => errs.push(message.to_string()),
     }
-    match opts.get_one::<String>(options::FOOTER_NUMBERING) {
+    match opts
+        .get_one::<String>(options::BODY_NUMBERING)
+        .map(String::as_str)
+        .map(TryInto::try_into)
+    {
         None => {}
-        Some(val) => {
-            let chars: Vec<char> = val.chars().collect();
-            match parse_style(&chars) {
-                Ok(s) => {
-                    settings.footer_numbering = s;
-                }
-                Err(message) => {
-                    errs.push(message);
-                }
-            }
-        }
+        Some(Ok(style)) => settings.body_numbering = style,
+        Some(Err(message)) => errs.push(message.to_string()),
     }
-    match opts.get_one::<String>(options::HEADER_NUMBERING) {
+    match opts
+        .get_one::<String>(options::FOOTER_NUMBERING)
+        .map(String::as_str)
+        .map(TryInto::try_into)
+    {
         None => {}
-        Some(val) => {
-            let chars: Vec<char> = val.chars().collect();
-            match parse_style(&chars) {
-                Ok(s) => {
-                    settings.header_numbering = s;
-                }
-                Err(message) => {
-                    errs.push(message);
-                }
-            }
-        }
+        Some(Ok(style)) => settings.footer_numbering = style,
+        Some(Err(message)) => errs.push(message.to_string()),
     }
     match opts.get_one::<usize>(options::NUMBER_WIDTH) {
         None => {}
diff --git a/src/uu/nl/src/nl.rs b/src/uu/nl/src/nl.rs
@@ -71,6 +71,23 @@ enum NumberingStyle {
     Regex(Box<regex::Regex>),
 }
 
+impl TryFrom<&str> for NumberingStyle {
+    type Error = String;
+
+    fn try_from(s: &str) -> Result<Self, Self::Error> {
+        match s {
+            "a" => Ok(Self::All),
+            "t" => Ok(Self::NonEmpty),
+            "n" => Ok(Self::None),
+            _ if s.starts_with('p') => match regex::Regex::new(&s[1..]) {
+                Ok(re) => Ok(Self::Regex(Box::new(re))),
+                Err(_) => Err(String::from("invalid regular expression")),
+            },
+            _ => Err(format!("invalid numbering style: '{s}'")),
+        }
+    }
+}
+
 // NumberFormat specifies how line numbers are output within their allocated
 // space. They are justified to the left or right, in the latter case with
 // the option of having all unused space to its left turned into leading zeroes.
diff --git a/tests/by-util/test_nl.rs b/tests/by-util/test_nl.rs
@@ -1,4 +1,4 @@
-// spell-checker:ignore iinvalid linvalid ninvalid vinvalid winvalid
+// spell-checker:ignore binvalid finvalid hinvalid iinvalid linvalid ninvalid vinvalid winvalid
 use crate::common::util::TestScenario;
 
 #[test]
@@ -426,3 +426,41 @@ fn test_numbering_matched_lines() {
         }
     }
 }
+
+#[test]
+fn test_invalid_numbering() {
+    let invalid_args = [
+        "-hinvalid",
+        "--header-numbering=invalid",
+        "-binvalid",
+        "--body-numbering=invalid",
+        "-finvalid",
+        "--footer-numbering=invalid",
+    ];
+
+    for invalid_arg in invalid_args {
+        new_ucmd!()
+            .arg(invalid_arg)
+            .fails()
+            .stderr_contains("invalid numbering style: 'invalid'");
+    }
+}
+
+#[test]
+fn test_invalid_regex_numbering() {
+    let invalid_args = [
+        "-hp[",
+        "--header-numbering=p[",
+        "-bp[",
+        "--body-numbering=p[",
+        "-fp[",
+        "--footer-numbering=p[",
+    ];
+
+    for invalid_arg in invalid_args {
+        new_ucmd!()
+            .arg(invalid_arg)
+            .fails()
+            .stderr_contains("invalid regular expression");
+    }
+}
