fix(yt/select/selection_file/duration): Improve the duration parser

The previous parser was very brittle, it failed for (valid) outputs like
`1d 10h 30m` (as it only expected two number unit pairs).

On top of that, extending it was failure prone (as proven by the
roundtrip failure in combination with the `d` unit).

1 files changed, 114 insertions, 53 deletions

diff --git a/crates/yt/src/select/selection_file/duration.rs b/crates/yt/src/select/selection_file/duration.rs
index 77c4fc5..668a0b8 100644
--- a/crates/yt/src/select/selection_file/duration.rs
+++ b/crates/yt/src/select/selection_file/duration.rs
@@ -12,7 +12,7 @@
 use std::str::FromStr;
 use std::time::Duration;
 
-use anyhow::{Context, Result};
+use anyhow::{Result, bail};
 
 const SECOND: u64 = 1;
 const MINUTE: u64 = 60 * SECOND;
@@ -73,52 +73,109 @@ impl FromStr for MaybeDuration {
     type Err = anyhow::Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        fn parse_num(str: &str, suffix: char) -> Result<u64> {
-            str.strip_suffix(suffix)
-                .with_context(|| format!("Failed to strip suffix '{suffix}' of number: '{str}'"))?
-                .parse::<u64>()
-                .with_context(|| format!("Failed to parse '{suffix}'"))
+        #[derive(Debug, Clone, Copy)]
+        enum Token {
+            Number(u64),
+            UnitConstant((char, u64)),
+        }
+
+        struct Tokenizer<'a> {
+            input: &'a str,
+        }
+
+        impl Tokenizer<'_> {
+            fn next(&mut self) -> Result<Option<Token>> {
+                loop {
+                    if let Some(next) = self.peek() {
+                        match next {
+                            '0'..='9' => {
+                                let mut number = self.expect_num();
+                                while matches!(self.peek(), Some('0'..='9')) {
+                                    number *= 10;
+                                    number += self.expect_num();
+                                }
+                                break Ok(Some(Token::Number(number)));
+                            }
+                            's' => {
+                                self.chomp();
+                                break Ok(Some(Token::UnitConstant(('s', SECOND))));
+                            }
+                            'm' => {
+                                self.chomp();
+                                break Ok(Some(Token::UnitConstant(('m', MINUTE))));
+                            }
+                            'h' => {
+                                self.chomp();
+                                break Ok(Some(Token::UnitConstant(('h', HOUR))));
+                            }
+                            'd' => {
+                                self.chomp();
+                                break Ok(Some(Token::UnitConstant(('d', DAY))));
+                            }
+                            ' ' => {
+                                // Simply ignore white space
+                                self.chomp();
+                            }
+                            other => bail!("Unknown unit: {other:#?}"),
+                        }
+                    } else {
+                        break Ok(None);
+                    }
+                }
+            }
+
+            fn chomp(&mut self) {
+                self.input = &self.input[1..];
+            }
+
+            fn peek(&self) -> Option<char> {
+                self.input.chars().next()
+            }
+
+            fn expect_num(&mut self) -> u64 {
+                let next = self.peek().expect("Should be some at this point");
+                self.chomp();
+                assert!(next.is_ascii_digit());
+                (next as u64) - ('0' as u64)
+            }
         }
 
         if s == "[No duration]" {
             return Ok(Self { time: None });
         }
 
-        let buf: Vec<_> = s.split(' ').collect();
+        let mut tokenizer = Tokenizer { input: s };
 
-        let days;
-        let hours;
-        let minutes;
-        let seconds;
+        let mut value = 0;
+        let mut current_val = None;
+        while let Some(token) = tokenizer.next()? {
+            match token {
+                Token::Number(number) => {
+                    if let Some(current_val) = current_val {
+                        bail!("Failed to find unit for number: {current_val}");
+                    }
 
-        assert_eq!(buf.len(), 2, "Other lengths should not happen");
+                    {
+                        current_val = Some(number);
+                    }
+                }
+                Token::UnitConstant((name, unit)) => {
+                    if let Some(cval) = current_val {
+                        value += cval * unit;
+                        current_val = None;
+                    } else {
+                        bail!("Found unit without number: {name:#?}");
+                    }
+                }
+            }
+        }
 
-        if buf[0].ends_with('d') {
-            days = parse_num(buf[0], 'd')?;
-            hours = parse_num(buf[1], 'h')?;
-            minutes = parse_num(buf[2], 'm')?;
-            seconds = 0;
-        } else if buf[0].ends_with('h') {
-            days = 0;
-            hours = parse_num(buf[0], 'h')?;
-            minutes = parse_num(buf[1], 'm')?;
-            seconds = 0;
-        } else if buf[0].ends_with('m') {
-            days = 0;
-            hours = 0;
-            minutes = parse_num(buf[0], 'm')?;
-            seconds = parse_num(buf[1], 's')?;
-        } else {
-            unreachable!(
-                "The first part always ends with 'h' or 'm', but was: {:#?}",
-                buf
-            )
+        if let Some(current_val) = current_val {
+            bail!("Duration endet without unit, number was: {current_val}");
         }
 
         Ok(Self {
-            time: Some(Duration::from_secs(
-                days * DAY + hours * HOUR + minutes * MINUTE + seconds * SECOND,
-            )),
+            time: Some(Duration::from_secs(value)),
         })
     }
 }
@@ -156,30 +213,34 @@ mod test {
 
     use super::MaybeDuration;
 
-    #[test]
-    fn test_display_duration_1h() {
-        let dur = MaybeDuration::from_secs(HOUR);
-        assert_eq!("1h 0m".to_owned(), dur.to_string());
+    fn mk_roundtrip(input: MaybeDuration, expected: &str) {
+        let output = MaybeDuration::from_str(expected).unwrap();
+
+        assert_eq!(input.to_string(), output.to_string());
+        assert_eq!(input.to_string(), expected);
+        assert_eq!(
+            MaybeDuration::from_str(input.to_string().as_str()).unwrap(),
+            output
+        );
     }
+
     #[test]
-    fn test_display_duration_30min() {
-        let dur = MaybeDuration::from_secs(MINUTE * 30);
-        assert_eq!("30m 0s".to_owned(), dur.to_string());
+    fn test_roundtrip_duration_1h() {
+        mk_roundtrip(MaybeDuration::from_secs(HOUR), "1h 0m");
     }
     #[test]
-    fn test_display_duration_1d() {
-        let dur = MaybeDuration::from_secs(DAY + MINUTE * 30 + HOUR * 2);
-        assert_eq!("1d 2h 30m".to_owned(), dur.to_string());
+    fn test_roundtrip_duration_30min() {
+        mk_roundtrip(MaybeDuration::from_secs(MINUTE * 30), "30m 0s");
     }
-
     #[test]
-    fn test_display_duration_roundtrip() {
-        let dur = MaybeDuration::zero();
-        let dur_str = dur.to_string();
-
-        assert_eq!(
-            MaybeDuration::zero(),
-            MaybeDuration::from_str(&dur_str).unwrap()
+    fn test_roundtrip_duration_1d() {
+        mk_roundtrip(
+            MaybeDuration::from_secs(DAY + MINUTE * 30 + HOUR * 2),
+            "1d 2h 30m",
         );
     }
+    #[test]
+    fn test_roundtrip_duration_none() {
+        mk_roundtrip(MaybeDuration::from_maybe_secs_f64(None), "[No duration]");
+    }
 }