[librsvg: 2/3] Fix #318 - Use a custom parser for the font-weight property

[Federico Mena Quintero <federico src gnome org>]

commit 913a589b11620ff626bed523314b1f145396138d
Author: Federico Mena Quintero <federico gnome org>
Date:   Wed Aug 8 18:38:44 2018 -0500

    Fix #318 - Use a custom parser for the font-weight property
    
    When I moved this to to an identifier-based parser, I broke the
    ability to have numbers like "100" for font-weight.
    
    https://gitlab.gnome.org/GNOME/librsvg/issues/318

 rsvg_internals/src/font_props.rs | 89 +++++++++++++++++++++++++++++++++++++++-
 rsvg_internals/src/state.rs      | 23 +++--------
 rsvg_internals/src/text.rs       | 34 +++++++--------
 3 files changed, 110 insertions(+), 36 deletions(-)
---
diff --git a/rsvg_internals/src/font_props.rs b/rsvg_internals/src/font_props.rs
index 32963869..db2408a0 100644
--- a/rsvg_internals/src/font_props.rs
+++ b/rsvg_internals/src/font_props.rs
@@ -3,7 +3,7 @@ use cssparser::{Parser, Token};
 use drawing_ctx::DrawingCtx;
 use error::*;
 use length::{Length, LengthDir, LengthUnit, POINTS_PER_INCH};
-use parsers::Parse;
+use parsers::{Parse, ParseError};
 use state::ComputedValues;
 
 #[derive(Debug, Copy, Clone, PartialEq)]
@@ -111,6 +111,70 @@ impl Parse for FontSizeSpec {
     }
 }
 
+// https://www.w3.org/TR/2008/REC-CSS2-20080411/fonts.html#propdef-font-weight
+#[derive(Debug, Copy, Clone, PartialEq)]
+pub enum FontWeightSpec {
+    Normal,
+    Bold,
+    Bolder,
+    Lighter,
+    W100, // FIXME: we should use Weight(100),
+    W200, // but we need a smarter macro for that
+    W300,
+    W400,
+    W500,
+    W600,
+    W700,
+    W800,
+    W900,
+}
+
+impl Parse for FontWeightSpec {
+    type Data = ();
+    type Err = AttributeError;
+
+    fn parse(
+        parser: &mut Parser,
+        _: Self::Data,
+    ) -> Result<FontWeightSpec, ::error::AttributeError> {
+        if let Ok(r) = parser.try(|p| {
+            p.expect_ident()
+                .map_err(|_| ())
+                .and_then(|cow| match cow.as_ref() {
+                    "normal" => Ok(FontWeightSpec::Normal),
+                    "bold" => Ok(FontWeightSpec::Bold),
+                    "bolder" => Ok(FontWeightSpec::Bolder),
+                    "lighter" => Ok(FontWeightSpec::Lighter),
+                    _ => Err(()),
+                })
+        }) {
+            return Ok(r);
+        }
+
+        if let Ok(r) = parser
+            .expect_integer()
+            .map_err(|_| ())
+            .and_then(|i| match i {
+                100 => Ok(FontWeightSpec::W100),
+                200 => Ok(FontWeightSpec::W200),
+                300 => Ok(FontWeightSpec::W300),
+                400 => Ok(FontWeightSpec::W400),
+                500 => Ok(FontWeightSpec::W500),
+                600 => Ok(FontWeightSpec::W600),
+                700 => Ok(FontWeightSpec::W700),
+                800 => Ok(FontWeightSpec::W800),
+                900 => Ok(FontWeightSpec::W900),
+                _ => Err(()),
+            }) {
+            Ok(r)
+        } else {
+            Err(AttributeError::Parse(ParseError::new(
+                "invalid font-weight specification",
+            )))
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -120,4 +184,27 @@ mod tests {
         assert!(is_parse_error(&FontSizeSpec::parse_str("furlong", ())));
     }
 
+    #[test]
+    fn parses_font_weight() {
+        assert_eq!(
+            <FontWeightSpec as Parse>::parse_str("normal", ()),
+            Ok(FontWeightSpec::Normal)
+        );
+        assert_eq!(
+            <FontWeightSpec as Parse>::parse_str("bold", ()),
+            Ok(FontWeightSpec::Bold)
+        );
+        assert_eq!(
+            <FontWeightSpec as Parse>::parse_str("100", ()),
+            Ok(FontWeightSpec::W100)
+        );
+    }
+
+    #[test]
+    fn detects_invalid_font_weight() {
+        assert!(<FontWeightSpec as Parse>::parse_str("", ()).is_err());
+        assert!(<FontWeightSpec as Parse>::parse_str("strange", ()).is_err());
+        assert!(<FontWeightSpec as Parse>::parse_str("314", ()).is_err());
+        assert!(<FontWeightSpec as Parse>::parse_str("3.14", ()).is_err());
+    }
 }
diff --git a/rsvg_internals/src/state.rs b/rsvg_internals/src/state.rs
index 78c40fb8..9ad0d301 100644
--- a/rsvg_internals/src/state.rs
+++ b/rsvg_internals/src/state.rs
@@ -8,7 +8,7 @@ use std::str::FromStr;
 
 use attributes::Attribute;
 use error::*;
-use font_props::FontSizeSpec;
+use font_props::{FontSizeSpec, FontWeightSpec};
 use handle::RsvgHandle;
 use iri::IRI;
 use length::{Dasharray, Length, LengthDir, LengthUnit};
@@ -1015,27 +1015,14 @@ make_property!(
     "small-caps" => SmallCaps,
 );
 
-// https://www.w3.org/TR/SVG/text.html#FontWeightProperty
+// https://www.w3.org/TR/2008/REC-CSS2-20080411/fonts.html#propdef-font-weight
 make_property!(
     ComputedValues,
     FontWeight,
-    default: Normal,
+    default: FontWeightSpec::Normal,
     inherits_automatically: true,
-
-    identifiers:
-    "normal" => Normal,
-    "bold" => Bold,
-    "bolder" => Bolder,
-    "lighter" => Lighter,
-    "100" => W100, // FIXME: we should use Weight(100),
-    "200" => W200, // but we need a smarter macro for that
-    "300" => W300,
-    "400" => W400,
-    "500" => W500,
-    "600" => W600,
-    "700" => W700,
-    "800" => W800,
-    "900" => W900,
+    newtype_parse: FontWeightSpec,
+    parse_data_type: ()
 );
 
 // https://www.w3.org/TR/SVG/text.html#LetterSpacingProperty
diff --git a/rsvg_internals/src/text.rs b/rsvg_internals/src/text.rs
index 44669ab9..037a2474 100644
--- a/rsvg_internals/src/text.rs
+++ b/rsvg_internals/src/text.rs
@@ -6,6 +6,7 @@ use std::str;
 
 use attributes::Attribute;
 use drawing_ctx::DrawingCtx;
+use font_props::FontWeightSpec;
 use handle::RsvgHandle;
 use length::*;
 use node::{boxed_node_new, CascadedValues, NodeResult, NodeTrait, NodeType, RsvgNode};
@@ -18,7 +19,6 @@ use state::{
     FontStretch,
     FontStyle,
     FontVariant,
-    FontWeight,
     TextAnchor,
     UnicodeBidi,
     WritingMode,
@@ -410,22 +410,22 @@ impl From<FontStretch> for pango::Stretch {
     }
 }
 
-impl From<FontWeight> for pango::Weight {
-    fn from(w: FontWeight) -> pango::Weight {
+impl From<FontWeightSpec> for pango::Weight {
+    fn from(w: FontWeightSpec) -> pango::Weight {
         match w {
-            FontWeight::Normal => pango::Weight::Normal,
-            FontWeight::Bold => pango::Weight::Bold,
-            FontWeight::Bolder => pango::Weight::Ultrabold,
-            FontWeight::Lighter => pango::Weight::Light,
-            FontWeight::W100 => pango::Weight::Thin,
-            FontWeight::W200 => pango::Weight::Ultralight,
-            FontWeight::W300 => pango::Weight::Semilight,
-            FontWeight::W400 => pango::Weight::Normal,
-            FontWeight::W500 => pango::Weight::Medium,
-            FontWeight::W600 => pango::Weight::Semibold,
-            FontWeight::W700 => pango::Weight::Bold,
-            FontWeight::W800 => pango::Weight::Ultrabold,
-            FontWeight::W900 => pango::Weight::Heavy,
+            FontWeightSpec::Normal => pango::Weight::Normal,
+            FontWeightSpec::Bold => pango::Weight::Bold,
+            FontWeightSpec::Bolder => pango::Weight::Ultrabold,
+            FontWeightSpec::Lighter => pango::Weight::Light,
+            FontWeightSpec::W100 => pango::Weight::Thin,
+            FontWeightSpec::W200 => pango::Weight::Ultralight,
+            FontWeightSpec::W300 => pango::Weight::Semilight,
+            FontWeightSpec::W400 => pango::Weight::Normal,
+            FontWeightSpec::W500 => pango::Weight::Medium,
+            FontWeightSpec::W600 => pango::Weight::Semibold,
+            FontWeightSpec::W700 => pango::Weight::Bold,
+            FontWeightSpec::W800 => pango::Weight::Ultrabold,
+            FontWeightSpec::W900 => pango::Weight::Heavy,
         }
     }
 }
@@ -509,7 +509,7 @@ fn create_pango_layout(
 
     font_desc.set_variant(pango::Variant::from(values.font_variant));
 
-    font_desc.set_weight(pango::Weight::from(values.font_weight));
+    font_desc.set_weight(pango::Weight::from(values.font_weight.0));
 
     font_desc.set_stretch(pango::Stretch::from(values.font_stretch));