[librsvg: 24/43] Replace number_optional_number() with a NumberOptionalNumber type

[Federico Mena Quintero <federico src gnome org>]

commit 4758373383335c9dc9a1528bf7c96b0df5a975a0
Author: Federico Mena Quintero <federico gnome org>
Date:   Fri Dec 20 17:18:41 2019 -0600

    Replace number_optional_number() with a NumberOptionalNumber type

 rsvg_internals/src/filters/convolve_matrix.rs |  61 +++++-----
 rsvg_internals/src/filters/gaussian_blur.rs   |  16 ++-
 rsvg_internals/src/filters/light/lighting.rs  |  29 ++---
 rsvg_internals/src/filters/morphology.rs      |  17 +--
 rsvg_internals/src/filters/turbulence.rs      |  17 +--
 rsvg_internals/src/parsers.rs                 | 156 ++++++++++++++------------
 6 files changed, 158 insertions(+), 138 deletions(-)
---
diff --git a/rsvg_internals/src/filters/convolve_matrix.rs b/rsvg_internals/src/filters/convolve_matrix.rs
index 1c24a022..462464b1 100644
--- a/rsvg_internals/src/filters/convolve_matrix.rs
+++ b/rsvg_internals/src/filters/convolve_matrix.rs
@@ -7,7 +7,7 @@ use crate::drawing_ctx::DrawingCtx;
 use crate::error::*;
 use crate::node::{NodeResult, NodeTrait, RsvgNode};
 use crate::number_list::{NumberList, NumberListLength};
-use crate::parsers::{self, Parse, ParseValue, ParseValueToParseError};
+use crate::parsers::{NumberOptionalNumber, Parse, ParseValue, ParseValueToParseError};
 use crate::property_bag::PropertyBag;
 use crate::rect::IRect;
 use crate::surface_utils::{
@@ -62,45 +62,42 @@ impl NodeTrait for FeConvolveMatrix {
         for (attr, value) in pbag.iter() {
             match attr.expanded() {
                 expanded_name!(svg "order") => {
-                    self.order = parsers::integer_optional_integer(value)
-                        .and_then(|(x, y)| {
-                            if x > 0 && y > 0 {
-                                Ok((x as u32, y as u32))
+                    let NumberOptionalNumber(x, y) = attr.parse_to_parse_error_and_validate(
+                        value,
+                        |v: NumberOptionalNumber<i32>| {
+                            if v.0 > 0 && v.1 > 0 {
+                                Ok(v)
                             } else {
-                                Err(ValueErrorKind::value_error("values must be greater than 0"))
+                                Err(ValueErrorKind::value_error("values must be greater than 0"))?
                             }
-                        })
-                        .attribute(attr)?
+                        },
+                    )?;
+                    self.order = (x as u32, y as u32);
                 }
                 expanded_name!(svg "divisor") => {
-                    self.divisor = Some(
-                        f64::parse_str(value)
-                            .and_then(|x| {
-                                if x != 0.0 {
-                                    Ok(x)
-                                } else {
-                                    Err(ValueErrorKind::value_error("divisor cannot be equal to 0"))
-                                }
-                            })
-                            .attribute(attr)?,
-                    )
+                    self.divisor = Some(attr.parse_to_parse_error_and_validate(value, |x| {
+                        if x != 0.0 {
+                            Ok(x)
+                        } else {
+                            Err(ValueErrorKind::value_error("divisor cannot be equal to 0"))
+                        }
+                    })?)
                 }
                 expanded_name!(svg "bias") => self.bias = attr.parse(value)?,
                 expanded_name!(svg "edgeMode") => self.edge_mode = attr.parse(value)?,
                 expanded_name!(svg "kernelUnitLength") => {
-                    self.kernel_unit_length = Some(
-                        parsers::number_optional_number(value)
-                            .and_then(|(x, y)| {
-                                if x > 0.0 && y > 0.0 {
-                                    Ok((x, y))
-                                } else {
-                                    Err(ValueErrorKind::value_error(
-                                        "kernelUnitLength can't be less or equal to zero",
-                                    ))
-                                }
-                            })
-                            .attribute(attr)?,
-                    )
+                    let NumberOptionalNumber(x, y) =
+                        attr.parse_to_parse_error_and_validate(value, |v: NumberOptionalNumber<f64>| {
+                            if v.0 > 0.0 && v.1 > 0.0 {
+                                Ok(v)
+                            } else {
+                                Err(ValueErrorKind::value_error(
+                                    "kernelUnitLength can't be less or equal to zero",
+                                ))
+                            }
+                        })?;
+
+                    self.kernel_unit_length = Some((x, y))
                 }
                 expanded_name!(svg "preserveAlpha") => self.preserve_alpha = attr.parse(value)?,
 
diff --git a/rsvg_internals/src/filters/gaussian_blur.rs b/rsvg_internals/src/filters/gaussian_blur.rs
index 62bf6651..bd6f1464 100644
--- a/rsvg_internals/src/filters/gaussian_blur.rs
+++ b/rsvg_internals/src/filters/gaussian_blur.rs
@@ -7,7 +7,7 @@ use nalgebra::{DMatrix, Dynamic, VecStorage};
 use crate::drawing_ctx::DrawingCtx;
 use crate::error::*;
 use crate::node::{NodeResult, NodeTrait, RsvgNode};
-use crate::parsers;
+use crate::parsers::{NumberOptionalNumber, ParseValueToParseError};
 use crate::property_bag::PropertyBag;
 use crate::rect::IRect;
 use crate::surface_utils::{
@@ -49,14 +49,18 @@ impl NodeTrait for FeGaussianBlur {
         for (attr, value) in pbag.iter() {
             match attr.expanded() {
                 expanded_name!(svg "stdDeviation") => {
-                    self.std_deviation = parsers::number_optional_number(value)
-                        .and_then(|(x, y)| {
-                            if x >= 0.0 && y >= 0.0 {
-                                Ok((x, y))
+                    let NumberOptionalNumber(x, y) = attr.parse_to_parse_error_and_validate(
+                        value,
+                        |v: NumberOptionalNumber<f64>| {
+                            if v.0 >= 0.0 && v.1 >= 0.0 {
+                                Ok(v)
                             } else {
                                 Err(ValueErrorKind::value_error("values can't be negative"))
                             }
-                        }).attribute(attr)?
+                        },
+                    )?;
+
+                    self.std_deviation = (x, y);
                 }
                 _ => (),
             }
diff --git a/rsvg_internals/src/filters/light/lighting.rs b/rsvg_internals/src/filters/light/lighting.rs
index 4356a45a..a17087c7 100644
--- a/rsvg_internals/src/filters/light/lighting.rs
+++ b/rsvg_internals/src/filters/light/lighting.rs
@@ -32,7 +32,7 @@ use crate::filters::{
     PrimitiveWithInput,
 };
 use crate::node::{CascadedValues, NodeResult, NodeTrait, NodeType, RsvgNode};
-use crate::parsers::{self, Parse, ParseValue};
+use crate::parsers::{NumberOptionalNumber, Parse, ParseValue, ParseValueToParseError};
 use crate::property_bag::PropertyBag;
 use crate::surface_utils::{
     shared_surface::{SharedImageSurface, SurfaceType},
@@ -70,19 +70,20 @@ impl Common {
                 expanded_name!(svg "surfaceScale") => self.surface_scale = attr.parse(value)?,
 
                 expanded_name!(svg "kernelUnitLength") => {
-                    self.kernel_unit_length = Some(
-                        parsers::number_optional_number(value)
-                            .and_then(|(x, y)| {
-                                if x > 0.0 && y > 0.0 {
-                                    Ok((x, y))
-                                } else {
-                                    Err(ValueErrorKind::value_error(
-                                        "kernelUnitLength can't be less or equal to zero",
-                                    ))
-                                }
-                            })
-                            .attribute(attr)?,
-                    )
+                    let NumberOptionalNumber(x, y) = attr.parse_to_parse_error_and_validate(
+                        value,
+                        |v: NumberOptionalNumber<f64>| {
+                            if v.0 > 0.0 && v.1 > 0.0 {
+                                Ok(v)
+                            } else {
+                                Err(ValueErrorKind::value_error(
+                                    "kernelUnitLength can't be less or equal to zero",
+                                ))
+                            }
+                        }
+                    )?;
+
+                    self.kernel_unit_length = Some((x, y));
                 }
                 _ => (),
             }
diff --git a/rsvg_internals/src/filters/morphology.rs b/rsvg_internals/src/filters/morphology.rs
index 862d842f..a0f9a63a 100644
--- a/rsvg_internals/src/filters/morphology.rs
+++ b/rsvg_internals/src/filters/morphology.rs
@@ -7,7 +7,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 use crate::drawing_ctx::DrawingCtx;
 use crate::error::*;
 use crate::node::{NodeResult, NodeTrait, RsvgNode};
-use crate::parsers::{self, Parse, ParseValue};
+use crate::parsers::{NumberOptionalNumber, Parse, ParseValue, ParseValueToParseError};
 use crate::property_bag::PropertyBag;
 use crate::rect::IRect;
 use crate::surface_utils::{
@@ -54,15 +54,18 @@ impl NodeTrait for FeMorphology {
             match attr.expanded() {
                 expanded_name!(svg "operator") => self.operator = attr.parse(value)?,
                 expanded_name!(svg "radius") => {
-                    self.radius = parsers::number_optional_number(value)
-                        .and_then(|(x, y)| {
-                            if x >= 0.0 && y >= 0.0 {
-                                Ok((x, y))
+                    let NumberOptionalNumber(x, y) = attr.parse_to_parse_error_and_validate(
+                        value,
+                        |v: NumberOptionalNumber<f64>| {
+                            if v.0 >= 0.0 && v.1 >= 0.0 {
+                                Ok(v)
                             } else {
                                 Err(ValueErrorKind::value_error("radius cannot be negative"))
                             }
-                        })
-                        .attribute(attr)?
+                        }
+                    )?;
+
+                    self.radius = (x, y);
                 }
                 _ => (),
             }
diff --git a/rsvg_internals/src/filters/turbulence.rs b/rsvg_internals/src/filters/turbulence.rs
index 50a5b020..c9a9b403 100644
--- a/rsvg_internals/src/filters/turbulence.rs
+++ b/rsvg_internals/src/filters/turbulence.rs
@@ -5,7 +5,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 use crate::drawing_ctx::DrawingCtx;
 use crate::error::*;
 use crate::node::{CascadedValues, NodeResult, NodeTrait, RsvgNode};
-use crate::parsers::{self, Parse, ParseValue, ParseValueToParseError};
+use crate::parsers::{NumberOptionalNumber, Parse, ParseValue, ParseValueToParseError};
 use crate::property_bag::PropertyBag;
 use crate::surface_utils::{
     shared_surface::{SharedImageSurface, SurfaceType},
@@ -65,15 +65,18 @@ impl NodeTrait for FeTurbulence {
         for (attr, value) in pbag.iter() {
             match attr.expanded() {
                 expanded_name!(svg "baseFrequency") => {
-                    self.base_frequency = parsers::number_optional_number(value)
-                        .and_then(|(x, y)| {
-                            if x >= 0.0 && y >= 0.0 {
-                                Ok((x, y))
+                    let NumberOptionalNumber(x, y) = attr.parse_to_parse_error_and_validate(
+                        value,
+                        |v: NumberOptionalNumber<f64>| {
+                            if v.0 >= 0.0 && v.1 >= 0.0 {
+                                Ok(v)
                             } else {
                                 Err(ValueErrorKind::value_error("values can't be negative"))
                             }
-                        })
-                        .attribute(attr)?
+                        }
+                    )?;
+
+                    self.base_frequency = (x, y);
                 }
                 expanded_name!(svg "numOctaves") => {
                     self.num_octaves = attr.parse_to_parse_error(value)?;
diff --git a/rsvg_internals/src/parsers.rs b/rsvg_internals/src/parsers.rs
index c96c59ed..f3de8892 100644
--- a/rsvg_internals/src/parsers.rs
+++ b/rsvg_internals/src/parsers.rs
@@ -130,26 +130,24 @@ impl ParseToParseError for f64 {
     }
 }
 
-// number-optional-number
-//
-// https://www.w3.org/TR/SVG/types.html#DataTypeNumberOptionalNumber
-
-pub fn number_optional_number(s: &str) -> Result<(f64, f64), ValueErrorKind> {
-    let mut input = ParserInput::new(s);
-    let mut parser = Parser::new(&mut input);
-
-    let x = f64::parse(&mut parser)?;
-
-    if !parser.is_exhausted() {
-        optional_comma(&mut parser);
-
-        let y = f64::parse(&mut parser)?;
-
-        parser.expect_exhausted()?;
+/// CSS number-optional-number
+///
+/// https://www.w3.org/TR/SVG/types.html#DataTypeNumberOptionalNumber
+#[derive(Debug, Copy, Clone, PartialEq)]
+pub struct NumberOptionalNumber<T: ParseToParseError>(pub T, pub T);
 
-        Ok((x, y))
-    } else {
-        Ok((x, x))
+impl<T: ParseToParseError + Copy> ParseToParseError for NumberOptionalNumber<T> {
+    fn parse_to_parse_error<'i>(parser: &mut Parser<'i, '_>) -> Result<Self, CssParseError<'i>> {
+        let x = ParseToParseError::parse_to_parse_error(parser)?;
+
+        if !parser.is_exhausted() {
+            optional_comma(parser);
+            let y = ParseToParseError::parse_to_parse_error(parser)?;
+            parser.expect_exhausted()?;
+            Ok(NumberOptionalNumber(x, y))
+        } else {
+            Ok(NumberOptionalNumber(x, x))
+        }
     }
 }
 
@@ -162,28 +160,6 @@ impl ParseToParseError for i32 {
     }
 }
 
-// integer-optional-integer
-//
-// Like number-optional-number but with integers.
-pub fn integer_optional_integer(s: &str) -> Result<(i32, i32), ValueErrorKind> {
-    let mut input = ParserInput::new(s);
-    let mut parser = Parser::new(&mut input);
-
-    let x = parser.expect_integer()?;
-
-    if !parser.is_exhausted() {
-        optional_comma(&mut parser);
-
-        let y = parser.expect_integer()?;
-
-        parser.expect_exhausted()?;
-
-        Ok((x, y))
-    } else {
-        Ok((x, x))
-    }
-}
-
 /// Parses a list of identifiers from a `cssparser::Parser`
 ///
 /// # Example
@@ -217,24 +193,42 @@ mod tests {
 
     #[test]
     fn parses_number_optional_number() {
-        assert_eq!(number_optional_number("1, 2"), Ok((1.0, 2.0)));
-        assert_eq!(number_optional_number("1 2"), Ok((1.0, 2.0)));
-        assert_eq!(number_optional_number("1"), Ok((1.0, 1.0)));
-
-        assert_eq!(number_optional_number("-1, -2"), Ok((-1.0, -2.0)));
-        assert_eq!(number_optional_number("-1 -2"), Ok((-1.0, -2.0)));
-        assert_eq!(number_optional_number("-1"), Ok((-1.0, -1.0)));
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("1, 2"),
+            Ok(NumberOptionalNumber(1.0, 2.0))
+        );
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("1 2"),
+            Ok(NumberOptionalNumber(1.0, 2.0))
+        );
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("1"),
+            Ok(NumberOptionalNumber(1.0, 1.0))
+        );
+
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("-1, -2"),
+            Ok(NumberOptionalNumber(-1.0, -2.0))
+        );
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("-1 -2"),
+            Ok(NumberOptionalNumber(-1.0, -2.0))
+        );
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("-1"),
+            Ok(NumberOptionalNumber(-1.0, -1.0))
+        );
     }
 
     #[test]
     fn invalid_number_optional_number() {
-        assert!(number_optional_number("").is_err());
-        assert!(number_optional_number("1x").is_err());
-        assert!(number_optional_number("x1").is_err());
-        assert!(number_optional_number("1 x").is_err());
-        assert!(number_optional_number("1 , x").is_err());
-        assert!(number_optional_number("1 , 2x").is_err());
-        assert!(number_optional_number("1 2 x").is_err());
+        assert!(NumberOptionalNumber::<f64>::parse_str_to_parse_error("").is_err());
+        assert!(NumberOptionalNumber::<f64>::parse_str_to_parse_error("1x").is_err());
+        assert!(NumberOptionalNumber::<f64>::parse_str_to_parse_error("x1").is_err());
+        assert!(NumberOptionalNumber::<f64>::parse_str_to_parse_error("1 x").is_err());
+        assert!(NumberOptionalNumber::<f64>::parse_str_to_parse_error("1 , x").is_err());
+        assert!(NumberOptionalNumber::<f64>::parse_str_to_parse_error("1 , 2x").is_err());
+        assert!(NumberOptionalNumber::<f64>::parse_str_to_parse_error("1 2 x").is_err());
     }
 
     #[test]
@@ -252,26 +246,44 @@ mod tests {
 
     #[test]
     fn parses_integer_optional_integer() {
-        assert_eq!(integer_optional_integer("1, 2"), Ok((1, 2)));
-        assert_eq!(integer_optional_integer("1 2"), Ok((1, 2)));
-        assert_eq!(integer_optional_integer("1"), Ok((1, 1)));
-
-        assert_eq!(integer_optional_integer("-1, -2"), Ok((-1, -2)));
-        assert_eq!(integer_optional_integer("-1 -2"), Ok((-1, -2)));
-        assert_eq!(integer_optional_integer("-1"), Ok((-1, -1)));
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("1, 2"),
+            Ok(NumberOptionalNumber(1, 2))
+        );
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("1 2"),
+            Ok(NumberOptionalNumber(1, 2))
+        );
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("1"),
+            Ok(NumberOptionalNumber(1, 1))
+        );
+
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("-1, -2"),
+            Ok(NumberOptionalNumber(-1, -2))
+        );
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("-1 -2"),
+            Ok(NumberOptionalNumber(-1, -2))
+        );
+        assert_eq!(
+            NumberOptionalNumber::parse_str_to_parse_error("-1"),
+            Ok(NumberOptionalNumber(-1, -1))
+        );
     }
 
     #[test]
     fn invalid_integer_optional_integer() {
-        assert!(integer_optional_integer("").is_err());
-        assert!(integer_optional_integer("1x").is_err());
-        assert!(integer_optional_integer("x1").is_err());
-        assert!(integer_optional_integer("1 x").is_err());
-        assert!(integer_optional_integer("1 , x").is_err());
-        assert!(integer_optional_integer("1 , 2x").is_err());
-        assert!(integer_optional_integer("1 2 x").is_err());
-        assert!(integer_optional_integer("1.5").is_err());
-        assert!(integer_optional_integer("1 2.5").is_err());
-        assert!(integer_optional_integer("1, 2.5").is_err());
+        assert!(NumberOptionalNumber::<i32>::parse_str_to_parse_error("").is_err());
+        assert!(NumberOptionalNumber::<i32>::parse_str_to_parse_error("1x").is_err());
+        assert!(NumberOptionalNumber::<i32>::parse_str_to_parse_error("x1").is_err());
+        assert!(NumberOptionalNumber::<i32>::parse_str_to_parse_error("1 x").is_err());
+        assert!(NumberOptionalNumber::<i32>::parse_str_to_parse_error("1 , x").is_err());
+        assert!(NumberOptionalNumber::<i32>::parse_str_to_parse_error("1 , 2x").is_err());
+        assert!(NumberOptionalNumber::<i32>::parse_str_to_parse_error("1 2 x").is_err());
+        assert!(NumberOptionalNumber::<i32>::parse_str_to_parse_error("1.5").is_err());
+        assert!(NumberOptionalNumber::<i32>::parse_str_to_parse_error("1 2.5").is_err());
+        assert!(NumberOptionalNumber::<i32>::parse_str_to_parse_error("1, 2.5").is_err());
     }
 }