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