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 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), - } - } -} To: acezar, #hg-reviewers Cc: mercurial-patches, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
