This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8612

rhg: add RootCommand using hg-core FindRoot operation to prepare `hg root`
ClosedPublic

Authored by acezar on Jun 5 2020, 10:31 AM.

Details

Reviewers
marmoute
Alphare
Group Reviewers
hg-reviewers
Commits
rHG513b3ef277a3: rhg: add RootCommand using hg-core FindRoot operation to prepare `hg root`
rHG269c823cca34: rhg: add RootCommand using hg-core FindRoot operation to prepare `hg root`
rHGafecc7f76f2d: rhg: add RootCommand using hg-core FindRoot operation to prepare `hg root`
Summary

The println macro is not used to avoid string usage.
Dealing only with bytes allows us to be compatible with any encoding
and not just UTF8.

Later on, format macro will be made to have more readable code.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

acezar created this revision.Jun 5 2020, 10:31 AM
marmoute requested changes to this revision.Jun 9 2020, 5:33 AM
marmoute added a subscriber: marmoute.
marmoute added inline comments.
rust/rhg/src/commands/root.rs
25

If stdout is closed, this will fails. We probably want a better error than that eventually.

35

Maybe we use a bit more explicit error, like: "Error is RootNotFound, but no path provided" or soemthing in that flavor.

51

Same feedback, there wil be legit cas were we cannot write to stdout and we should improve the errors here (and test theses case)

61

What kind of error does Mercurial issue in this case ? Could we keep the same behavior ?

This revision now requires changes to proceed.Jun 9 2020, 5:33 AM
acezar updated this revision to Diff 21647.Jun 16 2020, 12:20 PM
acezar updated this revision to Diff 21657.Jun 17 2020, 4:25 AM
acezar marked 4 inline comments as done.Jun 22 2020, 8:24 AM
Alphare accepted this revision.Jun 22 2020, 10:44 AM
Alphare added a subscriber: Alphare.

Looks good. If I could nitpick some more, the description talks about WTF8, when it's UTF8 (because we're talking about String and not OsString).

acezar edited the summary of this revision. (Show Details)Jun 24 2020, 9:04 AM
acezar updated this revision to Diff 21697.
Alphare accepted this revision.Jun 24 2020, 10:05 AM
acezar added a commit: rHGafecc7f76f2d: rhg: add RootCommand using hg-core FindRoot operation to prepare `hg root`.Jul 1 2020, 6:33 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGafecc7f76f2d: rhg: add RootCommand using hg-core FindRoot operation to prepare `hg root` (authored by acezar). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 21748

rust/Cargo.lock

source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [ dependencies = [
"winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
] ]
[[package]] [[package]]
name = "rhg" name = "rhg"
version = "0.1.0" version = "0.1.0"
dependencies = [
"hg-core 0.1.0",
]
[[package]] [[package]]
name = "rustc_version" name = "rustc_version"
version = "0.2.3" version = "0.2.3"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [ dependencies = [
"semver 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", "semver 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
] ]

rust/rhg/Cargo.toml

[package] [package]
name = "rhg" name = "rhg"
version = "0.1.0" version = "0.1.0"
authors = ["Antoine Cezar <antoine.cezar@octobus.net>"] authors = ["Antoine Cezar <antoine.cezar@octobus.net>"]
edition = "2018" edition = "2018"
[dependencies] [dependencies]
hg-core = { path = "../hg-core"}

rust/rhg/src/commands.rs

pub mod root;
use crate::error::CommandError; use crate::error::CommandError;
/// The common trait for rhg commands /// The common trait for rhg commands
/// ///
/// Normalize the interface of the commands provided by rhg /// Normalize the interface of the commands provided by rhg
pub trait Command { pub trait Command {
fn run(&self) -> Result<(), CommandError>; fn run(&self) -> Result<(), CommandError>;
} }

rust/rhg/src/commands/root.rs

  • This file was added.
use crate::commands::Command;
use crate::error::{CommandError, CommandErrorKind};
use crate::ui::Ui;
use hg::operations::{FindRoot, FindRootError, FindRootErrorKind, Operation};
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.
Returns 0 on success.
";
pub struct RootCommand {
ui: Ui,
}
impl RootCommand {
pub fn new() -> Self {
RootCommand { ui: Ui::new() }
}
fn display_found_path(
&self,
path_buf: PathBuf,
marmouteUnsubmitted
Done

If stdout is closed, this will fails. We probably want a better error than that eventually.

marmoute: If stdout is closed, this will fails. We probably want a better error than that eventually.
) -> Result<(), CommandError> {
let bytes = get_bytes_from_path(path_buf);
// TODO use formating macro
self.ui.write_stdout(&[bytes.as_slice(), b"\n"].concat())?;
Err(CommandErrorKind::Ok.into())
}
fn display_error(&self, error: FindRootError) -> Result<(), CommandError> {
marmouteUnsubmitted
Done

Maybe we use a bit more explicit error, like: "Error is RootNotFound, but no path provided" or soemthing in that flavor.

marmoute: Maybe we use a bit more explicit error, like: "Error is RootNotFound, but no path provided" or…
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())
}
marmouteUnsubmitted
Done

Same feedback, there wil be legit cas were we cannot write to stdout and we should improve the errors here (and test theses case)

marmoute: Same feedback, there wil be legit cas were we cannot write to stdout and we should improve the…
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(),
)?;
marmouteUnsubmitted
Done

What kind of error does Mercurial issue in this case ? Could we keep the same behavior ?

marmoute: What kind of error does Mercurial issue in this case ? Could we keep the same behavior ?
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),
}
}
}

rust/rhg/src/error.rs

use crate::exitcode;
use crate::ui::UiError;
use std::convert::From;
/// The kind of command error
#[derive(Debug, PartialEq)]
pub enum CommandErrorKind {
/// The command finished without error
Ok,
/// The root of the repository cannot be found
RootNotFound,
/// The current directory cannot be found
CurrentDirNotFound,
/// The standard output stream cannot be written to
StdoutError,
/// The standard error stream cannot be written to
StderrError,
}
impl CommandErrorKind {
pub fn get_exit_code(&self) -> exitcode::ExitCode {
match self {
CommandErrorKind::Ok => exitcode::OK,
CommandErrorKind::RootNotFound => exitcode::ABORT,
CommandErrorKind::CurrentDirNotFound => exitcode::ABORT,
CommandErrorKind::StdoutError => exitcode::ABORT,
CommandErrorKind::StderrError => exitcode::ABORT,
}
}
}
/// The error type for the Command trait /// The error type for the Command trait
#[derive(Debug, PartialEq)] #[derive(Debug, PartialEq)]
pub struct CommandError {} pub struct CommandError {
pub kind: CommandErrorKind,
}
impl CommandError {
/// Exist the process with the corresponding exit code.
pub fn exit(&self) -> () {
std::process::exit(self.kind.get_exit_code())
}
}
impl From<CommandErrorKind> for CommandError {
fn from(kind: CommandErrorKind) -> Self {
CommandError { kind }
}
}
impl From<UiError> for CommandError {
fn from(error: UiError) -> Self {
CommandError {
kind: match error {
UiError::StdoutError(_) => CommandErrorKind::StdoutError,
UiError::StderrError(_) => CommandErrorKind::StderrError,
},
}
}
}

rust/rhg/src/exitcode.rs

pub type ExitCode = i32; pub type ExitCode = i32;
/// Successful exit
pub const OK: ExitCode = 0;
/// Generic abort
pub const ABORT: ExitCode = 255;
/// Command not implemented by rhg /// Command not implemented by rhg
pub const UNIMPLEMENTED_COMMAND: ExitCode = 252; pub const UNIMPLEMENTED_COMMAND: ExitCode = 252;

rust/rhg/src/main.rs

mod commands; mod commands;
mod error; mod error;
mod exitcode; mod exitcode;
mod ui;
fn main() { fn main() {
std::process::exit(exitcode::UNIMPLEMENTED_COMMAND) std::process::exit(exitcode::UNIMPLEMENTED_COMMAND)
} }

rust/rhg/src/ui.rs

  • This file was added.
use std::io;
use std::io::Write;
pub struct Ui {}
/// The kind of user interface error
pub enum UiError {
/// The standard output stream cannot be written to
StdoutError(io::Error),
/// The standard error stream cannot be written to
StderrError(io::Error),
}
/// The commandline user interface
impl Ui {
pub fn new() -> Self {
Ui {}
}
/// Write bytes to stdout
pub fn write_stdout(&self, bytes: &[u8]) -> Result<(), UiError> {
let mut stdout = io::stdout();
self.write_stream(&mut stdout, bytes)
.or_else(|e| self.into_stdout_error(e))?;
stdout.flush().or_else(|e| self.into_stdout_error(e))
}
fn into_stdout_error(&self, error: io::Error) -> Result<(), UiError> {
self.write_stderr(
&[b"abort: ", error.to_string().as_bytes(), b"\n"].concat(),
)?;
Err(UiError::StdoutError(error))
}
/// Write bytes to stderr
pub fn write_stderr(&self, bytes: &[u8]) -> Result<(), UiError> {
let mut stderr = io::stderr();
self.write_stream(&mut stderr, bytes)
.or_else(|e| Err(UiError::StderrError(e)))?;
stderr.flush().or_else(|e| Err(UiError::StderrError(e)))
}
fn write_stream(
&self,
stream: &mut impl Write,
bytes: &[u8],
) -> Result<(), io::Error> {
stream.write_all(bytes)
}
}