Merge pull request #10 from vrmiguel/better-error-messages

Better error messages

src/cli.rs

@@ -29,7 +29,7 @@ pub enum Flags {
     // Flag -y, --yes supplied
     AlwaysYes,
     // Flag -n, --no supplied
-    AlwaysNo
+    AlwaysNo,
 }
 
 #[derive(PartialEq, Eq, Debug)]
@@ -104,7 +104,7 @@ pub fn parse_matches(matches: clap::ArgMatches<'static>) -> crate::Result<(Comma
         (true, true) => unreachable!(),
         (true, _) => Flags::AlwaysYes,
         (_, true) => Flags::AlwaysNo,
-        (_, _) => Flags::None
+        (_, _) => Flags::None,
     };
 
     Ok((Command::try_from(matches)?, flag))
@@ -179,7 +179,6 @@ impl TryFrom<clap::ArgMatches<'static>> for Command {
                     output: Some(File {
                         path: output_file.into(),
                         contents_in_memory: None,
-                        // extension: output_file_extension.ok(),
                         extension: Some(output_file_extension.unwrap()),
                     }),
                 })

src/compressors/bzip.rs

@@ -36,8 +36,7 @@ impl BzipCompressor {
         let bytes = match file.contents_in_memory {
             Some(bytes) => bytes,
             None => {
-                // TODO: error message,
+                return Err(crate::Error::InternalError);
-                return Err(crate::Error::InvalidInput);
             }
         };
 

src/compressors/tar.rs

@@ -14,8 +14,8 @@ impl TarCompressor {
     // TODO: implement this
     fn make_archive_from_memory(_input: File) -> crate::Result<Vec<u8>> {
         println!(
-            "{}: .tar.tar and .zip.tar is currently unimplemented.",
+            "{} .tar.tar and .zip.tar is currently unimplemented.",
-            "error".red()
+            "[ERROR]".red()
         );
         Err(crate::Error::InvalidZipArchive(""))
     }

src/decompressors/decompressor.rs

@@ -1,9 +1,6 @@
 use std::path::PathBuf;
 
-use crate::{
+use crate::{cli::Flags, file::File};
-    cli::Flags,
-    file::File
-};
 
 pub enum DecompressionResult {
     FilesUnpacked(Vec<PathBuf>),
@@ -11,5 +8,10 @@ pub enum DecompressionResult {
 }
 
 pub trait Decompressor {
-    fn decompress(&self, from: File, into: &Option<File>, flags: Flags) -> crate::Result<DecompressionResult>;
+    fn decompress(
+        &self,
+        from: File,
+        into: &Option<File>,
+        flags: Flags,
+    ) -> crate::Result<DecompressionResult>;
 }

src/decompressors/tar.rs

@@ -24,9 +24,7 @@ impl TarDecompressor {
         let confirm = Confirmation::new("Do you want to overwrite 'FILE'?", Some("FILE"));
 
         let mut archive: Archive<Box<dyn Read>> = match from.contents_in_memory {
-            Some(bytes) => {
+            Some(bytes) => tar::Archive::new(Box::new(Cursor::new(bytes))),
-                tar::Archive::new(Box::new(Cursor::new(bytes)))
-            },
             None => {
                 let file = fs::File::open(&from.path)?;
                 tar::Archive::new(Box::new(file))
@@ -62,7 +60,12 @@ impl TarDecompressor {
 }
 
 impl Decompressor for TarDecompressor {
-    fn decompress(&self, from: File, into: &Option<File>, flags: Flags) -> crate::Result<DecompressionResult> {
+    fn decompress(
+        &self,
+        from: File,
+        into: &Option<File>,
+        flags: Flags,
+    ) -> crate::Result<DecompressionResult> {
         let destination_path = utils::get_destination_path(into);
 
         utils::create_path_if_non_existent(destination_path)?;

src/decompressors/to_memory.rs

@@ -62,19 +62,34 @@ impl DecompressorToMemory {
 }
 
 impl Decompressor for GzipDecompressor {
-    fn decompress(&self, from: File, into: &Option<File>, _: Flags) -> crate::Result<DecompressionResult> {
+    fn decompress(
+        &self,
+        from: File,
+        into: &Option<File>,
+        _: Flags,
+    ) -> crate::Result<DecompressionResult> {
         DecompressorToMemory::decompress(from, CompressionFormat::Gzip, into)
     }
 }
 
 impl Decompressor for BzipDecompressor {
-    fn decompress(&self, from: File, into: &Option<File>, _: Flags) -> crate::Result<DecompressionResult> {
+    fn decompress(
+        &self,
+        from: File,
+        into: &Option<File>,
+        _: Flags,
+    ) -> crate::Result<DecompressionResult> {
         DecompressorToMemory::decompress(from, CompressionFormat::Bzip, into)
     }
 }
 
 impl Decompressor for LzmaDecompressor {
-    fn decompress(&self, from: File, into: &Option<File>, _: Flags) -> crate::Result<DecompressionResult> {
+    fn decompress(
+        &self,
+        from: File,
+        into: &Option<File>,
+        _: Flags,
+    ) -> crate::Result<DecompressionResult> {
         DecompressorToMemory::decompress(from, CompressionFormat::Lzma, into)
     }
 }

src/decompressors/zip.rs

@@ -95,11 +95,7 @@ impl ZipDecompressor {
     }
 
     fn unpack_files(from: File, into: &Path, flags: Flags) -> crate::Result<Vec<PathBuf>> {
-        println!(
+        println!("{} decompressing {:?}", "[OUCH]".bright_blue(), &from.path);
-            "{} decompressing {:?}",
-            "[OUCH]".bright_blue(),
-            &from.path
-        );
 
         match from.contents_in_memory {
             Some(bytes) => {

src/error.rs

@@ -2,7 +2,7 @@ use std::{fmt, path::PathBuf};
 
 use colored::Colorize;
 
-#[derive(PartialEq, Eq, Debug)]
+#[derive(PartialEq, Eq)]
 pub enum Error {
     UnknownExtensionError(String),
     MissingExtensionError(String),
@@ -17,24 +17,57 @@ pub enum Error {
     UnsupportedZipArchive(&'static str),
     InputsMustHaveBeenDecompressible(PathBuf),
     InternalError,
+    CompressingRootFolder,
+    WalkdirError
 }
 
 pub type Result<T> = std::result::Result<T, Error>;
 
+// impl std::error::Error for Error {
+//     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+//         // TODO: get rid of PartialEq and Eq in self::Error in order to
+//         // correctly use `source`.
+//         None
+//     }
+// }
+
+impl fmt::Debug for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self)
+    }
+}
+
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{} ", "[ERROR]".red())?;
         match self {
             Error::MissingExtensionError(filename) => {
+                write!(f, "{} ", "[ERROR]".red())?;
                 write!(f, "cannot compress to \'{}\', likely because it has an unsupported (or missing) extension.", filename)
             }
             Error::InputsMustHaveBeenDecompressible(file) => {
+                write!(f, "{} ", "[ERROR]".red())?;
                 write!(f, "file '{:?}' is not decompressible", file)
             }
+            Error::WalkdirError => {                
+                // Already printed in the From block
+                write!(f, "")
+            }
             Error::FileNotFound(file) => {
+                write!(f, "{} ", "[ERROR]".red())?;
                 // TODO: check if file == ""
                 write!(f, "file {:?} not found!", file)
             }
+            Error::CompressingRootFolder => {
+                write!(f, "{} ", "[ERROR]".red())?;
+                let spacing = "        ";
+                writeln!(f, "It seems you're trying to compress the root folder.")?;
+                writeln!(f, "{}This is unadvisable since ouch does compressions in-memory.", spacing)?;
+                write!(f, "{}Use a more appropriate tool for this, such as {}.", spacing, "rsync".green())
+            }
+            Error::InternalError => {
+                write!(f, "{} ", "[ERROR]".red())?;
+                write!(f, "You've reached an internal error! This really should not have happened.\nPlease file an issue at {}", "https://github.com/vrmiguel/ouch".green())
+            }
             _err => {
                 // TODO
                 write!(f, "")
@@ -50,7 +83,7 @@ impl From<std::io::Error> for Error {
             std::io::ErrorKind::PermissionDenied => Self::PermissionDenied,
             std::io::ErrorKind::AlreadyExists => Self::AlreadyExists,
             _other => {
-                println!("{}: {}", "IO error".red(), err);
+                println!("{} {}", "[IO error]".red(), err);
                 Self::IoError
             }
         }
@@ -71,7 +104,7 @@ impl From<zip::result::ZipError> for Error {
 
 impl From<walkdir::Error> for Error {
     fn from(err: walkdir::Error) -> Self {
-        eprintln!("{}: {}", "error".red(), err);
+        eprintln!("{} {}", "[ERROR]".red(), err);
-        Self::InvalidInput
+        Self::WalkdirError
     }
 }

src/evaluator.rs

@@ -29,10 +29,10 @@ impl Evaluator {
             None => {
                 // This block *should* be unreachable
                 eprintln!(
-                    "{}: reached Evaluator::get_decompressor without known extension.",
+                    "{} reached Evaluator::get_decompressor without known extension.",
-                    "internal error".red()
+                    "[internal error]".red()
                 );
-                return Err(crate::Error::InvalidInput);
+                return Err(crate::Error::InternalError);
             }
         };
 
@@ -71,8 +71,8 @@ impl Evaluator {
             None => {
                 // This block *should* be unreachable
                 eprintln!(
-                    "{}: reached Evaluator::get_decompressor without known extension.",
+                    "{} reached Evaluator::get_decompressor without known extension.",
-                    "internal error".red()
+                    "[internal error]".red()
                 );
                 return Err(crate::Error::InvalidInput);
             }

src/main.rs

@@ -12,7 +12,14 @@ mod utils;
 use error::{Error, Result};
 use evaluator::Evaluator;
 
-fn main() -> crate::Result<()> {
+fn main() {
+    if let Err(err) = run() {
+        println!("{}", err);
+        std::process::exit(127);
+    }
+}
+
+fn run() -> crate::Result<()> {
     let matches = cli::get_matches();
     let (command, flags) = cli::parse_matches(matches)?;
     Evaluator::evaluate(command, flags)

src/utils.rs

@@ -13,11 +13,6 @@ where
 {
     let exists = path.as_ref().exists();
     if !exists {
-        eprintln!(
-            "{}: could not find file {:?}",
-            "[ERROR]".red(),
-            path.as_ref()
-        );
         return Err(crate::Error::FileNotFound(PathBuf::from(path.as_ref())));
     }
     Ok(())
@@ -70,28 +65,18 @@ pub(crate) fn change_dir_and_return_parent(filename: &PathBuf) -> crate::Result<
     let parent = if let Some(parent) = filename.parent() {
         parent
     } else {
-        let spacing = "          ";
+        return Err(crate::Error::CompressingRootFolder);
-        println!(
-            "{} It seems you're trying to compress the root folder.",
-            "[WARNING]".red()
-        );
-        println!(
-            "{}This is unadvisable since ouch does compressions in-memory.",
-            spacing
-        );
-        println!(
-            "{}Use a more appropriate tool for this, such as {}.",
-            spacing,
-            "rsync".green()
-        );
-        return Err(crate::Error::InvalidInput);
     };
 
     env::set_current_dir(parent)?;
     Ok(previous_location)
 }
 
-pub fn permission_for_overwriting(path: &PathBuf, flags: Flags, confirm: &Confirmation) -> crate::Result<bool> {
+pub fn permission_for_overwriting(
+    path: &PathBuf,
+    flags: Flags,
+    confirm: &Confirmation,
+) -> crate::Result<bool> {
     match flags {
         Flags::AlwaysYes => return Ok(true),
         Flags::AlwaysNo => return Ok(false),