Merge pull request #97 from SpyrosRoum/Improve-FinalError

Change FinalError builder pattern to take and give ownership of self
ouch-org · Oct 15, 2021 · 10af868 · 10af868
2 parents 1bcdfeb + d33dd7f
commit 10af868
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 45 deletions.
diff --git a/rust-toolchain b/rust-toolchain
@@ -0,0 +1 @@
+nightly
diff --git a/rustfmt.toml b/rustfmt.toml
@@ -6,3 +6,5 @@ reorder_imports = true
 reorder_modules = true
 use_try_shorthand = true
 use_small_heuristics = "Max"
+unstable_features = true
+force_multiline_blocks = true
diff --git a/src/commands.rs b/src/commands.rs
@@ -50,8 +50,7 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> {
                     .hint("")
                     .hint("Examples:")
                     .hint(format!("  ouch compress ... {}.tar.gz", to_utf(&output_path)))
-                    .hint(format!("  ouch compress ... {}.zip", to_utf(&output_path)))
-                    .clone();
+                    .hint(format!("  ouch compress ... {}.zip", to_utf(&output_path)));
 
                 return Err(Error::with_reason(reason));
             }
@@ -79,8 +78,7 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> {
                     .detail("The only supported formats that archive files into an archive are .tar and .zip.")
                     .hint(format!("Try inserting '.tar' or '.zip' before '{}'.", &formats[0]))
                     .hint(format!("From: {}", output_path))
-                    .hint(format!(" To : {}", suggested_output_path))
-                    .clone();
+                    .hint(format!(" To : {}", suggested_output_path));
 
                 return Err(Error::with_reason(reason));
             }
@@ -90,8 +88,7 @@ pub fn run(command: Command, flags: &oof::Flags) -> crate::Result<()> {
                     .detail(format!("Found the format '{}' in an incorrect position.", format))
                     .detail(format!("'{}' can only be used at the start of the file extension.", format))
                     .hint(format!("If you wish to compress multiple files, start the extension with '{}'.", format))
-                    .hint(format!("Otherwise, remove the last '{}' from '{}'.", format, to_utf(&output_path)))
-                    .clone();
+                    .hint(format!("Otherwise, remove the last '{}' from '{}'.", format, to_utf(&output_path)));
 
                 return Err(Error::with_reason(reason));
             }

diff --git a/src/error.rs b/src/error.rs
@@ -70,12 +70,12 @@ impl FinalError {
         Self { title: title.to_string(), details: vec![], hints: vec![] }
     }
 
-    pub fn detail(&mut self, detail: impl ToString) -> &mut Self {
+    pub fn detail(mut self, detail: impl ToString) -> Self {
         self.details.push(detail.to_string());
         self
     }
 
-    pub fn hint(&mut self, hint: impl ToString) -> &mut Self {
+    pub fn hint(mut self, hint: impl ToString) -> Self {
         self.hints.push(hint.to_string());
         self
     }
@@ -85,70 +85,54 @@ impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let err = match self {
             Error::MissingExtensionError(filename) => {
-                let error = FinalError::with_title(format!("Cannot compress to {:?}", filename))
+                FinalError::with_title(format!("Cannot compress to {:?}", filename))
                     .detail("Ouch could not detect the compression format")
                     .hint("Use a supported format extension, like '.zip' or '.tar.gz'")
                     .hint("Check https://github.com/vrmiguel/ouch for a full list of supported formats")
-                    .clone();
-
-                error
             }
             Error::WalkdirError { reason } => FinalError::with_title(reason),
             Error::FileNotFound(file) => {
-                let error = if file == Path::new("") {
+                if file == Path::new("") {
                     FinalError::with_title("file not found!")
                 } else {
                     FinalError::with_title(format!("file {:?} not found!", file))
-                };
-
-                error
+                }
             }
             Error::CompressingRootFolder => {
-                let error = FinalError::with_title("It seems you're trying to compress the root folder.")
+                FinalError::with_title("It seems you're trying to compress the root folder.")
                     .detail("This is unadvisable since ouch does compressions in-memory.")
                     .hint("Use a more appropriate tool for this, such as rsync.")
-                    .clone();
-
-                error
             }
             Error::MissingArgumentsForCompression => {
-                let error = FinalError::with_title("Could not compress")
+                FinalError::with_title("Could not compress")
                     .detail("The compress command requires at least 2 arguments")
                     .hint("You must provide:")
                     .hint("  - At least one input argument.")
                     .hint("  - The output argument.")
                     .hint("")
                     .hint("Example: `ouch compress image.png img.zip`")
-                    .clone();
-
-                error
             }
             Error::MissingArgumentsForDecompression => {
-                let error = FinalError::with_title("Could not decompress")
+                FinalError::with_title("Could not decompress")
                     .detail("The compress command requires at least one argument")
                     .hint("You must provide:")
                     .hint("  - At least one input argument.")
                     .hint("")
                     .hint("Example: `ouch decompress imgs.tar.gz`")
-                    .clone();
-
-                error
             }
             Error::InternalError => {
-                let error = FinalError::with_title("InternalError :(")
+                FinalError::with_title("InternalError :(")
                     .detail("This should not have happened")
                     .detail("It's probably our fault")
                     .detail("Please help us improve by reporting the issue at:")
                     .detail(format!("    {}https://github.com/vrmiguel/ouch/issues ", *CYAN))
-                    .clone();
-
-                error
             }
             Error::OofError(err) => FinalError::with_title(err),
             Error::IoError { reason } => FinalError::with_title(reason),
-            Error::CompressionTypo => FinalError::with_title("Possible typo detected")
-                .hint(format!("Did you mean '{}ouch compress{}'?", *MAGENTA, *RESET))
-                .clone(),
+            Error::CompressionTypo => {
+                FinalError::with_title("Possible typo detected")
+                    .hint(format!("Did you mean '{}ouch compress{}'?", *MAGENTA, *RESET))
+            }
             Error::UnknownExtensionError(_) => todo!(),
             Error::AlreadyExists => todo!(),
             Error::InvalidZipArchive(_) => todo!(),

diff --git a/src/oof/error.rs b/src/oof/error.rs
@@ -31,18 +31,22 @@ impl fmt::Display for OofError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         // TODO: implement proper debug messages
         match self {
-            OofError::FlagValueConflict { flag, previous_value, new_value } => write!(
-                f,
-                "CLI flag value conflicted for flag '--{}', previous: {:?}, new: {:?}.",
-                flag.long, previous_value, new_value
-            ),
+            OofError::FlagValueConflict { flag, previous_value, new_value } => {
+                write!(
+                    f,
+                    "CLI flag value conflicted for flag '--{}', previous: {:?}, new: {:?}.",
+                    flag.long, previous_value, new_value
+                )
+            }
             OofError::InvalidUnicode(flag) => write!(f, "{:?} is not valid Unicode.", flag),
             OofError::UnknownShortFlag(ch) => write!(f, "Unknown argument '-{}'", ch),
-            OofError::MisplacedShortArgFlagError(ch) => write!(
-                f,
-                "Invalid placement of `-{}`.\nOnly the last letter in a sequence of short flags can take values.",
-                ch
-            ),
+            OofError::MisplacedShortArgFlagError(ch) => {
+                write!(
+                    f,
+                    "Invalid placement of `-{}`.\nOnly the last letter in a sequence of short flags can take values.",
+                    ch
+                )
+            }
             OofError::MissingValueToFlag(flag) => write!(f, "Flag {} takes value but none was supplied.", flag),
             OofError::DuplicatedFlag(flag) => write!(f, "Duplicated usage of {}.", flag),
             OofError::UnknownLongFlag(flag) => write!(f, "Unknown argument '--{}'", flag),