Allow specifying whitespace on save

This lets the user optionally customize the whitespace used during
serialization.

The implementation is slightly annoying. the `quick_xml` and `rust-plist`
crates specify whitespace in different ways; `quick_xml` takes a (u8,
usize) pair (the byte and its count) and `rust-plist` takes a
`Cow<'static str>`. Additionally, in our own glyph serialization code we
need to write whitespace directly when appending the 'lib' section.

Anyway: you *create* a `WriteOptions` object by passing it a string, and
then we figure out what the appropriate (u8, usize) pair is. The reason
we want to think about this is because eventually writing out a UFO
should be parallelized, and that could mean cloning `WriteOptions` once
for each glyph; we would like to make sure this isn't allocating.

So this seems to work, even if some of the internals are a bit gross.

Cargo.toml

@@ -18,7 +18,9 @@ exclude = [
 features = ["kurbo"]
 
 [dependencies]
-plist = "1.2"
+# we need the enabled_fancy_long_feature feature until
+# https://github.com/ebarnard/rust-plist/pull/69 lands/is released
+plist = { version =  "~1.2", features = ["serde", "enable_unstable_features_that_may_break_with_minor_version_bumps"] }
 uuid = { version = "0.8", features = ["v4"] }
 serde = { version =  "1.0", features = ["rc"] }
 serde_derive = "1.0"

src/font.rs

@@ -16,6 +16,7 @@ use crate::layer::{Layer, LayerSet, LAYER_CONTENTS_FILE};
 use crate::names::NameList;
 use crate::shared_types::{Plist, PUBLIC_OBJECT_LIBS_KEY};
 use crate::upconversion;
+use crate::write::{self, WriteOptions};
 use crate::DataRequest;
 use crate::Error;
 
@@ -218,10 +219,20 @@ impl Font {
     /// `public.objectLibs` key, as object lib management is done automatically.
     pub fn save(&self, path: impl AsRef<Path>) -> Result<(), Error> {
         let path = path.as_ref();
-        self.save_impl(path)
+        self.save_impl(path, &Default::default())
     }
 
-    fn save_impl(&self, path: &Path) -> Result<(), Error> {
+    /// Attempt to save the UFO, using the provided [`WriteOptions`].
+    pub fn save_with_options(
+        &self,
+        path: impl AsRef<Path>,
+        options: &WriteOptions,
+    ) -> Result<(), Error> {
+        let path = path.as_ref();
+        self.save_impl(path, options)
+    }
+
+    fn save_impl(&self, path: &Path, options: &WriteOptions) -> Result<(), Error> {
         if self.meta.format_version != FormatVersion::V3 {
             return Err(Error::DowngradeUnsupported);
         }
@@ -238,13 +249,17 @@ impl Font {
         // we want to always set ourselves as the creator when serializing,
         // but we also don't have mutable access to self.
         if self.meta.creator == DEFAULT_METAINFO_CREATOR {
-            plist::to_file_xml(path.join(METAINFO_FILE), &self.meta)?;
+            write::write_xml_to_file(&path.join(METAINFO_FILE), &self.meta, options.xml_options())?;
         } else {
-            plist::to_file_xml(path.join(METAINFO_FILE), &MetaInfo::default())?;
+            write::write_xml_to_file(
+                &path.join(METAINFO_FILE),
+                &MetaInfo::default(),
+                options.xml_options(),
+            )?;
         }
 
         if let Some(font_info) = self.font_info.as_ref() {
-            plist::to_file_xml(path.join(FONTINFO_FILE), &font_info)?;
+            write::write_xml_to_file(&path.join(FONTINFO_FILE), font_info, options.xml_options())?;
         }
 
         // Object libs are treated specially. The UFO v3 format won't allow us
@@ -262,17 +277,25 @@ impl Font {
         }
         if !lib.is_empty() {
             crate::util::recursive_sort_plist_keys(&mut lib);
-            plist::Value::Dictionary(lib).to_file_xml(path.join(LIB_FILE))?;
+            write::write_plist_value_to_file(
+                &path.join(LIB_FILE),
+                &lib.into(),
+                options.xml_options(),
+            )?;
         }
 
         if let Some(groups) = self.groups.as_ref() {
             validate_groups(groups).map_err(Error::InvalidGroups)?;
-            plist::to_file_xml(path.join(GROUPS_FILE), groups)?;
+            write::write_xml_to_file(&path.join(GROUPS_FILE), groups, options.xml_options())?;
         }
 
         if let Some(kerning) = self.kerning.as_ref() {
             let kerning_serializer = crate::kerning::KerningSerializer { kerning };
-            plist::to_file_xml(path.join(KERNING_FILE), &kerning_serializer)?;
+            write::write_xml_to_file(
+                &path.join(KERNING_FILE),
+                &kerning_serializer,
+                options.xml_options(),
+            )?;
         }
 
         if let Some(features) = self.features.as_ref() {
@@ -281,11 +304,15 @@ impl Font {
 
         let contents: Vec<(&str, &PathBuf)> =
             self.layers.iter().map(|l| (l.name.as_ref(), &l.path)).collect();
-        plist::to_file_xml(path.join(LAYER_CONTENTS_FILE), &contents)?;
+        write::write_xml_to_file(
+            &path.join(LAYER_CONTENTS_FILE),
+            &contents,
+            options.xml_options(),
+        )?;
 
         for layer in self.layers.iter() {
             let layer_path = path.join(&layer.path);
-            layer.save(layer_path)?;
+            layer.save_with_options(&layer_path, options)?;
         }
 
         Ok(())

src/glyph/mod.rs

@@ -15,7 +15,7 @@ use druid::{Data, Lens};
 use crate::error::{Error, ErrorKind, GlifError, GlifErrorInternal};
 use crate::names::NameList;
 use crate::shared_types::PUBLIC_OBJECT_LIBS_KEY;
-use crate::{Color, Guideline, Identifier, Line, Plist};
+use crate::{Color, Guideline, Identifier, Line, Plist, WriteOptions};
 
 /// The name of a glyph.
 pub type GlyphName = Arc<str>;
@@ -64,13 +64,20 @@ impl Glyph {
 
     #[doc(hidden)]
     pub fn save<P: AsRef<Path>>(&self, path: P) -> Result<(), Error> {
+        let path = path.as_ref();
+        let opts = WriteOptions::default();
+        self.save_with_options(path, &opts)
+    }
+
+    pub(crate) fn save_with_options(&self, path: &Path, opts: &WriteOptions) -> Result<(), Error> {
         if self.format != GlifVersion::V2 {
             return Err(Error::DowngradeUnsupported);
         }
         if self.lib.contains_key(PUBLIC_OBJECT_LIBS_KEY) {
             return Err(Error::PreexistingPublicObjectLibsKey);
         }
-        let data = self.encode_xml()?;
+
+        let data = self.encode_xml_with_options(opts)?;
         std::fs::write(path, &data)?;
         Ok(())
     }

src/glyph/serialize.rs

@@ -10,7 +10,7 @@ use quick_xml::{
 use super::PUBLIC_OBJECT_LIBS_KEY;
 use crate::{
     util, AffineTransform, Anchor, Color, Component, Contour, ContourPoint, GlifVersion, Glyph,
-    Guideline, Image, Line, Plist, PointType,
+    Guideline, Image, Line, Plist, PointType, WriteOptions,
 };
 
 use crate::error::{GlifWriteError, WriteError};
@@ -22,11 +22,21 @@ impl Glyph {
     ///
     /// [ufonormalizer]: https://github.com/unified-font-object/ufoNormalizer/
     pub fn encode_xml(&self) -> Result<Vec<u8>, GlifWriteError> {
-        self.encode_xml_impl().map_err(|inner| GlifWriteError { name: self.name.clone(), inner })
+        let options = WriteOptions::default();
+        self.encode_xml_with_options(&options)
     }
 
-    fn encode_xml_impl(&self) -> Result<Vec<u8>, WriteError> {
-        let mut writer = Writer::new_with_indent(Cursor::new(Vec::new()), b'\t', 1);
+    pub fn encode_xml_with_options(&self, opts: &WriteOptions) -> Result<Vec<u8>, GlifWriteError> {
+        self.encode_xml_impl(opts)
+            .map_err(|inner| GlifWriteError { name: self.name.clone(), inner })
+    }
+
+    fn encode_xml_impl(&self, options: &WriteOptions) -> Result<Vec<u8>, WriteError> {
+        let mut writer = Writer::new_with_indent(
+            Cursor::new(Vec::new()),
+            options.whitespace_char,
+            options.whitespace_count,
+        );
         writer.write_event(Event::Decl(BytesDecl::new(b"1.0", Some(b"UTF-8"), None)))?;
         let mut start = BytesStart::borrowed_name(b"glyph");
         start.push_attribute(("name", &*self.name));
@@ -87,7 +97,7 @@ impl Glyph {
 
         if !lib.is_empty() {
             util::recursive_sort_plist_keys(&mut lib);
-            write_lib_section(&lib, &mut writer)?;
+            write_lib_section(lib, &mut writer, options)?;
         }
 
         if let Some(ref note) = self.note {
@@ -111,13 +121,14 @@ impl Glyph {
 /// such as the xml declaration and the <plist> tag.
 ///
 /// We then take this and write it into the middle of our active write session.
-///
-/// By a lovely coincidence the whitespace is the same in both places; if this
-/// changes we will need to do custom whitespace handling.
-fn write_lib_section<T: Write>(lib: &Plist, writer: &mut Writer<T>) -> Result<(), WriteError> {
-    let as_value: plist::Value = lib.to_owned().into();
+fn write_lib_section<T: Write>(
+    lib: Plist,
+    writer: &mut Writer<T>,
+    options: &WriteOptions,
+) -> Result<(), WriteError> {
+    let as_value: plist::Value = lib.into();
     let mut out_buffer = Vec::with_capacity(256); // a reasonable min size?
-    as_value.to_writer_xml(&mut out_buffer)?;
+    as_value.to_writer_xml_with_options(&mut out_buffer, options.xml_options())?;
     let lib_xml = String::from_utf8(out_buffer).expect("XML writer wrote invalid UTF-8");
     let header = "<plist version=\"1.0\">\n";
     let footer = "\n</plist>";
@@ -130,7 +141,9 @@ fn write_lib_section<T: Write>(lib: &Plist, writer: &mut Writer<T>) -> Result<()
 
     writer.write_event(Event::Start(BytesStart::borrowed_name(b"lib")))?;
     for line in to_write.lines() {
-        writer.inner().write_all("\n\t\t".as_bytes())?;
+        writer.inner().write_all("\n".as_bytes())?;
+        writer.inner().write_all(options.indent_str.as_bytes())?;
+        writer.inner().write_all(options.indent_str.as_bytes())?;
         writer.inner().write_all(line.as_bytes())?;
     }
     writer.write_event(Event::End(BytesEnd::borrowed(b"lib")))?;

src/glyph/tests.rs

@@ -34,6 +34,66 @@ fn serialize_full_glyph() {
     pretty_assertions::assert_eq!(glif, source);
 }
 
+#[test]
+fn serialize_with_default_formatting() {
+    let data = include_str!("../../testdata/small_lib.glif");
+    let glyph = parse_glyph(data.as_bytes()).unwrap();
+    let one_tab = glyph.encode_xml().unwrap();
+    let one_tab = std::str::from_utf8(&one_tab).unwrap();
+    pretty_assertions::assert_eq!(
+        one_tab,
+        r#"<?xml version="1.0" encoding="UTF-8"?>
+<glyph name="hello" format="2">
+	<advance width="1200"/>
+	<outline>
+		<contour>
+			<point x="2" y="30" type="line"/>
+			<point x="44" y="10" type="line"/>
+		</contour>
+	</outline>
+	<lib>
+		<dict>
+			<key>test.key</key>
+			<string>I am a creative professional :)</string>
+		</dict>
+	</lib>
+	<note>durp</note>
+</glyph>
+"#
+    );
+}
+
+#[test]
+fn serialize_with_custom_whitespace() {
+    let data = include_str!("../../testdata/small_lib.glif");
+    let glyph = parse_glyph(data.as_bytes()).unwrap();
+    let options = WriteOptions::default().whitespace("  ");
+    let two_spaces = glyph.encode_xml_with_options(&options).unwrap();
+    let two_spaces = std::str::from_utf8(&two_spaces).unwrap();
+
+    pretty_assertions::assert_eq!(
+        two_spaces,
+        r#"<?xml version="1.0" encoding="UTF-8"?>
+<glyph name="hello" format="2">
+  <advance width="1200"/>
+  <outline>
+    <contour>
+      <point x="2" y="30" type="line"/>
+      <point x="44" y="10" type="line"/>
+    </contour>
+  </outline>
+  <lib>
+    <dict>
+      <key>test.key</key>
+      <string>I am a creative professional :)</string>
+    </dict>
+  </lib>
+  <note>durp</note>
+</glyph>
+"#
+    );
+}
+
 #[test]
 fn parse() {
     let bytes = include_bytes!("../../testdata/sample_period.glif");

src/layer.rs

@@ -11,7 +11,7 @@ use rayon::prelude::*;
 use crate::glyph::GlyphName;
 use crate::names::NameList;
 use crate::shared_types::Color;
-use crate::{util, Error, Glyph, Plist};
+use crate::{util, Error, Glyph, Plist, WriteOptions};
 
 static CONTENTS_FILE: &str = "contents.plist";
 static LAYER_INFO_FILE: &str = "layerinfo.plist";
@@ -304,7 +304,15 @@ impl Layer {
         Ok((color, lib))
     }
 
-    fn layerinfo_to_file(&self, path: &Path) -> Result<(), Error> {
+    fn layerinfo_to_file_if_needed(
+        &self,
+        path: &Path,
+        options: &WriteOptions,
+    ) -> Result<(), Error> {
+        if self.color.is_none() && self.lib.is_empty() {
+            return Ok(());
+        }
+
         let mut dict = plist::dictionary::Dictionary::new();
 
         if let Some(c) = &self.color {
@@ -316,25 +324,34 @@ impl Layer {
 
         util::recursive_sort_plist_keys(&mut dict);
 
-        plist::Value::Dictionary(dict).to_file_xml(path.join(LAYER_INFO_FILE))?;
-
-        Ok(())
-    }
-
-    fn layerinfo_is_empty(&self) -> bool {
-        self.color.is_none() && self.lib.is_empty()
+        crate::write::write_plist_value_to_file(
+            &path.join(LAYER_INFO_FILE),
+            &dict.into(),
+            options.xml_options(),
+        )
     }
 
     /// Attempt to write this layer to the given path.
     ///
     /// The path should not exist.
     pub fn save(&self, path: impl AsRef<Path>) -> Result<(), Error> {
-        let path = path.as_ref();
+        let options = WriteOptions::default();
+        self.save_with_options(path.as_ref(), &options)
+    }
+
+    pub fn save_with_options(&self, path: &Path, opts: &WriteOptions) -> Result<(), Error> {
         fs::create_dir(&path)?;
-        plist::to_file_xml(path.join(CONTENTS_FILE), &self.contents)?;
-        // Avoid writing empty layerinfo.plist file.
-        if !self.layerinfo_is_empty() {
-            self.layerinfo_to_file(path)?;
+        crate::write::write_xml_to_file(
+            &path.join(CONTENTS_FILE),
+            &self.contents,
+            opts.xml_options(),
+        )?;
+
+        self.layerinfo_to_file_if_needed(path, opts)?;
+
+        for (name, glyph_path) in self.contents.iter() {
+            let glyph = self.glyphs.get(name).expect("all glyphs in contents must exist.");
+            glyph.save_with_options(&path.join(glyph_path), opts)?;
         }
 
         #[cfg(feature = "rayon")]

src/lib.rs

@@ -37,6 +37,7 @@ mod names;
 mod shared_types;
 mod upconversion;
 pub mod util;
+mod write;
 
 pub use data_request::DataRequest;
 pub use error::Error;
@@ -50,6 +51,7 @@ pub use guideline::{Guideline, Line};
 pub use identifier::Identifier;
 pub use layer::{Layer, LayerSet};
 pub use shared_types::{Color, IntegerOrFloat, NonNegativeIntegerOrFloat, Plist};
+pub use write::WriteOptions;
 
 #[allow(deprecated)]
 pub use font::Ufo;