Error tweaks

examples/load_save.rs

@@ -9,7 +9,7 @@ use norad::Font;
 
 static HELP: &str = "
 USAGE:
-    open_ufo PATH [OUTPATH]
+    load_save PATH [OUTPATH]
 
 If an OUTPATH is provided, the UFO will be saved after opening.
 ";

src/error.rs

@@ -39,16 +39,45 @@ pub enum Error {
         /// The glyph name.
         glyph: String,
     },
-    /// An error returned when there is an input/output problem during processing
-    IoError(IoError),
-    /// An error returned when there is an XML parsing problem.
-    ParseError(XmlError),
-    /// An error that wraps a [GlifError].
-    Glif(GlifError),
-    /// An error that wraps a [GlifWriteError].
+    /// An error returned when there is an input problem during processing
+    UfoLoad {
+        /// The path of the relevant file.
+        path: PathBuf,
+        /// The underlying error.
+        inner: IoError,
+    },
+    /// An error returned when there is an output problem during processing
+    UfoWrite {
+        /// The path of the relevant file.
+        path: PathBuf,
+        /// The underlying error.
+        inner: IoError,
+    },
+    /// A `.glif` file could not be loaded.
+    GlifLoad {
+        /// The path of the relevant `.glif` file.
+        path: PathBuf,
+        /// The underlying error.
+        inner: GlifLoadError,
+    },
+    /// An error that occurs when attempting to write a [`Glyph`] to disk.
+    ///
+    /// [`Glyph`]: crate::Glyph
     GlifWrite(GlifWriteError),
-    /// An error that wraps a [PlistError].
-    PlistError(PlistError),
+    /// A plist file could not be read.
+    PlistLoad {
+        /// The path of the relevant file.
+        path: PathBuf,
+        /// The underlying error.
+        error: PlistError,
+    },
+    /// A plist file could not be written.
+    PlistWrite {
+        /// The path of the relevant file.
+        path: PathBuf,
+        /// The underlying error.
+        error: PlistError,
+    },
     /// An error returned when there is invalid fontinfo.plist data.
     InvalidFontInfo,
     /// An error returned when there is a problem during fontinfo.plist version up-conversion.
@@ -79,6 +108,17 @@ pub enum Error {
     InvalidStoreEntry(PathBuf, StoreError),
 }
 
+/// An error that occurs while attempting to read a .glif file from disk.
+#[derive(Debug)]
+pub enum GlifLoadError {
+    /// An [`std::io::Error`].
+    Io(IoError),
+    /// A [`quick_xml::Error`].
+    Xml(XmlError),
+    /// The .glif file was malformed.
+    Parse(ErrorKind),
+}
+
 /// An error representing a failure to insert content into a [`crate::datastore::Store`].
 #[derive(Clone, Debug)]
 #[non_exhaustive]
@@ -130,19 +170,7 @@ impl InvalidColorString {
     }
 }
 
-/// An error representing a failure during .glif file parsing.
-#[derive(Debug)]
-pub struct GlifError {
-    /// The glif file path.
-    pub path: Option<PathBuf>,
-    /// The buffer position.
-    pub position: usize,
-    /// The kind of error.
-    pub kind: ErrorKind,
-}
-
-/// An error representing a failure during .glif file serialization. This
-/// error wraps [GlyphName] and [WriteError] types.
+/// An error when attempting to write a .glif file.
 #[derive(Debug)]
 pub struct GlifWriteError {
     /// The name of the glif where the error occured.
@@ -163,22 +191,12 @@ pub enum WriteError {
     /// If for some reason the implementation of that crate changes, we could
     /// be affected, although this is very unlikely.
     InternalLibWriteError,
-    /// Generic serialization error.  Wraps an [IoError].
-    IoError(IoError),
+    /// An error originating in [`std::io`].
+    Io(IoError),
     /// Plist serialization error. Wraps a [PlistError].
     Plist(PlistError),
 }
 
-/// Errors that happen when parsing `glif` files. This is converted into either
-/// `Error::ParseError` or `Error::Glif` at the parse boundary.
-#[derive(Debug)]
-pub(crate) enum GlifErrorInternal {
-    /// A problem with the xml data.
-    Xml(XmlError),
-    /// A violation of the ufo spec.
-    Spec { kind: ErrorKind, position: usize },
-}
-
 /// The reason for a glif parse failure.
 #[derive(Debug, Clone, Copy)]
 pub enum ErrorKind {
@@ -269,16 +287,25 @@ impl std::fmt::Display for Error {
             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::InvalidColor(e) => e.fmt(f),
-            Error::Glif(GlifError { path, position, kind }) => {
-                write!(f, "Glif error in {:?} index {}: '{}", path, position, kind)
+            Error::UfoLoad { path, .. } => {
+                write!(f, "Failed to read file or directory '{}'", path.display())
             }
-            Error::GlifWrite(GlifWriteError { name, inner }) => {
-                write!(f, "Failed to save glyph {}, error: '{}'", name, inner)
+            Error::UfoWrite { path, .. } => {
+                write!(f, "Failed to write file or directory '{}'", path.display())
+            }
+            Error::GlifLoad { path, .. } => {
+                write!(f, "Failed to read glyph file from '{}'", path.display())
+            }
+            Error::GlifWrite(GlifWriteError { name, .. }) => {
+                write!(f, "Failed to write out glyph '{}'", name)
+            }
+            Error::PlistLoad { path, .. } => {
+                write!(f, "Failed to read Plist file from '{}'", path.display())
+            }
+            Error::PlistWrite { path, .. } => {
+                write!(f, "Failed to write Plist file to '{}'", path.display())
             }
-            Error::PlistError(e) => e.fmt(f),
             Error::InvalidFontInfo => write!(f, "FontInfo contains invalid data"),
             Error::FontInfoUpconversion => {
                 write!(f, "FontInfo contains invalid data after upconversion")
@@ -300,15 +327,25 @@ impl std::fmt::Display for Error {
             Error::MissingUfoDir(path) => {
                 write!(f, "{} directory was not found", path)
             }
-            Error::InvalidStoreEntry(path, e) => {
-                write!(f, "Store entry '{}' error: {}", path.display(), e)
+            Error::InvalidStoreEntry(path, _) => {
+                write!(f, "Store entry '{}' is invalid", path.display())
             }
             #[cfg(feature = "kurbo")]
             Error::ConvertContour(cause) => write!(f, "Failed to convert contour: '{}'", cause),
         }
     }
 }
 
+impl std::fmt::Display for GlifLoadError {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        match self {
+            GlifLoadError::Xml(_) => write!(f, "Failed to read or parse XML structure"),
+            GlifLoadError::Io(_) => write!(f, "Failed to read file"),
+            GlifLoadError::Parse(e) => write!(f, "Failed to parse glyph data: {}", e),
+        }
+    }
+}
+
 impl std::fmt::Display for StoreError {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         use StoreError::*;
@@ -325,13 +362,22 @@ impl std::fmt::Display for StoreError {
             NotPlainFile => write!(f, "Only plain files are allowed, no symlinks."),
             Subdir => write!(f, "Subdirectories are not allowed in the image store."),
             InvalidImage => write!(f, "An image must be a valid PNG."),
-            Io(e) => {
-                write!(f, "Encountered an IO error while trying to load content: {}.", e)
+            Io(_) => {
+                write!(f, "Encountered an IO error while trying to load content")
             }
         }
     }
 }
 
+impl std::error::Error for StoreError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match self {
+            StoreError::Io(e) => Some(e),
+            _ => None,
+        }
+    }
+}
+
 impl std::fmt::Display for GroupsValidationError {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         match self {
@@ -396,9 +442,9 @@ impl std::fmt::Display for ErrorKind {
 impl std::fmt::Display for WriteError {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         match self {
-            WriteError::IoError(err) => err.fmt(f),
-            WriteError::Xml(err) => err.fmt(f),
-            WriteError::Plist(err) => err.fmt(f),
+            WriteError::Io(_) => write!(f, "Error writing to disk"),
+            WriteError::Xml(_) => write!(f, "Error writing an XML file to disk"),
+            WriteError::Plist(_) => write!(f, "Error writing a Plist file to disk"),
             WriteError::InternalLibWriteError => {
                 write!(f, "Internal error while writing lib data. Please open an issue.")
             }
@@ -408,14 +454,14 @@ impl std::fmt::Display for WriteError {
 
 impl std::fmt::Display for GlifWriteError {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        write!(f, "Failed to write glyph '{}': {}", self.name, self.inner)
+        write!(f, "Failed to write glyph '{}'", self.name)
     }
 }
 
 impl std::error::Error for WriteError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match self {
-            WriteError::IoError(inner) => Some(inner),
+            WriteError::Io(inner) => Some(inner),
             WriteError::Xml(inner) => Some(inner),
             WriteError::Plist(inner) => Some(inner),
             WriteError::InternalLibWriteError => None,
@@ -429,12 +475,26 @@ impl std::error::Error for GlifWriteError {
     }
 }
 
+impl std::error::Error for GlifLoadError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match self {
+            GlifLoadError::Io(e) => Some(e),
+            GlifLoadError::Xml(e) => Some(e),
+            GlifLoadError::Parse(_) => None,
+        }
+    }
+}
+
 impl std::error::Error for Error {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match self {
-            Error::IoError(inner) => Some(inner),
-            Error::PlistError(inner) => Some(inner),
+            Error::GlifLoad { inner, .. } => Some(inner),
             Error::GlifWrite(inner) => Some(&inner.inner),
+            Error::PlistLoad { error, .. } => Some(error),
+            Error::PlistWrite { error, .. } => Some(error),
+            Error::UfoLoad { inner, .. } => Some(inner),
+            Error::UfoWrite { inner, .. } => Some(inner),
+            Error::InvalidStoreEntry(_, e) => Some(e),
             _ => None,
         }
     }
@@ -448,12 +508,6 @@ impl std::fmt::Display for InvalidColorString {
 
 impl std::error::Error for InvalidColorString {}
 
-impl ErrorKind {
-    pub(crate) fn to_error(self, position: usize) -> GlifErrorInternal {
-        GlifErrorInternal::Spec { kind: self, position }
-    }
-}
-
 #[doc(hidden)]
 impl From<InvalidColorString> for Error {
     fn from(src: InvalidColorString) -> Error {
@@ -468,48 +522,6 @@ impl From<GlifWriteError> for Error {
     }
 }
 
-#[doc(hidden)]
-impl From<(ErrorKind, usize)> for GlifErrorInternal {
-    fn from(src: (ErrorKind, usize)) -> GlifErrorInternal {
-        GlifErrorInternal::Spec { kind: src.0, position: src.1 }
-    }
-}
-
-#[doc(hidden)]
-impl From<XmlError> for Error {
-    fn from(src: XmlError) -> Error {
-        Error::ParseError(src)
-    }
-}
-
-#[doc(hidden)]
-impl From<PlistError> for Error {
-    fn from(src: PlistError) -> Error {
-        Error::PlistError(src)
-    }
-}
-
-#[doc(hidden)]
-impl From<IoError> for Error {
-    fn from(src: IoError) -> Error {
-        Error::IoError(src)
-    }
-}
-
-#[doc(hidden)]
-impl From<GlifError> for Error {
-    fn from(src: GlifError) -> Error {
-        Error::Glif(src)
-    }
-}
-
-#[doc(hidden)]
-impl From<XmlError> for GlifErrorInternal {
-    fn from(src: XmlError) -> GlifErrorInternal {
-        GlifErrorInternal::Xml(src)
-    }
-}
-
 #[doc(hidden)]
 impl From<XmlError> for WriteError {
     fn from(src: XmlError) -> WriteError {
@@ -520,7 +532,7 @@ impl From<XmlError> for WriteError {
 #[doc(hidden)]
 impl From<IoError> for WriteError {
     fn from(src: IoError) -> WriteError {
-        WriteError::IoError(src)
+        WriteError::Io(src)
     }
 }