From e41d717242a61817619a4eea7a60201bde383352 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Tue, 11 May 2021 09:55:25 -0400 Subject: [PATCH] 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 | 2 +- src/error.rs | 14 +++++++ src/layer.rs | 90 +++++++++++++++++++++++++++++++++++-------- src/lib.rs | 2 +- src/ufo.rs | 4 +- tests/save.rs | 12 +++--- 6 files changed, 97 insertions(+), 27 deletions(-) diff --git a/examples/normalize.rs b/examples/normalize.rs index 40ea8434..697cecb9 100644 --- a/examples/normalize.rs +++ b/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.") diff --git a/src/error.rs b/src/error.rs index fbed614a..4ca456d0 100644 --- a/src/error.rs +++ b/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 }) => { diff --git a/src/layer.rs b/src/layer.rs index a599640c..3ca3a229 100644 --- a/src/layer.rs +++ b/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) -> 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(&self, glyph: &K) -> Option<&Arc> + pub fn glyph(&self, glyph: &K) -> Option<&Arc> where GlyphName: Borrow, 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(&mut self, glyph: &K) -> Option<&mut Glyph> + where + GlyphName: Borrow, + 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(&self, glyph: &K) -> Option<&Arc> + where + GlyphName: Borrow, + K: Ord + ?Sized, + { + self.glyph(glyph) + } + + #[doc(hidden)] + #[deprecated(since = "0.4.0", note = "renamed to 'glyph_mut'")] pub fn get_glyph_mut(&mut self, glyph: &K) -> Option<&mut Glyph> where GlyphName: Borrow, 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> { 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> + '_ { + self.iter() + } + + #[doc(hidden)] + #[deprecated(since = "0.4.0", note = "renamed to 'iter_mut'")] + pub fn iter_contents_mut(&mut self) -> impl Iterator { + self.iter_mut() + } + /// Iterate over the glyphs in this layer. - pub fn iter_contents(&self) -> impl Iterator> + '_ { - self.glyphs.values().map(Arc::clone) + pub fn iter(&self) -> impl Iterator> + '_ { + self.glyphs.values() } /// Iterate over the glyphs in this layer, mutably. - pub fn iter_contents_mut(&mut self) -> impl Iterator { + pub fn iter_mut(&mut self) -> impl Iterator { 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!["A".to_string()] diff --git a/src/lib.rs b/src/lib.rs index aba5ee14..ced8a3f6 100644 --- a/src/lib.rs +++ b/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"); //! ``` diff --git a/src/ufo.rs b/src/ufo.rs index 2dd06b89..d7e47b82 100644 --- a/src/ufo.rs +++ b/src/ufo.rs @@ -406,7 +406,7 @@ impl Font { GlyphName: Borrow, 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: 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. diff --git a/tests/save.rs b/tests/save.rs index d84f3e4c..14e76338 100644 --- a/tests/save.rs +++ b/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()); } }