[librsvg] RsvgLength::parse() - return a Result, not an RsvgLength
- From: Federico Mena Quintero <federico src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [librsvg] RsvgLength::parse() - return a Result, not an RsvgLength
- Date: Tue, 28 Feb 2017 04:13:26 +0000 (UTC)
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]
