[librsvg] RsvgLength::parse() - return a Result, not an RsvgLength



commit e299ef0e285f7d1267528a38d8c41596b89da526
Author: Federico Mena Quintero <federico gnome org>
Date:   Mon Feb 27 22:12:15 2017 -0600

    RsvgLength::parse() - return a Result, not an RsvgLength
    
    The callers now deal with errors from parsing.  In reality we ignore
    the errors and fall back to default values, but the parsing machinery
    is there now to detect errors.

 rust/src/gradient.rs     |   14 ++--
 rust/src/length.rs       |  198 ++++++++++++++++++++++-----------------------
 rust/src/marker.rs       |    6 +-
 rust/src/parsers.rs      |    6 +-
 rust/src/pattern.rs      |    8 +-
 rust/src/property_bag.rs |    4 +-
 rust/src/shapes.rs       |   20 ++++-
 7 files changed, 133 insertions(+), 123 deletions(-)
---
diff --git a/rust/src/gradient.rs b/rust/src/gradient.rs
index c83c2d6..8ecc763 100644
--- a/rust/src/gradient.rs
+++ b/rust/src/gradient.rs
@@ -192,16 +192,16 @@ impl GradientVariant {
 
         match *self {
             GradientVariant::Linear { ref mut x1, ref mut y1, ref mut x2, ref mut y2 } => {
-                fallback_to! (*x1, Some (RsvgLength::parse ("0%", LengthDir::Horizontal)));
-                fallback_to! (*y1, Some (RsvgLength::parse ("0%", LengthDir::Vertical)));
-                fallback_to! (*x2, Some (RsvgLength::parse ("100%", LengthDir::Horizontal)));
-                fallback_to! (*y2, Some (RsvgLength::parse ("0%", LengthDir::Vertical)));
+                fallback_to! (*x1, Some (RsvgLength::parse ("0%", LengthDir::Horizontal).unwrap ()));
+                fallback_to! (*y1, Some (RsvgLength::parse ("0%", LengthDir::Vertical).unwrap ()));
+                fallback_to! (*x2, Some (RsvgLength::parse ("100%", LengthDir::Horizontal).unwrap ()));
+                fallback_to! (*y2, Some (RsvgLength::parse ("0%", LengthDir::Vertical).unwrap ()));
             },
 
             GradientVariant::Radial { ref mut cx, ref mut cy, ref mut r, ref mut fx, ref mut fy } => {
-                fallback_to! (*cx, Some (RsvgLength::parse ("50%", LengthDir::Horizontal)));
-                fallback_to! (*cy, Some (RsvgLength::parse ("50%", LengthDir::Vertical)));
-                fallback_to! (*r,  Some (RsvgLength::parse ("50%", LengthDir::Both)));
+                fallback_to! (*cx, Some (RsvgLength::parse ("50%", LengthDir::Horizontal).unwrap ()));
+                fallback_to! (*cy, Some (RsvgLength::parse ("50%", LengthDir::Vertical).unwrap ()));
+                fallback_to! (*r,  Some (RsvgLength::parse ("50%", LengthDir::Both).unwrap ()));
 
                 /* fx and fy fall back to the presentational value of cx and cy */
                 fallback_to! (*fx, *cx);
diff --git a/rust/src/length.rs b/rust/src/length.rs
index 69bb04e..112db76 100644
--- a/rust/src/length.rs
+++ b/rust/src/length.rs
@@ -5,9 +5,9 @@ use std::f64;
 
 use self::glib::translate::*;
 
-use strtod::*;
 use drawing_ctx;
 use drawing_ctx::RsvgDrawingCtx;
+use parsers;
 
 /* Keep this in sync with ../../rsvg-private.h:LengthUnit */
 #[repr(C)]
@@ -70,7 +70,7 @@ fn compute_named_size (name: &str) -> f64 {
         "large"    => { power = 1.0; },
         "x-large"  => { power = 2.0; },
         "xx-large" => { power = 3.0; },
-        _          => { return 0.0; }
+        _          => { unreachable! (); }
     }
 
     12.0 * 1.2f64.powf (power) / POINTS_PER_INCH
@@ -80,7 +80,8 @@ fn compute_named_size (name: &str) -> f64 {
 pub extern fn rsvg_length_parse (string: *const libc::c_char, dir: LengthDir) -> RsvgLength {
     let my_string = unsafe { &String::from_glib_none (string) };
 
-    RsvgLength::parse (my_string, dir)
+    // FIXME: this ignores errors; propagate them upstream
+    RsvgLength::parse (my_string, dir).unwrap_or (RsvgLength::default ())
 }
 
 /* https://www.w3.org/TR/SVG/types.html#DataTypeLength
@@ -94,84 +95,79 @@ pub extern fn rsvg_length_parse (string: *const libc::c_char, dir: LengthDir) ->
  * inside RsvgLength::normalize(), when it needs to know to what the
  * length refers.
  */
-impl RsvgLength {
-    pub fn parse (string: &str, dir: LengthDir) -> RsvgLength {
-        let unit: LengthUnit;
-
-        let (mut value, rest) = strtod (string);
-
-        match rest.as_ref () {
-            "%" => {
-                value *= 0.01; // normalize to [0, 1]
-                unit = LengthUnit::Percent;
-            },
-
-            "em" => {
-                unit = LengthUnit::FontEm;
-            },
-
-            "ex" => {
-                unit = LengthUnit::FontEx;
-            },
+#[derive(Debug, Copy, Clone, PartialEq)]
+pub struct ParseLengthError;
 
-            "pt" => {
-                value /= POINTS_PER_INCH;
-                unit = LengthUnit::Inch;
-            },
-
-            "in" => {
-                unit = LengthUnit::Inch;
-            },
-
-            "cm" => {
-                value /= CM_PER_INCH;
-                unit = LengthUnit::Inch;
-            },
-
-            "mm" => {
-                value /= MM_PER_INCH;
-                unit = LengthUnit::Inch;
-            },
-
-            "pc" => {
-                value /= PICA_PER_INCH;
-                unit = LengthUnit::Inch;
-            },
-
-            "larger" => {
-                unit = LengthUnit::RelativeLarger;
-            },
-
-            "smaller" => {
-                unit = LengthUnit::RelativeSmaller;
-            },
-
-            "xx-small" |
-            "x-small" |
-            "small" |
-            "medium" |
-            "large" |
-            "x-large" |
-            "xx-large" => {
-                value = compute_named_size (rest);
-                unit = LengthUnit::Inch;
-            },
-
-            "px" |
-            "" => {
-                unit = LengthUnit::Default;
+impl RsvgLength {
+    pub fn parse (string: &str, dir: LengthDir) -> Result <RsvgLength, ParseLengthError> {
+        let r = parsers::number_and_units (string.as_bytes ()).to_full_result ();
+
+        match r {
+            Ok ((value, unit)) => {
+                match unit {
+                    b"%" => Ok (RsvgLength { length: value * 0.01, // normalize to [0, 1]
+                                             unit:   LengthUnit::Percent,
+                                             dir:    dir }),
+
+                    b"em" => Ok (RsvgLength { length: value,
+                                              unit:   LengthUnit::FontEm,
+                                              dir:    dir }),
+
+                    b"ex" => Ok (RsvgLength { length: value,
+                                              unit:   LengthUnit::FontEx,
+                                              dir:    dir }),
+
+                    b"pt" => Ok (RsvgLength { length: value / POINTS_PER_INCH,
+                                              unit:   LengthUnit::Inch,
+                                              dir:    dir }),
+
+                    b"in" => Ok (RsvgLength { length: value,
+                                              unit:   LengthUnit::Inch,
+                                              dir:    dir }),
+
+                    b"cm" => Ok (RsvgLength { length: value / CM_PER_INCH,
+                                              unit:   LengthUnit::Inch,
+                                              dir:    dir }),
+
+                    b"mm" => Ok (RsvgLength { length: value / MM_PER_INCH,
+                                              unit:   LengthUnit::Inch,
+                                              dir:    dir }),
+
+                    b"pc" => Ok (RsvgLength { length: value / PICA_PER_INCH,
+                                              unit:   LengthUnit::Inch,
+                                              dir:    dir }),
+
+                    b"px" |
+                    b"" => Ok (RsvgLength { length: value,
+                                            unit:   LengthUnit::Default,
+                                            dir:    dir }),
+
+                    _ => Err (ParseLengthError)
+                }
             },
 
-            _ => {
-                unit = LengthUnit::Default;
+            _ => match string {
+                "larger" => Ok (RsvgLength { length: 0.0,
+                                             unit:   LengthUnit::RelativeLarger,
+                                             dir:    dir }),
+
+                "smaller" => Ok (RsvgLength { length: 0.0,
+                                              unit:  LengthUnit::RelativeSmaller,
+                                              dir:   dir }),
+
+                "xx-small" |
+                "x-small" |
+                "small" |
+                "medium" |
+                "large" |
+                "x-large" |
+                "xx-large" => Ok (RsvgLength { length: compute_named_size (string),
+                                               unit:   LengthUnit::Inch,
+                                               dir:    dir }),
+
+                _ => Err (ParseLengthError)
             }
         }
-
-        RsvgLength {
-            length: value,
-            unit: unit,
-            dir: dir
-        }
     }
 
     pub fn normalize (&self, draw_ctx: *const RsvgDrawingCtx) -> f64 {
@@ -272,94 +268,94 @@ mod tests {
     #[test]
     fn parses_default () {
         assert_eq! (RsvgLength::parse ("42", LengthDir::Horizontal),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: 42.0,
                         unit:   LengthUnit::Default,
-                        dir:    LengthDir::Horizontal});
+                        dir:    LengthDir::Horizontal}));
 
         assert_eq! (RsvgLength::parse ("-42px", LengthDir::Horizontal),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: -42.0,
                         unit:   LengthUnit::Default,
-                        dir:    LengthDir::Horizontal});
+                        dir:    LengthDir::Horizontal}));
     }
 
     #[test]
     fn parses_percent () {
         assert_eq! (RsvgLength::parse ("50.0%", LengthDir::Horizontal),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: 0.5,
                         unit:   LengthUnit::Percent,
-                        dir:    LengthDir::Horizontal});
+                        dir:    LengthDir::Horizontal}));
     }
 
     #[test]
     fn parses_font_em () {
         assert_eq! (RsvgLength::parse ("22.5em", LengthDir::Vertical),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: 22.5,
                         unit:   LengthUnit::FontEm,
-                        dir:    LengthDir::Vertical });
+                        dir:    LengthDir::Vertical }));
     }
 
     #[test]
     fn parses_font_ex () {
         assert_eq! (RsvgLength::parse ("22.5ex", LengthDir::Vertical),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: 22.5,
                         unit:   LengthUnit::FontEx,
-                        dir:    LengthDir::Vertical });
+                        dir:    LengthDir::Vertical }));
     }
 
     #[test]
     fn parses_physical_units () {
         assert_eq! (RsvgLength::parse ("72pt", LengthDir::Both),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: 1.0,
                         unit:   LengthUnit::Inch,
-                        dir:    LengthDir::Both });
+                        dir:    LengthDir::Both }));
 
         assert_eq! (RsvgLength::parse ("-22.5in", LengthDir::Both),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: -22.5,
                         unit:   LengthUnit::Inch,
-                        dir:    LengthDir::Both });
+                        dir:    LengthDir::Both }));
 
         assert_eq! (RsvgLength::parse ("-25.4cm", LengthDir::Both),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: -10.0,
                         unit:   LengthUnit::Inch,
-                        dir:    LengthDir::Both });
+                        dir:    LengthDir::Both }));
 
         assert_eq! (RsvgLength::parse ("254mm", LengthDir::Both),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: 10.0,
                         unit:   LengthUnit::Inch,
-                        dir:    LengthDir::Both });
+                        dir:    LengthDir::Both }));
 
         assert_eq! (RsvgLength::parse ("60pc", LengthDir::Both),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: 10.0,
                         unit:   LengthUnit::Inch,
-                        dir:    LengthDir::Both });
+                        dir:    LengthDir::Both }));
     }
 
     #[test]
     fn parses_relative_larger () {
         assert_eq! (RsvgLength::parse ("larger", LengthDir::Vertical),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: 0.0,
                         unit:   LengthUnit::RelativeLarger,
-                        dir:    LengthDir::Vertical });
+                        dir:    LengthDir::Vertical }));
     }
 
     #[test]
     fn parses_relative_smaller () {
         assert_eq! (RsvgLength::parse ("smaller", LengthDir::Vertical),
-                    RsvgLength {
+                    Ok (RsvgLength {
                         length: 0.0,
                         unit:   LengthUnit::RelativeSmaller,
-                        dir:    LengthDir::Vertical });
+                        dir:    LengthDir::Vertical }));
     }
 
     #[test]
@@ -378,7 +374,7 @@ mod tests {
         // enforce a particular sequence.
 
         for name in names {
-            let length = RsvgLength::parse (name, LengthDir::Both);
+            let length = RsvgLength::parse (name, LengthDir::Both).unwrap ();
 
             assert_eq! (length.unit, LengthUnit::Inch);
             assert_eq! (length.dir, LengthDir::Both);
diff --git a/rust/src/marker.rs b/rust/src/marker.rs
index 4f01b67..ed6f011 100644
--- a/rust/src/marker.rs
+++ b/rust/src/marker.rs
@@ -126,7 +126,7 @@ impl NodeMarker {
 
     fn get_default_size () -> RsvgLength {
         // per the spec
-        RsvgLength::parse ("3", LengthDir::Both)
+        RsvgLength::parse ("3", LengthDir::Both).unwrap ()
     }
 
     fn render (&self,
@@ -220,9 +220,9 @@ impl NodeTrait for NodeMarker {
         self.ref_y.set (property_bag::lookup_length (pbag, "refY", LengthDir::Vertical));
 
         self.width.set (property_bag::lookup (pbag, "markerWidth").map_or (NodeMarker::get_default_size (),
-                                                                           |v| RsvgLength::parse (&v, 
LengthDir::Horizontal)));
+                                                                           |v| RsvgLength::parse (&v, 
LengthDir::Horizontal).unwrap_or (NodeMarker::get_default_size ())));
         self.height.set (property_bag::lookup (pbag, "markerHeight").map_or (NodeMarker::get_default_size (),
-                                                                             |v| RsvgLength::parse (&v, 
LengthDir::Vertical)));
+                                                                             |v| RsvgLength::parse (&v, 
LengthDir::Vertical).unwrap_or (NodeMarker::get_default_size ())));
 
         self.orient.set (property_bag::lookup_and_parse (pbag, "orient"));
         self.aspect.set (property_bag::lookup_and_parse (pbag, "preserveAspectRatio"));
diff --git a/rust/src/parsers.rs b/rust/src/parsers.rs
index 0c96c16..a7cf8e3 100644
--- a/rust/src/parsers.rs
+++ b/rust/src/parsers.rs
@@ -1,4 +1,4 @@
-use nom::{IResult, is_digit, double, double_s, ErrorKind, space, sp, is_alphabetic};
+use nom::{IResult, is_digit, double, is_alphabetic};
 use std::str;
 use std::f64::consts::*;
 
@@ -81,10 +81,10 @@ named! (comma_wsp,
 pub struct ParseAngleError;
 
 fn is_alphabetic_or_dash (c: u8) -> bool {
-     is_alphabetic (c) || c == '-' as u8
+     is_alphabetic (c) || c == '-' as u8 || c == '%' as u8
 }
 
-named! (number_and_units<(f64, &[u8])>,
+named! (pub number_and_units<(f64, &[u8])>,
         tuple! (double,
                 take_while! (is_alphabetic_or_dash)));
 
diff --git a/rust/src/pattern.rs b/rust/src/pattern.rs
index e258b39..897961f 100644
--- a/rust/src/pattern.rs
+++ b/rust/src/pattern.rs
@@ -92,10 +92,10 @@ impl Pattern {
         fallback_to! (self.preserve_aspect_ratio, Some (AspectRatio::default ()));
         fallback_to! (self.affine,                Some (cairo::Matrix::identity ()));
 
-        fallback_to! (self.x,                     Some (RsvgLength::parse ("0", LengthDir::Horizontal)));
-        fallback_to! (self.y,                     Some (RsvgLength::parse ("0", LengthDir::Horizontal)));
-        fallback_to! (self.width,                 Some (RsvgLength::parse ("0", LengthDir::Horizontal)));
-        fallback_to! (self.height,                Some (RsvgLength::parse ("0", LengthDir::Horizontal)));
+        fallback_to! (self.x,                     Some (RsvgLength::default ()));
+        fallback_to! (self.y,                     Some (RsvgLength::default ()));
+        fallback_to! (self.width,                 Some (RsvgLength::default ()));
+        fallback_to! (self.height,                Some (RsvgLength::default ()));
 
         self.fallback = None;
 
diff --git a/rust/src/property_bag.rs b/rust/src/property_bag.rs
index 1e296fd..b907951 100644
--- a/rust/src/property_bag.rs
+++ b/rust/src/property_bag.rs
@@ -46,7 +46,9 @@ pub fn lookup_length (pbag: *const RsvgPropertyBag, key: &str, length_dir: Lengt
     let value = lookup (pbag, key);
 
     if let Some (v) = value {
-        RsvgLength::parse (&v, length_dir)
+
+        // FIXME: Error is discarded here.  Figure out a way to propagate it upstream.
+        RsvgLength::parse (&v, length_dir).unwrap_or (RsvgLength::default ())
     } else {
         RsvgLength::default ()
     }
diff --git a/rust/src/shapes.rs b/rust/src/shapes.rs
index 14a4c07..beac79c 100644
--- a/rust/src/shapes.rs
+++ b/rust/src/shapes.rs
@@ -255,11 +255,23 @@ impl NodeTrait for NodeRect {
         self.w.set (property_bag::lookup_length (pbag, "width", LengthDir::Horizontal));
         self.h.set (property_bag::lookup_length (pbag, "height", LengthDir::Vertical));
 
-        self.rx.set (property_bag::lookup (pbag, "rx").map_or (None,
-                                                               |v| Some (RsvgLength::parse (&v, 
LengthDir::Horizontal))));
+        let v = property_bag::lookup (pbag, "rx");
+        if let Some (val) = v {
+            let rlength = RsvgLength::parse (&val, LengthDir::Horizontal);
+            rlength.map (|v| Some (v)).unwrap_or (None);
+            self.rx.set (rlength.map (|v| Some (v)).unwrap_or (None));
+        } else {
+            self.rx.set (None);
+        }
 
-        self.ry.set (property_bag::lookup (pbag, "ry").map_or (None,
-                                                               |v| Some (RsvgLength::parse (&v, 
LengthDir::Vertical))));
+        let v = property_bag::lookup (pbag, "ry");
+        if let Some (val) = v {
+            let rlength = RsvgLength::parse (&val, LengthDir::Vertical);
+            rlength.map (|v| Some (v)).unwrap_or (None);
+            self.ry.set (rlength.map (|v| Some (v)).unwrap_or (None));
+        } else {
+            self.ry.set (None);
+        }
     }
 
     fn draw (&self, node: &RsvgNode, draw_ctx: *const RsvgDrawingCtx, dominate: i32) {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]