Patchwork D8777: rhg: Ask the error message from `CommandError`

login
register
mail settings
Submitter phabricator
Date July 21, 2020, 2:07 p.m.
Message ID <differential-rev-PHID-DREV-dpmucdsrhja6xopldarr-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/46823/
State Superseded
Headers show

Comments

phabricator - July 21, 2020, 2:07 p.m.
acezar created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Avoid repeating the display part of the same error in different commands.
  For example, a lot of commands while use the `FindRoot` `Operation`, rather than
  having to display the same error message in different command, the command can
  just return the proper error which know what is the appropriate message to
  display.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8777

AFFECTED FILES
  rust/rhg/src/commands/root.rs
  rust/rhg/src/error.rs
  rust/rhg/src/main.rs
  rust/rhg/src/ui.rs

CHANGE DETAILS




To: acezar, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/rhg/src/ui.rs b/rust/rhg/src/ui.rs
--- a/rust/rhg/src/ui.rs
+++ b/rust/rhg/src/ui.rs
@@ -1,6 +1,7 @@ 
 use std::io;
 use std::io::Write;
 
+#[derive(Copy, Clone)]
 pub struct Ui {}
 
 /// The kind of user interface error
diff --git a/rust/rhg/src/main.rs b/rust/rhg/src/main.rs
--- a/rust/rhg/src/main.rs
+++ b/rust/rhg/src/main.rs
@@ -22,9 +22,11 @@ 
         std::process::exit(exitcode::UNIMPLEMENTED_COMMAND)
     });
 
+    let ui = ui::Ui::new();
+
     let command_result = match matches.subcommand_name() {
         Some(name) => match name {
-            "root" => commands::root::RootCommand::new().run(),
+            "root" => commands::root::RootCommand::new(ui).run(),
             _ => std::process::exit(exitcode::UNIMPLEMENTED_COMMAND),
         },
         _ => {
@@ -37,6 +39,15 @@ 
 
     match command_result {
         Ok(_) => std::process::exit(exitcode::OK),
-        Err(e) => e.exit(),
+        Err(e) => {
+            let message = e.get_error_message_bytes();
+            if let Some(msg) = message {
+                match ui.write_stderr(&msg) {
+                    Ok(_) => (),
+                    Err(_) => std::process::exit(exitcode::ABORT),
+                };
+            };
+            e.exit()
+        }
     }
 }
diff --git a/rust/rhg/src/error.rs b/rust/rhg/src/error.rs
--- a/rust/rhg/src/error.rs
+++ b/rust/rhg/src/error.rs
@@ -1,14 +1,16 @@ 
 use crate::exitcode;
 use crate::ui::UiError;
+use hg::utils::files::get_bytes_from_path;
 use std::convert::From;
+use std::path::PathBuf;
 
 /// The kind of command error
-#[derive(Debug, PartialEq)]
+#[derive(Debug)]
 pub enum CommandErrorKind {
     /// The root of the repository cannot be found
-    RootNotFound,
+    RootNotFound(PathBuf),
     /// The current directory cannot be found
-    CurrentDirNotFound,
+    CurrentDirNotFound(std::io::Error),
     /// The standard output stream cannot be written to
     StdoutError,
     /// The standard error stream cannot be written to
@@ -18,16 +20,43 @@ 
 impl CommandErrorKind {
     pub fn get_exit_code(&self) -> exitcode::ExitCode {
         match self {
-            CommandErrorKind::RootNotFound => exitcode::ABORT,
-            CommandErrorKind::CurrentDirNotFound => exitcode::ABORT,
+            CommandErrorKind::RootNotFound(_) => exitcode::ABORT,
+            CommandErrorKind::CurrentDirNotFound(_) => exitcode::ABORT,
             CommandErrorKind::StdoutError => exitcode::ABORT,
             CommandErrorKind::StderrError => exitcode::ABORT,
         }
     }
+
+    pub fn get_error_message_bytes(&self) -> Option<Vec<u8>> {
+        match self {
+            // TODO use formating macro
+            CommandErrorKind::RootNotFound(path) => {
+                let bytes = get_bytes_from_path(path);
+                Some(
+                    [
+                        b"abort: no repository found in '",
+                        bytes.as_slice(),
+                        b"' (.hg not found)!\n",
+                    ]
+                    .concat(),
+                )
+            }
+            // TODO use formating macro
+            CommandErrorKind::CurrentDirNotFound(e) => Some(
+                [
+                    b"abort: error getting current working directory: ",
+                    e.to_string().as_bytes(),
+                    b"\n",
+                ]
+                .concat(),
+            ),
+            _ => None,
+        }
+    }
 }
 
 /// The error type for the Command trait
-#[derive(Debug, PartialEq)]
+#[derive(Debug)]
 pub struct CommandError {
     pub kind: CommandErrorKind,
 }
@@ -37,6 +66,10 @@ 
     pub fn exit(&self) -> () {
         std::process::exit(self.kind.get_exit_code())
     }
+
+    pub fn get_error_message_bytes(&self) -> Option<Vec<u8>> {
+        self.kind.get_error_message_bytes()
+    }
 }
 
 impl From<CommandErrorKind> for CommandError {
diff --git a/rust/rhg/src/commands/root.rs b/rust/rhg/src/commands/root.rs
--- a/rust/rhg/src/commands/root.rs
+++ b/rust/rhg/src/commands/root.rs
@@ -16,14 +16,23 @@ 
 }
 
 impl RootCommand {
-    pub fn new() -> Self {
-        RootCommand { ui: Ui::new() }
+    pub fn new(ui: Ui) -> Self {
+        RootCommand { ui }
     }
+}
 
-    fn display_found_path(
-        &self,
-        path_buf: PathBuf,
-    ) -> Result<(), CommandError> {
+impl Command for RootCommand {
+    fn run(&self) -> Result<(), CommandError> {
+        let path_buf =
+            FindRoot::new().run().map_err(|err| match err.kind {
+                FindRootErrorKind::RootNotFound(path) => {
+                    CommandErrorKind::RootNotFound(path)
+                }
+                FindRootErrorKind::GetCurrentDirError(e) => {
+                    CommandErrorKind::CurrentDirNotFound(e)
+                }
+            })?;
+
         let bytes = get_bytes_from_path(path_buf);
 
         // TODO use formating macro
@@ -31,46 +40,4 @@ 
 
         Ok(())
     }
-
-    fn display_error(&self, error: FindRootError) -> Result<(), CommandError> {
-        match error.kind {
-            FindRootErrorKind::RootNotFound(path) => {
-                let bytes = get_bytes_from_path(path);
-
-                // TODO use formating macro
-                self.ui.write_stderr(
-                    &[
-                        b"abort: no repository found in '",
-                        bytes.as_slice(),
-                        b"' (.hg not found)!\n",
-                    ]
-                    .concat(),
-                )?;
-
-                Err(CommandErrorKind::RootNotFound.into())
-            }
-            FindRootErrorKind::GetCurrentDirError(e) => {
-                // TODO use formating macro
-                self.ui.write_stderr(
-                    &[
-                        b"abort: error getting current working directory: ",
-                        e.to_string().as_bytes(),
-                        b"\n",
-                    ]
-                    .concat(),
-                )?;
-
-                Err(CommandErrorKind::CurrentDirNotFound.into())
-            }
-        }
-    }
 }
-
-impl Command for RootCommand {
-    fn run(&self) -> Result<(), CommandError> {
-        match FindRoot::new().run() {
-            Ok(path_buf) => self.display_found_path(path_buf),
-            Err(e) => self.display_error(e),
-        }
-    }
-}