Change comma parsing to support an arbitrary number of elements, rather than jjust pairs

src/lib.rs

@@ -9,7 +9,7 @@ pub enum CronSpecifier {
     Any,
     Specifically(u8),
     Range(u8, u8),
-    Pair(Box<CronSpecifier>, Box<CronSpecifier>),
+    Several(Vec<CronSpecifier>),
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -34,7 +34,7 @@ mod tests {
     use super::*;
     use std::mem;
     use test_case::test_case;
-    use CronSpecifier::{Any, Pair, Range, Specifically};
+    use CronSpecifier::{Any, Range, Several, Specifically};
 
     // simple parse checks
     #[test_case("* * * * *", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Any})]
@@ -46,9 +46,10 @@ mod tests {
     #[test_case("* * * * mon-wed", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Range(1, 3)})]
     #[test_case("* * * * mon-5", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Range(1, 5)})]
     #[test_case("* * * jan-jun *", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Range(1, 6), day_of_week: Any})]
-    #[test_case("* * * 1,5 *", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Pair(Box::new(Specifically(1)), Box::new(Specifically(5))), day_of_week: Any})]
-    #[test_case("* * * * 1,5", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Pair(Box::new(Specifically(1)), Box::new(Specifically(5)))})]
-    #[test_case("* * * * mon-wed,1", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Pair(Box::new(Range(1, 3)), Box::new(Specifically(1)))})]
+    #[test_case("* * * 1,5 *", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Several(vec![Specifically(1), Specifically(5)]), day_of_week: Any})]
+    #[test_case("* * * * 1,5", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Several(vec![Specifically(1), Specifically(5)])})]
+    #[test_case("* * * * mon-wed,1", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Several(vec![Range(1, 3), Specifically(1)])})]
+    #[test_case("* * * * mon-wed,1,3-5", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Several(vec![Range(1, 3), Specifically(1), Range(3, 5)])})]
     // days of week
     #[test_case("* * * 1 mon", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Specifically(1), day_of_week: Specifically(1)})]
     #[test_case("* * * 2 MON", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Specifically(2), day_of_week: Specifically(1)})]

src/parse.rs

@@ -1,5 +1,10 @@
 use itertools::Itertools;
-use nom::{branch::alt, character::complete::char, IResult, Parser};
+use nom::{
+    branch::alt,
+    character::complete::char,
+    combinator::{cond, map},
+    IResult, Parser,
+};
 use thiserror::Error;
 
 use crate::{CronEntry, CronSpecifier};
@@ -163,7 +168,10 @@ where
     P: Parser<&'a str, CronSpecifier, E> + Copy,
     E: nom::error::ParseError<&'a str>,
 {
-    move |chunk: &str| alt((compound::make_pair_parser(base_parser), base_parser))(chunk)
+    alt((
+        compound::make_several_specifier_parser(base_parser),
+        base_parser,
+    ))
 }
 
 /// Parse five elements separated by some kind of constant specifier.

src/parse/compound.rs

@@ -3,54 +3,39 @@
 use crate::parse::{simple, word};
 use crate::CronSpecifier;
 use nom::combinator::fail;
+use nom::multi::separated_list1;
 use nom::InputLength;
 use nom::{
     branch::alt, character::complete::char, sequence::separated_pair, AsChar, IResult, InputIter,
     Parser, Slice,
 };
-use std::ops::{RangeFrom, RangeTo};
+use std::ops::RangeFrom;
 
-/// Make a parser that will parse a pair specifier in a cron entry. Each part of the pair must be matched by
+/// Make a parser that will parse a list specifier in a cron entry. Each part of the list must be matched by
 /// `component_parser`
-pub(super) fn make_pair_parser<P, I, E>(
-    mut component_parser: P,
+pub(super) fn make_several_specifier_parser<P, I, E>(
+    component_parser: P,
 ) -> impl FnMut(I) -> IResult<I, CronSpecifier, E>
 where
-    P: Parser<I, CronSpecifier, E>,
-    I: InputIter + InputLength + Slice<RangeTo<usize>> + Slice<RangeFrom<usize>>,
+    P: Parser<I, CronSpecifier, E> + Copy,
+    I: InputIter + InputLength + Slice<RangeFrom<usize>> + Clone,
     <I as InputIter>::Item: AsChar,
     E: nom::error::ParseError<I>,
 {
     move |chunk: I| {
         let space_loc_candidate = chunk.position(|c| c.as_char() == ' ');
         let comma_loc_candidate = chunk.position(|c| c.as_char() == ',');
-        if comma_loc_candidate.is_none() {
-            return fail(chunk);
-        }
-
-        let space_before_comma = if let Some(space_loc) = space_loc_candidate {
-            space_loc < comma_loc_candidate.unwrap()
-        } else {
-            false
+        let should_parse = match (space_loc_candidate, comma_loc_candidate) {
+            (Some(space_loc), Some(comma_loc)) => space_loc > comma_loc,
+            (None, Some(_)) => true,
+            (_, None) => false,
         };
-        if space_before_comma {
-            return fail(chunk);
-        }
-        let comma_loc = comma_loc_candidate.unwrap();
-        // If the comma is the last position, we can't split this into two segments
-        if comma_loc == chunk.input_len() - 1 {
-            return fail(chunk);
-        }
 
-        // To prevent infinite recursion, we must split the comma ourselves, instead of using separated_pair.
-        // Otherwise, we will always detect that a comma is present and continue the recursion.
-        let (remaining, child_spec1) = component_parser.parse(chunk.slice(..comma_loc))?;
-        if !remaining.input_len() == 0 {
+        if !should_parse {
             return fail(chunk);
         }
-
-        let (remaining, child_spec2) = component_parser.parse(chunk.slice(comma_loc + 1..))?;
-        let specifier = CronSpecifier::Pair(Box::new(child_spec1), Box::new(child_spec2));
+        let (remaining, elements) = separated_list1(char(','), component_parser)(chunk)?;
+        let specifier = CronSpecifier::Several(elements);
 
         Ok((remaining, specifier))
     }