[FIX] Reject malformed credits time options instead of aborting

src/rust/src/args.rs

@@ -779,33 +779,33 @@ pub struct Args {
     pub startcreditstext: Option<String>,
     /// Don't display the start credits before this
     /// time (S, or MM:SS). Default: 0
-    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS)]
-    pub startcreditsnotbefore: Option<String>,
+    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS, value_parser=Timestamp::parse_optional_hhmmss_from_str)]
+    pub startcreditsnotbefore: Option<Timestamp>,
     /// Don't display the start credits after this
     /// time (S, or MM:SS). Default: 5:00
-    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS)]
-    pub startcreditsnotafter: Option<String>,
+    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS, value_parser=Timestamp::parse_optional_hhmmss_from_str)]
+    pub startcreditsnotafter: Option<Timestamp>,
     /// Start credits need to be displayed for at least
     /// this time (S, or MM:SS). Default: 2
-    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS)]
-    pub startcreditsforatleast: Option<String>,
+    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS, value_parser=Timestamp::parse_optional_hhmmss_from_str)]
+    pub startcreditsforatleast: Option<Timestamp>,
     /// Start credits should be displayed for at most
     /// this time (S, or MM:SS). Default: 5
-    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS)]
-    pub startcreditsforatmost: Option<String>,
+    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS, value_parser=Timestamp::parse_optional_hhmmss_from_str)]
+    pub startcreditsforatmost: Option<Timestamp>,
     /// Write this text as end credits. If there are
     /// several lines, separate them with the
     /// characters \n, for example Line1\nLine 2.
     #[arg(long, verbatim_doc_comment, value_name="txt", help_heading=ADDING_CREDITS)]
     pub endcreditstext: Option<String>,
     /// End credits need to be displayed for at least
     /// this time (S, or MM:SS). Default: 2
-    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS)]
-    pub endcreditsforatleast: Option<String>,
+    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS, value_parser=Timestamp::parse_optional_hhmmss_from_str)]
+    pub endcreditsforatleast: Option<Timestamp>,
     /// End credits should be displayed for at most
     /// this time (S, or MM:SS). Default: 5
-    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS)]
-    pub endcreditsforatmost: Option<String>,
+    #[arg(long, verbatim_doc_comment, value_name="time", help_heading=ADDING_CREDITS, value_parser=Timestamp::parse_optional_hhmmss_from_str)]
+    pub endcreditsforatmost: Option<Timestamp>,
     /// Show lots of debugging output.
     #[arg(long, verbatim_doc_comment, help_heading=OUTPUT_AFFECTING_DEBUG_DATA)]
     pub debug: bool,

src/rust/src/parser.rs

@@ -5,7 +5,6 @@ use lib_ccxr::time::units::{Timestamp, TimestampFormat};
 use lib_ccxr::util::encoders_helper::{add_builtin_capitalization, add_builtin_profane};
 use lib_ccxr::util::encoding::Encoding;
 use lib_ccxr::util::log::{DebugMessageFlag, DebugMessageMask, ExitCause, OutputTarget};
-use lib_ccxr::util::time::stringztoms;
 use num_integer::Integer;
 use std::cell::Cell;
 use std::convert::TryInto;
@@ -792,37 +791,31 @@ impl OptionsExt for Options {
             self.enc_cfg.start_credits_text.clone_from(startcreditstext);
         }
 
-        if let Some(ref startcreditsnotbefore) = args.startcreditsnotbefore {
-            self.enc_cfg.startcreditsnotbefore =
-                stringztoms(startcreditsnotbefore.clone().as_str()).unwrap();
+        if let Some(startcreditsnotbefore) = args.startcreditsnotbefore {
+            self.enc_cfg.startcreditsnotbefore = startcreditsnotbefore;
         }
 
-        if let Some(ref startcreditsnotafter) = args.startcreditsnotafter {
-            self.enc_cfg.startcreditsnotafter =
-                stringztoms(startcreditsnotafter.clone().as_str()).unwrap();
+        if let Some(startcreditsnotafter) = args.startcreditsnotafter {
+            self.enc_cfg.startcreditsnotafter = startcreditsnotafter;
         }
 
-        if let Some(ref startcreditsforatleast) = args.startcreditsforatleast {
-            self.enc_cfg.startcreditsforatleast =
-                stringztoms(startcreditsforatleast.clone().as_str()).unwrap();
+        if let Some(startcreditsforatleast) = args.startcreditsforatleast {
+            self.enc_cfg.startcreditsforatleast = startcreditsforatleast;
         }
-        if let Some(ref startcreditsforatmost) = args.startcreditsforatmost {
-            self.enc_cfg.startcreditsforatmost =
-                stringztoms(startcreditsforatmost.clone().as_str()).unwrap();
+        if let Some(startcreditsforatmost) = args.startcreditsforatmost {
+            self.enc_cfg.startcreditsforatmost = startcreditsforatmost;
         }
 
         if let Some(ref endcreditstext) = args.endcreditstext {
             self.enc_cfg.end_credits_text.clone_from(endcreditstext);
         }
 
-        if let Some(ref endcreditsforatleast) = args.endcreditsforatleast {
-            self.enc_cfg.endcreditsforatleast =
-                stringztoms(endcreditsforatleast.clone().as_str()).unwrap();
+        if let Some(endcreditsforatleast) = args.endcreditsforatleast {
+            self.enc_cfg.endcreditsforatleast = endcreditsforatleast;
         }
 
-        if let Some(ref endcreditsforatmost) = args.endcreditsforatmost {
-            self.enc_cfg.endcreditsforatmost =
-                stringztoms(endcreditsforatmost.clone().as_str()).unwrap();
+        if let Some(endcreditsforatmost) = args.endcreditsforatmost {
+            self.enc_cfg.endcreditsforatmost = endcreditsforatmost;
         }
 
         /* More stuff */
@@ -2184,6 +2177,52 @@ pub mod tests {
         assert_eq!(options.subs_delay, Timestamp::from_millis(-200));
     }
 
+    const CREDITS_TIME_OPTIONS: [&str; 6] = [
+        "--startcreditsnotbefore",
+        "--startcreditsnotafter",
+        "--startcreditsforatleast",
+        "--startcreditsforatmost",
+        "--endcreditsforatleast",
+        "--endcreditsforatmost",
+    ];
+
+    #[test]
+    fn test_credits_time_rejects_invalid_values() {
+        for option in CREDITS_TIME_OPTIONS {
+            for value in ["", "invalid", "60", "00:60", "1::2", "1:2:3:4"] {
+                let error = Args::try_parse_from(["ccextractor", "input.scc", option, value])
+                    .expect_err("invalid credits time should be rejected by clap");
+                assert_eq!(error.kind(), clap::error::ErrorKind::ValueValidation);
+                assert!(error.to_string().contains(option));
+            }
+        }
+    }
+
+    #[test]
+    fn test_credits_time_formats() {
+        let (options, _) = parse_args(&[
+            "--startcreditsnotbefore",
+            "1",
+            "--startcreditsnotafter",
+            "03:00",
+            "--startcreditsforatleast",
+            "2",
+            "--startcreditsforatmost",
+            "01:30:45",
+            "--endcreditsforatleast",
+            "3",
+            "--endcreditsforatmost",
+            "4:05",
+        ]);
+        let cfg = &options.enc_cfg;
+        assert_eq!(cfg.startcreditsnotbefore, Timestamp::from_millis(1_000));
+        assert_eq!(cfg.startcreditsnotafter, Timestamp::from_millis(180_000));
+        assert_eq!(cfg.startcreditsforatleast, Timestamp::from_millis(2_000));
+        assert_eq!(cfg.startcreditsforatmost, Timestamp::from_millis(5_445_000));
+        assert_eq!(cfg.endcreditsforatleast, Timestamp::from_millis(3_000));
+        assert_eq!(cfg.endcreditsforatmost, Timestamp::from_millis(245_000));
+    }
+
     #[test]
     fn test_startat_sets_extraction_start() {
         let (options, _) = parse_args(&["--startat", "4"]);