D10004: rhg: Move `Repo` object creation into `main()`
SimonSapin
phabricator at mercurial-scm.org
Wed Feb 17 12:59:23 UTC 2021
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.
REVISION SUMMARY
⦠rather than in each sub-command that needs a local repository.
This will allow accessing e.g. `.hg/blackbox.log` before dispatching
to sub-commands.
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D10004
AFFECTED FILES
rust/hg-core/src/repo.rs
rust/rhg/src/commands/cat.rs
rust/rhg/src/commands/config.rs
rust/rhg/src/commands/debugdata.rs
rust/rhg/src/commands/debugrequirements.rs
rust/rhg/src/commands/files.rs
rust/rhg/src/commands/root.rs
rust/rhg/src/error.rs
rust/rhg/src/main.rs
CHANGE DETAILS
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
@@ -6,7 +6,8 @@
use clap::ArgMatches;
use format_bytes::format_bytes;
use hg::config::Config;
-use std::path::Path;
+use hg::repo::{Repo, RepoError};
+use std::path::{Path, PathBuf};
mod error;
mod exitcode;
@@ -74,17 +75,25 @@
let non_repo_config = &hg::config::Config::load(config_args)?;
let repo_path = value_of_global_arg("repository").map(Path::new);
+ let repo = match Repo::find(non_repo_config, repo_path) {
+ Ok(repo) => Ok(repo),
+ Err(RepoError::NotFound { at }) if repo_path.is_none() => {
+ // Not finding a repo is not fatal yet, if `-R` was not given
+ Err(NoRepoInCwdError { cwd: at })
+ }
+ Err(error) => return Err(error.into()),
+ };
run(&CliInvocation {
ui,
subcommand_args,
non_repo_config,
- repo_path,
+ repo: repo.as_ref(),
})
}
fn main() {
- let ui = Ui::new();
+ let ui = ui::Ui::new();
let exit_code = match main_with_result(&ui) {
Ok(()) => exitcode::OK,
@@ -146,5 +155,22 @@
ui: &'a Ui,
subcommand_args: &'a ArgMatches<'a>,
non_repo_config: &'a Config,
- repo_path: Option<&'a Path>,
+ /// References inside `Result` is a bit peculiar but allow
+ /// `invocation.repo?` to work out with `&CliInvocation` since this
+ /// `Result` type is `Copy`.
+ repo: Result<&'a Repo, &'a NoRepoInCwdError>,
+}
+
+struct NoRepoInCwdError {
+ cwd: PathBuf,
}
+
+impl CliInvocation<'_> {
+ fn config(&self) -> &Config {
+ if let Ok(repo) = self.repo {
+ repo.config()
+ } else {
+ self.non_repo_config
+ }
+ }
+}
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,5 +1,6 @@
use crate::ui::utf8_to_local;
use crate::ui::UiError;
+use crate::NoRepoInCwdError;
use format_bytes::format_bytes;
use hg::config::{ConfigError, ConfigParseError};
use hg::errors::HgError;
@@ -64,7 +65,7 @@
match error {
RepoError::NotFound { at } => CommandError::Abort {
message: format_bytes!(
- b"no repository found in '{}' (.hg not found)!",
+ b"repository {} not found",
get_bytes_from_path(at)
),
},
@@ -74,6 +75,18 @@
}
}
+impl<'a> From<&'a NoRepoInCwdError> for CommandError {
+ fn from(error: &'a NoRepoInCwdError) -> Self {
+ let NoRepoInCwdError { cwd } = error;
+ CommandError::Abort {
+ message: format_bytes!(
+ b"no repository found in '{}' (.hg not found)!",
+ get_bytes_from_path(cwd)
+ ),
+ }
+ }
+}
+
impl From<ConfigError> for CommandError {
fn from(error: ConfigError) -> Self {
match error {
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,6 +1,5 @@
use crate::error::CommandError;
use format_bytes::format_bytes;
-use hg::repo::Repo;
use hg::utils::files::get_bytes_from_path;
pub const HELP_TEXT: &str = "
@@ -14,7 +13,7 @@
}
pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
- let repo = Repo::find(invocation.non_repo_config, invocation.repo_path)?;
+ let repo = invocation.repo?;
let bytes = get_bytes_from_path(repo.working_directory_path());
invocation
.ui
diff --git a/rust/rhg/src/commands/files.rs b/rust/rhg/src/commands/files.rs
--- a/rust/rhg/src/commands/files.rs
+++ b/rust/rhg/src/commands/files.rs
@@ -29,15 +29,14 @@
pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
let rev = invocation.subcommand_args.value_of("rev");
- let repo = Repo::find(invocation.non_repo_config, invocation.repo_path)?;
+ let repo = invocation.repo?;
if let Some(rev) = rev {
- let files =
- list_rev_tracked_files(&repo, rev).map_err(|e| (e, rev))?;
- display_files(invocation.ui, &repo, files.iter())
+ let files = list_rev_tracked_files(repo, rev).map_err(|e| (e, rev))?;
+ display_files(invocation.ui, repo, files.iter())
} else {
- let distate = Dirstate::new(&repo)?;
+ let distate = Dirstate::new(repo)?;
let files = distate.tracked_files()?;
- display_files(invocation.ui, &repo, files)
+ display_files(invocation.ui, repo, files)
}
}
diff --git a/rust/rhg/src/commands/debugrequirements.rs b/rust/rhg/src/commands/debugrequirements.rs
--- a/rust/rhg/src/commands/debugrequirements.rs
+++ b/rust/rhg/src/commands/debugrequirements.rs
@@ -1,5 +1,4 @@
use crate::error::CommandError;
-use hg::repo::Repo;
pub const HELP_TEXT: &str = "
Print the current repo requirements.
@@ -10,7 +9,7 @@
}
pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
- let repo = Repo::find(invocation.non_repo_config, invocation.repo_path)?;
+ let repo = invocation.repo?;
let mut output = String::new();
let mut requirements: Vec<_> = repo.requirements().iter().collect();
requirements.sort();
diff --git a/rust/rhg/src/commands/debugdata.rs b/rust/rhg/src/commands/debugdata.rs
--- a/rust/rhg/src/commands/debugdata.rs
+++ b/rust/rhg/src/commands/debugdata.rs
@@ -2,7 +2,6 @@
use clap::Arg;
use clap::ArgGroup;
use hg::operations::{debug_data, DebugDataKind};
-use hg::repo::Repo;
use micro_timer::timed;
pub const HELP_TEXT: &str = "
@@ -55,8 +54,8 @@
}
};
- let repo = Repo::find(invocation.non_repo_config, invocation.repo_path)?;
- let data = debug_data(&repo, rev, kind).map_err(|e| (e, rev))?;
+ let repo = invocation.repo?;
+ let data = debug_data(repo, rev, kind).map_err(|e| (e, rev))?;
let mut stdout = invocation.ui.stdout_buffer();
stdout.write_all(&data)?;
diff --git a/rust/rhg/src/commands/config.rs b/rust/rhg/src/commands/config.rs
--- a/rust/rhg/src/commands/config.rs
+++ b/rust/rhg/src/commands/config.rs
@@ -2,7 +2,6 @@
use clap::Arg;
use format_bytes::format_bytes;
use hg::errors::HgError;
-use hg::repo::Repo;
use hg::utils::SliceExt;
pub const HELP_TEXT: &str = "
@@ -22,13 +21,6 @@
}
pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
- let opt_repo =
- Repo::find_optional(invocation.non_repo_config, invocation.repo_path)?;
- let config = if let Some(repo) = &opt_repo {
- repo.config()
- } else {
- invocation.non_repo_config
- };
let (section, name) = invocation
.subcommand_args
.value_of("name")
@@ -37,7 +29,7 @@
.split_2(b'.')
.ok_or_else(|| HgError::abort(""))?;
- let value = config.get(section, name).unwrap_or(b"");
+ let value = invocation.config().get(section, name).unwrap_or(b"");
invocation.ui.write_stdout(&format_bytes!(b"{}\n", value))?;
Ok(())
diff --git a/rust/rhg/src/commands/cat.rs b/rust/rhg/src/commands/cat.rs
--- a/rust/rhg/src/commands/cat.rs
+++ b/rust/rhg/src/commands/cat.rs
@@ -1,7 +1,6 @@
use crate::error::CommandError;
use clap::Arg;
use hg::operations::cat;
-use hg::repo::Repo;
use hg::utils::hg_path::HgPathBuf;
use micro_timer::timed;
use std::convert::TryFrom;
@@ -39,7 +38,7 @@
None => vec![],
};
- let repo = Repo::find(invocation.non_repo_config, invocation.repo_path)?;
+ let repo = invocation.repo?;
let cwd = hg::utils::current_dir()?;
let mut files = vec![];
diff --git a/rust/hg-core/src/repo.rs b/rust/hg-core/src/repo.rs
--- a/rust/hg-core/src/repo.rs
+++ b/rust/hg-core/src/repo.rs
@@ -81,28 +81,6 @@
}
}
- /// Like `Repo::find`, but not finding a repository is not an error if no
- /// explicit path is given. `Ok(None)` is returned in that case.
- ///
- /// If an explicit path *is* given, not finding a repository there is still
- /// an error.
- ///
- /// For sub-commands that donât need a repository, configuration should
- /// still be affected by a repositoryâs `.hg/hgrc` file. This is the
- /// constructor to use.
- pub fn find_optional(
- config: &Config,
- explicit_path: Option<&Path>,
- ) -> Result<Option<Self>, RepoError> {
- match Self::find(config, explicit_path) {
- Ok(repo) => Ok(Some(repo)),
- Err(RepoError::NotFound { .. }) if explicit_path.is_none() => {
- Ok(None)
- }
- Err(error) => Err(error),
- }
- }
-
/// To be called after checking that `.hg` is a sub-directory
fn new_at_path(
working_directory: PathBuf,
To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
More information about the Mercurial-devel
mailing list