[librsvg] shapes: use a Points struct and impl Parse for it

[Federico Mena Quintero <federico src gnome org>]

commit 03ffce90be67193631bb1dc1c590029dd0f6fa4b
Author: Paolo Borelli <pborelli gnome org>
Date:   Sat Dec 29 18:02:27 2018 +0100

    shapes: use a Points struct and impl Parse for it
    
    It is more idiomatic and consistent with the rest of the code

 rsvg_internals/src/parsers.rs | 53 -----------------------
 rsvg_internals/src/shapes.rs  | 99 +++++++++++++++++++++++++++++++++++++------
 2 files changed, 86 insertions(+), 66 deletions(-)
---
diff --git a/rsvg_internals/src/parsers.rs b/rsvg_internals/src/parsers.rs
index e87e373b..9c4dcf53 100644
--- a/rsvg_internals/src/parsers.rs
+++ b/rsvg_internals/src/parsers.rs
@@ -194,37 +194,6 @@ pub fn integer_optional_integer(s: &str) -> Result<(i32, i32), ValueErrorKind> {
     }
 }
 
-// Parse a list-of-points as for polyline and polygon elements
-// https://www.w3.org/TR/SVG/shapes.html#PointsBNF
-
-pub fn list_of_points(string: &str) -> Result<Vec<(f64, f64)>, ValueErrorKind> {
-    let mut input = ParserInput::new(string);
-    let mut parser = Parser::new(&mut input);
-
-    let mut v = Vec::new();
-
-    loop {
-        let x = f64::from(parser.expect_finite_number()?);
-
-        optional_comma(&mut parser);
-
-        let y = f64::from(parser.expect_finite_number()?);
-
-        v.push((x, y));
-
-        if parser.is_exhausted() {
-            break;
-        }
-
-        match parser.next_including_whitespace() {
-            Ok(&Token::WhiteSpace(_)) => (),
-            _ => optional_comma(&mut parser),
-        }
-    }
-
-    Ok(v)
-}
-
 // Lists of number values
 
 #[derive(Eq, PartialEq)]
@@ -328,28 +297,6 @@ mod tests {
         assert!(number_optional_number("1 2 x").is_err());
     }
 
-    #[test]
-    fn parses_list_of_points() {
-        assert_eq!(list_of_points(" 1 2 "), Ok(vec![(1.0, 2.0)]));
-        assert_eq!(list_of_points("1 2 3 4"), Ok(vec![(1.0, 2.0), (3.0, 4.0)]));
-        assert_eq!(list_of_points("1,2,3,4"), Ok(vec![(1.0, 2.0), (3.0, 4.0)]));
-        assert_eq!(list_of_points("1,2 3,4"), Ok(vec![(1.0, 2.0), (3.0, 4.0)]));
-        assert_eq!(
-            list_of_points("1,2 -3,4"),
-            Ok(vec![(1.0, 2.0), (-3.0, 4.0)])
-        );
-        assert_eq!(
-            list_of_points("1,2,-3,4"),
-            Ok(vec![(1.0, 2.0), (-3.0, 4.0)])
-        );
-    }
-
-    #[test]
-    fn errors_on_invalid_list_of_points() {
-        assert!(list_of_points("-1-2-3-4").is_err());
-        assert!(list_of_points("1 2-3,-4").is_err());
-    }
-
     #[test]
     fn parses_number_list() {
         assert_eq!(
diff --git a/rsvg_internals/src/shapes.rs b/rsvg_internals/src/shapes.rs
index 845c1b12..41e1b1ec 100644
--- a/rsvg_internals/src/shapes.rs
+++ b/rsvg_internals/src/shapes.rs
@@ -1,15 +1,17 @@
 use cairo;
 use std::cell::Cell;
 use std::cell::RefCell;
+use std::ops::Deref;
 
 use attributes::Attribute;
+use cssparser::{Parser, ParserInput, Token};
 use drawing_ctx::DrawingCtx;
 use error::*;
 use handle::RsvgHandle;
 use length::*;
 use marker;
 use node::*;
-use parsers::{self, parse, parse_and_validate};
+use parsers::{optional_comma, parse, parse_and_validate, CssParserExt, Parse};
 use path_builder::*;
 use path_parser;
 use property_bag::PropertyBag;
@@ -174,8 +176,49 @@ enum PolyKind {
     Closed,
 }
 
+#[derive(Debug, PartialEq)]
+struct Points(Vec<(f64, f64)>);
+
+impl Deref for Points {
+    type Target = [(f64, f64)];
+
+    fn deref(&self) -> &[(f64, f64)] {
+        &self.0
+    }
+}
+
+// Parse a list-of-points as for polyline and polygon elements
+// https://www.w3.org/TR/SVG/shapes.html#PointsBNF
+impl Parse for Points {
+    type Data = ();
+    type Err = ValueErrorKind;
+
+    fn parse(parser: &mut Parser<'_, '_>, _: ()) -> Result<Points, ValueErrorKind> {
+        let mut v = Vec::new();
+
+        loop {
+            let x = f64::from(parser.expect_finite_number()?);
+            optional_comma(parser);
+            let y = f64::from(parser.expect_finite_number()?);
+
+            v.push((x, y));
+
+            if parser.is_exhausted() {
+                break;
+            }
+
+            match parser.next_including_whitespace() {
+                Ok(&Token::WhiteSpace(_)) => (),
+                _ => optional_comma(parser),
+            }
+        }
+
+        Ok(Points(v))
+    }
+}
+
 pub struct NodePoly {
-    points: RefCell<Option<Vec<(f64, f64)>>>,
+    points: RefCell<Option<Points>>,
     kind: PolyKind,
 }
 
@@ -200,18 +243,12 @@ impl NodeTrait for NodePoly {
         for (attr, value) in pbag.iter() {
             // support for svg < 1.0 which used verts
             if attr == Attribute::Points || attr == Attribute::Verts {
-                let result = parsers::list_of_points(value.trim());
-
-                match result {
-                    Ok(v) => {
-                        *self.points.borrow_mut() = Some(v);
-                        break;
-                    }
+                let mut input = ParserInput::new(value.trim());
+                let mut parser = Parser::new(&mut input);
 
-                    Err(e) => {
-                        return Err(NodeError::attribute_error(attr, e));
-                    }
-                }
+                *self.points.borrow_mut() = Points::parse(&mut parser, ())
+                    .map_err(|err| NodeError::attribute_error(attr, err))
+                    .ok();
             }
         }
 
@@ -640,3 +677,39 @@ impl NodeTrait for NodeEllipse {
         render_ellipse(cx, cy, rx, ry, draw_ctx, node, values, clipping)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn parses_points() {
+        assert_eq!(Points::parse_str(" 1 2 ", ()), Ok(Points(vec![(1.0, 2.0)])));
+        assert_eq!(
+            Points::parse_str("1 2 3 4", ()),
+            Ok(Points(vec![(1.0, 2.0), (3.0, 4.0)]))
+        );
+        assert_eq!(
+            Points::parse_str("1,2,3,4", ()),
+            Ok(Points(vec![(1.0, 2.0), (3.0, 4.0)]))
+        );
+        assert_eq!(
+            Points::parse_str("1,2 3,4", ()),
+            Ok(Points(vec![(1.0, 2.0), (3.0, 4.0)]))
+        );
+        assert_eq!(
+            Points::parse_str("1,2 -3,4", ()),
+            Ok(Points(vec![(1.0, 2.0), (-3.0, 4.0)]))
+        );
+        assert_eq!(
+            Points::parse_str("1,2,-3,4", ()),
+            Ok(Points(vec![(1.0, 2.0), (-3.0, 4.0)]))
+        );
+    }
+
+    #[test]
+    fn errors_on_invalid_points() {
+        assert!(Points::parse_str("-1-2-3-4", ()).is_err());
+        assert!(Points::parse_str("1 2-3,-4", ()).is_err());
+    }
+}