Merge pull request #1151 from googlefonts/better-tag-errors

Better reporting of InvalidTag errors

Author: cmyr

fontir/src/error.rs

@@ -71,8 +71,8 @@ pub enum Error {
         mappings: Vec<(UserCoord, DesignCoord)>,
         value: DesignCoord,
     },
-    #[error("Invalid tag")]
-    InvalidTag(#[from] InvalidTag),
+    #[error("Invalid tag '{raw_tag}': {cause}")]
+    InvalidTag { raw_tag: String, cause: InvalidTag },
     #[error("Source file contained a construct we don't yet support: {0}")]
     UnsupportedConstruct(String),
 }

glyphs2fontir/src/source.rs

@@ -447,7 +447,11 @@ impl Work<Context, WorkId, Error> for StaticMetadataWork {
         .map_err(Error::VariationModelError)?;
         static_metadata.misc.selection_flags = selection_flags;
         if let Some(vendor_id) = font.vendor_id() {
-            static_metadata.misc.vendor_id = Tag::from_str(vendor_id).map_err(Error::InvalidTag)?;
+            static_metadata.misc.vendor_id =
+                Tag::from_str(vendor_id).map_err(|cause| Error::InvalidTag {
+                    raw_tag: vendor_id.to_owned(),
+                    cause,
+                })?;
         }
 
         // Default per <https://github.com/googlefonts/glyphsLib/blob/cb8a4a914b0a33431f0a77f474bf57eec2f19bcc/Lib/glyphsLib/builder/custom_params.py#L1117-L1119>

glyphs2fontir/src/toir.rs

@@ -207,7 +207,10 @@ fn to_ir_axis(
 
     Ok(fontdrasil::types::Axis {
         name: axis.name.clone(),
-        tag: Tag::from_str(&axis.tag).map_err(Error::InvalidTag)?,
+        tag: Tag::from_str(&axis.tag).map_err(|cause| Error::InvalidTag {
+            raw_tag: axis.tag.clone(),
+            cause,
+        })?,
         hidden: axis.hidden.unwrap_or(false),
         min: min.to_user(&converter),
         default: default.to_user(&converter),

ufo2fontir/src/source.rs

@@ -30,7 +30,7 @@ use norad::{
 };
 use write_fonts::{
     tables::{gdef::GlyphClassDef, os2::SelectionFlags},
-    types::{InvalidTag, NameId, Tag},
+    types::{NameId, Tag},
     OtRound,
 };
 
@@ -305,8 +305,15 @@ impl Source for DesignSpaceIrSource {
             .designspace
             .axes
             .iter()
-            .map(|a| Tag::from_str(&a.tag).map(|tag| (a.name.as_str(), tag)))
-            .collect::<Result<HashMap<&str, Tag>, InvalidTag>>()?;
+            .map(|a| {
+                Tag::from_str(&a.tag)
+                    .map(|tag| (a.name.as_str(), tag))
+                    .map_err(|cause| Error::InvalidTag {
+                        cause,
+                        raw_tag: a.tag.clone(),
+                    })
+            })
+            .collect::<Result<HashMap<&str, Tag>, _>>()?;
         for (idx, source) in sources_default_first.iter().enumerate() {
             // Track files within each UFO
             // The UFO dir *must* exist since we were able to find fontinfo in it earlier
@@ -952,7 +959,11 @@ impl Work<Context, WorkId, Error> for StaticMetadataWork {
         .map_err(Error::VariationModelError)?;
         static_metadata.misc.selection_flags = selection_flags;
         if let Some(vendor_id) = &font_info_at_default.open_type_os2_vendor_id {
-            static_metadata.misc.vendor_id = Tag::from_str(vendor_id).map_err(Error::InvalidTag)?;
+            static_metadata.misc.vendor_id =
+                Tag::from_str(vendor_id).map_err(|cause| Error::InvalidTag {
+                    raw_tag: vendor_id.to_owned(),
+                    cause,
+                })?;
         }
 
         // <https://github.com/googlefonts/glyphsLib/blob/cb8a4a914b0a33431f0a77f474bf57eec2f19bcc/Lib/glyphsLib/builder/custom_params.py#L1117-L1119>

ufo2fontir/src/toir.rs

@@ -114,7 +114,10 @@ pub fn to_ir_axes(axes: &[designspace::Axis]) -> Result<Vec<fontdrasil::types::A
 }
 
 pub fn to_ir_axis(axis: &designspace::Axis) -> Result<fontdrasil::types::Axis, Error> {
-    let tag = Tag::from_str(&axis.tag).map_err(Error::InvalidTag)?;
+    let tag = Tag::from_str(&axis.tag).map_err(|cause| Error::InvalidTag {
+        raw_tag: axis.tag.clone(),
+        cause,
+    })?;
 
     // <https://fonttools.readthedocs.io/en/latest/designspaceLib/xml.html#axis-element>
     let min = UserCoord::new(axis.minimum.unwrap());