Layer API improvements and fixups

- Added LayerSet::new
- Added Layer::rename_glyph
- Fixed bug where get_glyph_mut used Arc::get_mut instead of Arc::make_mut
- Renamed get_glyph to glyph
- Renamed get_glyph_mut to glyph_mut
- Renamed iter_contents & iter_contents_mut -> iter & iter_mut
- Removed deprecated method

A few other things along these lines

examples/normalize.rs

@@ -30,7 +30,7 @@ fn main() {
         });
 
         // Prune all glyphs' libs.
-        for glyph in default_layer.iter_contents_mut() {
+        for glyph in default_layer.iter_mut() {
             glyph.lib.retain(|k, &mut _| {
                 (k.starts_with("public.")
                     || k.starts_with("com.schriftgestaltung.")

src/error.rs

@@ -23,6 +23,14 @@ pub enum Error {
     MissingLayer(String),
     DuplicateLayer(String),
     MissingLayerContents,
+    DuplicateGlyph {
+        layer: String,
+        glyph: String,
+    },
+    MissingGlyph {
+        layer: String,
+        glyph: String,
+    },
     IoError(IoError),
     ParseError(XmlError),
     Glif(GlifError),
@@ -144,6 +152,12 @@ impl std::fmt::Display for Error {
             Error::MissingLayerContents => {
                 write!(f, "Missing required 'layercontents.plist' file.")
             }
+            Error::DuplicateGlyph { layer, glyph } => {
+                write!(f, "Glyph named '{}' already exists in layer '{}'", glyph, layer)
+            }
+            Error::MissingGlyph { layer, glyph } => {
+                write!(f, "Glyph '{}' missing from layer '{}'", glyph, layer)
+            }
             Error::IoError(e) => e.fmt(f),
             Error::ParseError(e) => e.fmt(f),
             Error::Glif(GlifError { path, position, kind }) => {

src/layer.rs

@@ -63,6 +63,15 @@ impl LayerSet {
         Ok(LayerSet { layers })
     }
 
+    /// Create a new `LayerSet`.
+    ///
+    /// Will panic if `layers` is empty.
+    pub fn new(mut layers: Vec<Layer>) -> Self {
+        assert!(!layers.is_empty());
+        layers.first_mut().unwrap().path = DEFAULT_GLYPHS_DIRNAME.into();
+        LayerSet { layers }
+    }
+
     /// The number of layers in the set.
     ///
     /// This should be non-zero.
@@ -323,7 +332,7 @@ impl Layer {
         Ok(())
     }
 
-    /// The number of glyphs in this layer.
+    /// The number of [`Glyph`]s in the layer.
     pub fn len(&self) -> usize {
         self.glyphs.len()
     }
@@ -350,7 +359,7 @@ impl Layer {
     }
 
     /// Returns a reference the glyph with the given name, if it exists.
-    pub fn get_glyph<K>(&self, glyph: &K) -> Option<&Arc<Glyph>>
+    pub fn glyph<K>(&self, glyph: &K) -> Option<&Arc<Glyph>>
     where
         GlyphName: Borrow<K>,
         K: Ord + ?Sized,
@@ -359,14 +368,36 @@ impl Layer {
     }
 
     /// Returns a mutable reference to the glyph with the given name, if it exists.
+    pub fn glyph_mut<K>(&mut self, glyph: &K) -> Option<&mut Glyph>
+    where
+        GlyphName: Borrow<K>,
+        K: Ord + ?Sized,
+    {
+        self.glyphs.get_mut(glyph).map(|g| Arc::make_mut(g))
+    }
+
+    #[doc(hidden)]
+    #[deprecated(since = "0.4.0", note = "renamed to 'glyph'")]
+    pub fn get_glyph<K>(&self, glyph: &K) -> Option<&Arc<Glyph>>
+    where
+        GlyphName: Borrow<K>,
+        K: Ord + ?Sized,
+    {
+        self.glyph(glyph)
+    }
+
+    #[doc(hidden)]
+    #[deprecated(since = "0.4.0", note = "renamed to 'glyph_mut'")]
     pub fn get_glyph_mut<K>(&mut self, glyph: &K) -> Option<&mut Glyph>
     where
         GlyphName: Borrow<K>,
         K: Ord + ?Sized,
     {
-        self.glyphs.get_mut(glyph).and_then(|g| Arc::get_mut(g))
+        self.glyph_mut(glyph)
     }
 
+    /// Returns a mutable reference to the glyph with the given name, if it exists.
+
     /// Returns `true` if this layer contains a glyph with this name.
     pub fn contains_glyph(&self, name: &str) -> bool {
         self.glyphs.contains_key(name)
@@ -391,26 +422,51 @@ impl Layer {
         self.glyphs.clear()
     }
 
-    /// Remove the named glyph from this layer.
-    #[doc(hidden)]
-    #[deprecated(since = "0.3.0", note = "use remove_glyph instead")]
-    pub fn delete_glyph(&mut self, name: &str) {
-        self.remove_glyph(name);
-    }
-
     /// Remove the named glyph from this layer and return it, if it exists.
     pub fn remove_glyph(&mut self, name: &str) -> Option<Arc<Glyph>> {
         self.contents.remove(name);
         self.glyphs.remove(name)
     }
 
+    /// Rename a glyph.
+    ///
+    /// If `overwrite` is true, and a glyph with the new name exists, it will
+    /// be replaced.
+    ///
+    /// Returns an error if `overwrite` is false but a glyph with the new
+    /// name exists, or if no glyph with the old name exists.
+    pub fn rename_glyph(&mut self, old: &str, new: &str, overwrite: bool) -> Result<(), Error> {
+        if !overwrite && self.glyphs.contains_key(new) {
+            Err(Error::DuplicateGlyph { glyph: new.into(), layer: self.name.to_string() })
+        } else if !self.glyphs.contains_key(old) {
+            Err(Error::MissingGlyph { glyph: old.into(), layer: self.name.to_string() })
+        } else {
+            let mut g = self.remove_glyph(old).unwrap();
+            Arc::make_mut(&mut g).name = new.into();
+            self.insert_glyph(g);
+            Ok(())
+        }
+    }
+
+    #[doc(hidden)]
+    #[deprecated(since = "0.4.0", note = "renamed to 'iter'")]
+    pub fn iter_contents(&self) -> impl Iterator<Item = &Arc<Glyph>> + '_ {
+        self.iter()
+    }
+
+    #[doc(hidden)]
+    #[deprecated(since = "0.4.0", note = "renamed to 'iter_mut'")]
+    pub fn iter_contents_mut(&mut self) -> impl Iterator<Item = &mut Glyph> {
+        self.iter_mut()
+    }
+
     /// Iterate over the glyphs in this layer.
-    pub fn iter_contents(&self) -> impl Iterator<Item = Arc<Glyph>> + '_ {
-        self.glyphs.values().map(Arc::clone)
+    pub fn iter(&self) -> impl Iterator<Item = &Arc<Glyph>> + '_ {
+        self.glyphs.values()
     }
 
     /// Iterate over the glyphs in this layer, mutably.
-    pub fn iter_contents_mut(&mut self) -> impl Iterator<Item = &mut Glyph> {
+    pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut Glyph> {
         self.glyphs.values_mut().map(Arc::make_mut)
     }
 
@@ -444,7 +500,7 @@ mod tests {
             layer.lib.get("com.typemytype.robofont.segmentType").unwrap().as_string().unwrap(),
             "curve"
         );
-        let glyph = layer.get_glyph("A").expect("failed to load glyph 'A'");
+        let glyph = layer.glyph("A").expect("failed to load glyph 'A'");
         assert_eq!(glyph.height, 0.);
         assert_eq!(glyph.width, 1290.);
         assert_eq!(glyph.codepoints, vec!['A']);
@@ -497,7 +553,7 @@ mod tests {
         let layer_path = "testdata/mutatorSans/MutatorSansBoldWide.ufo/glyphs";
         let mut layer = Layer::load(layer_path, DEFAULT_LAYER_NAME.into()).unwrap();
         layer.remove_glyph("A");
-        if let Some(glyph) = layer.get_glyph("A") {
+        if let Some(glyph) = layer.glyph("A") {
             panic!("{:?}", glyph);
         }
 
@@ -513,7 +569,7 @@ mod tests {
         let mut glyph = Glyph::new_named("A");
         glyph.width = 69.;
         layer.insert_glyph(glyph);
-        let glyph = layer.get_glyph("A").expect("failed to load glyph 'A'");
+        let glyph = layer.glyph("A").expect("failed to load glyph 'A'");
         assert_eq!(glyph.width, 69.);
     }
 
@@ -539,7 +595,7 @@ mod tests {
             ufo.layers
                 .get("misc")
                 .unwrap()
-                .iter_contents()
+                .iter()
                 .map(|g| g.name.to_string())
                 .collect::<Vec<String>>(),
             vec!["A".to_string()]

src/lib.rs

@@ -10,7 +10,7 @@
 //! let path = "RoflsExtraDim.ufo";
 //! let mut font_obj = Font::load(path).expect("failed to load font");
 //! let layer = font_obj.default_layer();
-//! let glyph_a = layer.get_glyph("A").expect("missing glyph");
+//! let glyph_a = layer.glyph("A").expect("missing glyph");
 //! assert_eq!(glyph_a.name.as_ref(), "A");
 //! ```
 

src/ufo.rs

@@ -406,7 +406,7 @@ impl Font {
         GlyphName: Borrow<K>,
         K: Ord + ?Sized,
     {
-        self.default_layer().get_glyph(key)
+        self.default_layer().glyph(key)
     }
 
     /// Returns a mutable reference to the glyph with the given name,
@@ -416,7 +416,7 @@ impl Font {
         GlyphName: Borrow<K>,
         K: Ord + ?Sized,
     {
-        self.default_layer_mut().get_glyph_mut(key)
+        self.default_layer_mut().glyph_mut(key)
     }
 
     /// Returns the total number of glyphs in the default layer.

tests/save.rs

@@ -39,8 +39,8 @@ fn save_new_file() {
     assert!(dir.path().join("glyphs/A_.glif").exists());
 
     let loaded = Font::load(dir).unwrap();
-    assert!(loaded.default_layer().get_glyph("A").is_some());
-    let glyph = loaded.default_layer().get_glyph("A").unwrap();
+    assert!(loaded.default_layer().glyph("A").is_some());
+    let glyph = loaded.default_layer().glyph("A").unwrap();
     assert_eq!(glyph.codepoints, vec!['A']);
     let lib_val = glyph.lib.get("my-cool-key").and_then(|val| val.as_unsigned_integer());
     assert_eq!(lib_val, Some(420));
@@ -59,12 +59,12 @@ fn save_fancy() {
     let loaded = Font::load(dir).unwrap();
     let pre_layer = my_ufo.default_layer();
     let post_layer = loaded.default_layer();
-    assert_eq!(pre_layer.iter_contents().count(), post_layer.iter_contents().count());
+    assert_eq!(pre_layer.iter().count(), post_layer.iter().count());
 
-    for glyph in pre_layer.iter_contents() {
-        let other = post_layer.get_glyph(&glyph.name);
+    for glyph in pre_layer.iter() {
+        let other = post_layer.glyph(&glyph.name);
         assert!(other.is_some(), "missing {}", &glyph.name);
-        assert_eq!(&glyph, other.unwrap());
+        assert_eq!(glyph, other.unwrap());
     }
 }