Patchwork D8865: rhg: ask the error message from `CommandError`

login
register
mail settings
Submitter phabricator
Date Aug. 3, 2020, 2:27 p.m.
Message ID <differential-rev-PHID-DREV-7y22ntzagtdvzusnkrqm-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/46966/
State Superseded
Headers show

Comments

phabricator - Aug. 3, 2020, 2:27 p.m.
acezar created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Avoid repeating the display of the same error messages in different commands.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

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

CHANGE DETAILS




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

Patch

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,44 @@ 
 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,
         }
     }
+
+    /// Return the message corresponding to the error kind if any
+    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 +67,11 @@ 
     pub fn exit(&self) -> () {
         std::process::exit(self.kind.get_exit_code())
     }
+
+    /// Return the message corresponding to the command error if any
+    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
@@ -1,9 +1,8 @@ 
 use crate::commands::Command;
 use crate::error::{CommandError, CommandErrorKind};
 use crate::ui::Ui;
-use hg::operations::{FindRoot, FindRootError, FindRootErrorKind};
+use hg::operations::{FindRoot, FindRootErrorKind};
 use hg::utils::files::get_bytes_from_path;
-use std::path::PathBuf;
 
 pub const HELP_TEXT: &str = "
 Print the root directory of the current repository.
@@ -11,19 +10,28 @@ 
 Returns 0 on success.
 ";
 
-pub struct RootCommand {
-    ui: Ui,
+pub struct RootCommand<'a> {
+    ui: &'a Ui,
+}
+
+impl<'a> RootCommand<'a> {
+    pub fn new(ui: &'a Ui) -> Self {
+        RootCommand { ui }
+    }
 }
 
-impl RootCommand {
-    pub fn new() -> Self {
-        RootCommand { ui: Ui::new() }
-    }
+impl<'a> Command<'a> for RootCommand<'a> {
+    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)
+                }
+            })?;
 
-    fn display_found_path(
-        &self,
-        path_buf: PathBuf,
-    ) -> Result<(), CommandError> {
         let bytes = get_bytes_from_path(path_buf);
 
         // TODO use formating macro
@@ -31,46 +39,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),
-        }
-    }
-}
diff --git a/rust/rhg/src/commands.rs b/rust/rhg/src/commands.rs
--- a/rust/rhg/src/commands.rs
+++ b/rust/rhg/src/commands.rs
@@ -4,6 +4,6 @@ 
 /// The common trait for rhg commands
 ///
 /// Normalize the interface of the commands provided by rhg
-pub trait Command {
+pub trait Command<'a> {
     fn run(&self) -> Result<(), CommandError>;
 }