[librsvg] Shuffle around node::Error and AttributeError

[Federico Mena Quintero <federico src gnome org>]

commit 4cea62cacbf9b433ac53aa842184fc453342a73f
Author: Federico Mena Quintero <federico gnome org>
Date:   Fri Mar 3 14:13:19 2017 -0600

    Shuffle around node::Error and AttributeError
    
    Now Error has the attr_name and an AttributeError, which actually has
    Parse and Value matches.

 rust/src/node.rs    |   59 +++++++++++++++++++++++-------------
 rust/src/parsers.rs |   83 +++++++++++++++++++++++++++++++++-----------------
 rust/src/shapes.rs  |   19 ++++++-----
 3 files changed, 103 insertions(+), 58 deletions(-)
---
diff --git a/rust/src/node.rs b/rust/src/node.rs
index c577574..b8ed91f 100644
--- a/rust/src/node.rs
+++ b/rust/src/node.rs
@@ -14,6 +14,7 @@ use drawing_ctx;
 
 use handle::RsvgHandle;
 
+use parsers::ParseError;
 use property_bag::RsvgPropertyBag;
 
 use state::RsvgState;
@@ -38,41 +39,57 @@ pub trait NodeTrait: Downcast {
 impl_downcast! (NodeTrait);
 
 #[derive(Debug)]
-pub struct AttributeError {
-    attr_name: &'static str,
-    error: Box<error::Error + Send + Sync>
+pub enum AttributeError {
+    // parse error
+    Parse (ParseError),
+
+    // invalid value
+    Value (String)
 }
 
 #[derive(Debug)]
-pub enum Error {
-    // parse error in an attribute's value
-    AttributeParse (AttributeError),
+pub struct Error {
+    attr_name: &'static str,
+    err:       AttributeError
+}
 
-    // attribute with an invalid value
-    AttributeValue (AttributeError),
+impl Error {
+    pub fn parse_error (attr_name: &'static str, error: ParseError) -> Error {
+        Error {
+            attr_name: attr_name,
+            err: AttributeError::Parse (error)
+        }
+    }
+
+    pub fn value_error (attr_name: &'static str, description: String) -> Error {
+        Error {
+            attr_name: attr_name,
+            err: AttributeError::Value (description)
+        }
+    }
 }
 
 impl error::Error for Error {
     fn description (&self) -> &str {
-        match *self {
-            Error::AttributeParse (_) => "parse error for attribute value",
-            Error::AttributeValue (_) => "invalid attribute value"
+        match self.err {
+            AttributeError::Parse (ref n) => &n.description,
+            AttributeError::Value (_) => &"invalid attribute value"
         }
     }
 }
 
 impl fmt::Display for Error {
     fn fmt (&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
-            Error::AttributeParse (ref ae) => write! (f,
-                                                      "error parsing value for attribute \"{}\": {}",
-                                                      ae.attr_name,
-                                                      ae.error),
-
-            Error::AttributeValue (ref ae) => write! (f,
-                                                      "invalid value for attribute \"{}\": {}",
-                                                      ae.attr_name,
-                                                      ae.error)
+        match self.err {
+            AttributeError::Parse (ref n) => write! (f,
+                                                     "error parsing value for attribute \"{}\": {}",
+                                                     self.attr_name,
+                                                     n.display),
+
+            AttributeError::Value (ref s) => write! (f,
+                                                     "invalid value for attribute \"{}\": {}",
+                                                     self.attr_name,
+                                                     s)
         }
     }
 }
diff --git a/rust/src/parsers.rs b/rust/src/parsers.rs
index 60050eb..abbea3c 100644
--- a/rust/src/parsers.rs
+++ b/rust/src/parsers.rs
@@ -1,7 +1,30 @@
-use nom::{IResult, is_digit, double, is_alphabetic};
+use nom::{IResult, double, is_alphabetic};
 use std::str;
 use std::f64::consts::*;
 
+// I don't know how to copy a nom::IError for long-term storage
+// (i.e. when it can no longer reference the &[u8]).  So, we explode a
+// nom::IError into a simple error struct that can be passed around.
+#[derive(Debug, Clone, PartialEq)]
+pub struct ParseError {
+    pub description: String,
+    pub display: String
+}
+
+/*
+impl<'a> From<IError<&'a [u8]>> for NomError {
+    fn from (e: IError<&[u8]>) -> NomError {
+        match e {
+            IError::Error (err) => NomError { description: err.description ().to_string (),
+                                              display: format! ("{}", err) },
+
+            IError::Incomplete (_) => NomError { description: "incomplete data".to_string (),
+                                                 display: "incomplete data".to_string () }
+        }
+    }
+}
+*/
+
 fn is_whitespace (c: u8) -> bool {
     match c as char {
         ' ' | '\t' | '\r' | '\n' => true,
@@ -30,9 +53,6 @@ named! (comma_wsp,
 //
 // Returns an f64 angle in degrees
 
-#[derive(Debug, Copy, Clone, PartialEq)]
-pub struct ParseAngleError;
-
 fn is_alphabetic_or_dash (c: u8) -> bool {
      is_alphabetic (c) || c == '-' as u8 || c == '%' as u8
 }
@@ -41,7 +61,7 @@ named! (pub number_and_units<(f64, &[u8])>,
         tuple! (double,
                 take_while! (is_alphabetic_or_dash)));
 
-pub fn angle_degrees (s: &str) -> Result <f64, ParseAngleError> {
+pub fn angle_degrees (s: &str) -> Result <f64, ParseError> {
     let r = number_and_units (s.as_bytes ()).to_full_result ();
 
     match r {
@@ -51,11 +71,13 @@ pub fn angle_degrees (s: &str) -> Result <f64, ParseAngleError> {
                 b"grad" => Ok (value * 360.0 / 400.0),
                 b"rad"  => Ok (value * 180.0 / PI),
                 b""     => Ok (value),
-                _       => Err (ParseAngleError)
+                _       => Err (ParseError { description: "angle parse error".to_string (),
+                                             display: "expected (\"deg\", \"rad\", \"grad\")? after 
number".to_string () })
             }
         },
 
-        _ => Err (ParseAngleError)
+        _ => Err (ParseError { description: "angle parse error".to_string (),
+                               display: "expected a number".to_string () })
     }
 }
 
@@ -92,10 +114,24 @@ named! (pub coordinate_pair<(f64, f64)>,
 // Parse a list-of-points as for polyline and polygon elements
 // https://www.w3.org/TR/SVG/shapes.html#PointsBNF
 
-named! (pub list_of_points<Vec<(f64, f64)>>,
+named! (list_of_points_impl<Vec<(f64, f64)>>,
         terminated! (separated_list! (comma_wsp, coordinate_pair),
                      eof! ()));
 
+pub fn list_of_points (string: &[u8]) -> Result <Vec<(f64, f64)>, ParseError> {
+    list_of_points_impl (string)
+        .to_full_result ()
+        .map_err (|_| ParseError { description: "parse error".to_string (),
+                                   display: "invalid syntax for list of points".to_string () })
+    /*
+        .map_err (|e| match e { IError::Error (err) => ParseError { description: err.description 
().to_string (),
+                                                                    display: format! ("{}", err) },
+                                _ => ParseError { description: "incomplete data".to_string (),
+                                                  display: "incomplete list of points".to_string () }
+        })
+     */
+}
+
 named! (pub separated_numbers<Vec<f64>>,
         separated_list! (comma_wsp, double));
 
@@ -195,27 +231,18 @@ mod tests {
     #[test]
     fn parses_list_of_points () {
         // FIXME: we are missing optional whitespace at the beginning and end of the list
-        assert_eq! (list_of_points (b"1 2"),  IResult::Done (&b""[..], vec! [(1.0, 2.0)]));
-        assert_eq! (list_of_points (b"1 2 3 4"),  IResult::Done (&b""[..], vec! [(1.0, 2.0), (3.0, 4.0)]));
-        assert_eq! (list_of_points (b"1,2,3,4"),  IResult::Done (&b""[..], vec! [(1.0, 2.0), (3.0, 4.0)]));
-        assert_eq! (list_of_points (b"1,2 3,4"),  IResult::Done (&b""[..], vec! [(1.0, 2.0), (3.0, 4.0)]));
-        assert_eq! (list_of_points (b"1,2 -3,4"),  IResult::Done (&b""[..], vec! [(1.0, 2.0), (-3.0, 4.0)]));
-        assert_eq! (list_of_points (b"1,2,-3,4"),  IResult::Done (&b""[..], vec! [(1.0, 2.0), (-3.0, 4.0)]));
+        assert_eq! (list_of_points (b"1 2"),      Ok (vec! [(1.0, 2.0)]));
+        assert_eq! (list_of_points (b"1 2 3 4"),  Ok (vec! [(1.0, 2.0), (3.0, 4.0)]));
+        assert_eq! (list_of_points (b"1,2,3,4"),  Ok (vec! [(1.0, 2.0), (3.0, 4.0)]));
+        assert_eq! (list_of_points (b"1,2 3,4"),  Ok (vec! [(1.0, 2.0), (3.0, 4.0)]));
+        assert_eq! (list_of_points (b"1,2 -3,4"), Ok (vec! [(1.0, 2.0), (-3.0, 4.0)]));
+        assert_eq! (list_of_points (b"1,2,-3,4"), Ok (vec! [(1.0, 2.0), (-3.0, 4.0)]));
     }
 
     #[test]
     fn errors_on_invalid_list_of_points () {
-        let result = list_of_points (b"-1-2-3-4");
-        match result {
-            IResult::Error (_) => { },
-            _ => { panic! ("{:?} should be an invalid list-of-points", result); }
-        }
-
-        let result = list_of_points (b"1 2-3,-4");
-        match result {
-            IResult::Error (_) => { },
-            _ => { panic! ("{:?} should be an invalid list-of-points", result); }
-        }
+        assert! (list_of_points (b"-1-2-3-4").is_err ());
+        assert! (list_of_points (b"1 2-3,-4").is_err ());
     }
 
     #[test]
@@ -233,8 +260,8 @@ mod tests {
         assert_eq! (angle_degrees ("1rad"),     Ok (180.0 / PI));
         assert_eq! (angle_degrees ("-400grad"), Ok (-360.0));
 
-        assert_eq! (angle_degrees (""), Err (ParseAngleError));
-        assert_eq! (angle_degrees ("foo"), Err (ParseAngleError));
-        assert_eq! (angle_degrees ("300foo"), Err (ParseAngleError));
+        assert! (angle_degrees ("").is_err ());
+        assert! (angle_degrees ("foo").is_err ());
+        assert! (angle_degrees ("300foo").is_err ());
     }
 }
diff --git a/rust/src/shapes.rs b/rust/src/shapes.rs
index 9291205..8027758 100644
--- a/rust/src/shapes.rs
+++ b/rust/src/shapes.rs
@@ -133,17 +133,18 @@ impl NodePoly {
 impl NodeTrait for NodePoly {
     fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
         // support for svg < 1.0 which used verts
-        if let Some (value) = property_bag::lookup (pbag, "verts").or (property_bag::lookup (pbag, 
"points")) {
-            let result = parsers::list_of_points (value.trim ().as_bytes ()).to_full_result ();
 
-            match result {
-                Ok (v) => {
-                    *self.points.borrow_mut () = Some (v);
-                },
+        for name in vec! ["verts", "points"] {
+            if let Some (value) = property_bag::lookup (pbag, name) {
+                let result = parsers::list_of_points (value.trim ().as_bytes ());
 
-                Err (_) => {
-                    // FIXME: propagate errors upstream
-                    *self.points.borrow_mut () = None;
+                match result {
+                    Ok (v) => {
+                        *self.points.borrow_mut () = Some (v);
+                        break;
+                    },
+
+                    Err (e) => { return Err (Error::parse_error (name, e)); }
                 }
             }
         }