Code review touchups

googlefonts · Aug 29, 2023 · 9c3e62d · 9c3e62d
1 parent 7b2e5f2
commit 9c3e62d
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 18 deletions.
diff --git a/fontc/src/lib.rs b/fontc/src/lib.rs
@@ -312,7 +312,7 @@ mod tests {
     use fontbe::orchestration::{
         AnyWorkId, Context as BeContext, Glyph, LocaFormatWrapper, WorkId as BeWorkIdentifier,
     };
-    use fontdrasil::types::{GlyphName, NOTDEF};
+    use fontdrasil::types::GlyphName;
     use fontir::{
         ir::{self, KernParticipant},
         orchestration::{Context as FeContext, Persistable, WorkId as FeWorkIdentifier},
@@ -1273,10 +1273,14 @@ mod tests {
             .fe_context
             .preliminary_glyph_order
             .get()
-            .contains(&NOTDEF));
+            .contains(&GlyphName::NOTDEF));
         assert_eq!(
             Some(0),
-            result.fe_context.glyph_order.get().glyph_id(&NOTDEF)
+            result
+                .fe_context
+                .glyph_order
+                .get()
+                .glyph_id(&GlyphName::NOTDEF)
         );
 
         let font_file = build_dir.join("font.ttf");

diff --git a/fontdrasil/src/types.rs b/fontdrasil/src/types.rs
@@ -10,10 +10,10 @@ use smol_str::SmolStr;
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
 pub struct GlyphName(SmolStr);
 
-/// The name of the undefined glyph
-pub static NOTDEF: GlyphName = GlyphName(SmolStr::new_inline(".notdef"));
-
 impl GlyphName {
+    /// The name of the undefined glyph
+    pub const NOTDEF: GlyphName = GlyphName(SmolStr::new_inline(".notdef"));
+
     pub fn as_str(&self) -> &str {
         self.0.as_str()
     }

diff --git a/fontir/src/glyph.rs b/fontir/src/glyph.rs
@@ -10,7 +10,7 @@ use std::{
 
 use fontdrasil::{
     orchestration::{Access, Work},
-    types::{GlyphName, NOTDEF},
+    types::GlyphName,
 };
 use kurbo::Affine;
 use log::{debug, log_enabled, trace};
@@ -363,15 +363,15 @@ fn ensure_notdef_exists_and_is_gid_0(
     glyph_order: &mut GlyphOrder,
 ) -> Result<(), WorkError> {
     // Make sure we have a .notdef and that it's gid 0
-    match glyph_order.glyph_id(&NOTDEF) {
+    match glyph_order.glyph_id(&GlyphName::NOTDEF) {
         Some(0) => (), // .notdef is gid 0; life is good
         Some(..) => {
-            trace!("Move {} to gid 0", NOTDEF);
-            glyph_order.set_glyph_id(&NOTDEF, 0);
+            trace!("Move {} to gid 0", GlyphName::NOTDEF);
+            glyph_order.set_glyph_id(&GlyphName::NOTDEF, 0);
         }
         None => {
-            trace!("Generate {} and make it gid 0", NOTDEF);
-            glyph_order.set_glyph_id(&NOTDEF, 0);
+            trace!("Generate {} and make it gid 0", GlyphName::NOTDEF);
+            glyph_order.set_glyph_id(&GlyphName::NOTDEF, 0);
             let static_metadata = context.static_metadata.get();
             let metrics = context
                 .global_metrics
@@ -472,10 +472,6 @@ impl Work<Context, WorkId, WorkError> for GlyphOrderWork {
             trace!("No new glyphs, final glyph order == preliminary glyph order");
         }
 
-        let Some(0) = new_glyph_order.glyph_id(&NOTDEF) else {
-            todo!(".notdef must be present and must be gid 0")
-        };
-
         context.glyph_order.set(new_glyph_order);
         Ok(())
     }

diff --git a/fontir/src/ir.rs b/fontir/src/ir.rs
@@ -13,7 +13,7 @@ use crate::{
 use chrono::{DateTime, Utc};
 use font_types::NameId;
 use font_types::Tag;
-use fontdrasil::types::{GlyphName, GroupName, NOTDEF};
+use fontdrasil::types::{GlyphName, GroupName};
 use indexmap::IndexSet;
 use kurbo::{Affine, BezPath, PathEl, Point};
 use log::warn;
@@ -1027,7 +1027,7 @@ impl GlyphBuilder {
         path.close_path();
 
         Self {
-            name: NOTDEF.clone(),
+            name: GlyphName::NOTDEF.clone(),
             codepoints: HashSet::from([0]),
             sources: HashMap::from([(
                 default_location,