Patchwork D10003: rhg: Group values passed to every sub-command into a struct

login
register
mail settings
Submitter phabricator
Date Feb. 17, 2021, 12:59 p.m.
Message ID <differential-rev-PHID-DREV-txbpvu6qcseminp2misa-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48319/
State Superseded
Headers show

Comments

phabricator - Feb. 17, 2021, 12:59 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The set of which values this is is evidently not stable yet,
  so this will make changes easier. Also it is growing, and the function
  signatures are getting out hand.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  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/main.rs

CHANGE DETAILS




To: SimonSapin, #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
@@ -1,9 +1,11 @@ 
 extern crate log;
+use crate::ui::Ui;
 use clap::App;
 use clap::AppSettings;
 use clap::Arg;
 use clap::ArgMatches;
 use format_bytes::format_bytes;
+use hg::config::Config;
 use std::path::Path;
 
 mod error;
@@ -48,31 +50,41 @@ 
     let (subcommand_name, subcommand_matches) = matches.subcommand();
     let run = subcommand_run_fn(subcommand_name)
         .expect("unknown subcommand name from clap despite AppSettings::SubcommandRequired");
-    let args = subcommand_matches
+    let subcommand_args = subcommand_matches
         .expect("no subcommand arguments from clap despite AppSettings::SubcommandRequired");
 
     // Global arguments can be in either based on e.g. `hg -R ./foo log` v.s.
     // `hg log -R ./foo`
-    let value_of_global_arg =
-        |name| args.value_of_os(name).or_else(|| matches.value_of_os(name));
+    let value_of_global_arg = |name| {
+        subcommand_args
+            .value_of_os(name)
+            .or_else(|| matches.value_of_os(name))
+    };
     // For arguments where multiple occurences are allowed, return a
     // possibly-iterator of all values.
     let values_of_global_arg = |name: &str| {
         let a = matches.values_of_os(name).into_iter().flatten();
-        let b = args.values_of_os(name).into_iter().flatten();
+        let b = subcommand_args.values_of_os(name).into_iter().flatten();
         a.chain(b)
     };
 
-    let repo_path = value_of_global_arg("repository").map(Path::new);
     let config_args = values_of_global_arg("config")
         // `get_bytes_from_path` works for OsStr the same as for Path
         .map(hg::utils::files::get_bytes_from_path);
-    let config = hg::config::Config::load(config_args)?;
-    run(&ui, &config, repo_path, args)
+    let non_repo_config = &hg::config::Config::load(config_args)?;
+
+    let repo_path = value_of_global_arg("repository").map(Path::new);
+
+    run(&CliInvocation {
+        ui,
+        subcommand_args,
+        non_repo_config,
+        repo_path,
+    })
 }
 
 fn main() {
-    let ui = ui::Ui::new();
+    let ui = Ui::new();
 
     let exit_code = match main_with_result(&ui) {
         Ok(()) => exitcode::OK,
@@ -109,12 +121,9 @@ 
             )+
         }
 
-        fn subcommand_run_fn(name: &str) -> Option<fn(
-            &ui::Ui,
-            &hg::config::Config,
-            Option<&Path>,
-            &ArgMatches,
-        ) -> Result<(), CommandError>> {
+        pub type RunFn = fn(&CliInvocation) -> Result<(), CommandError>;
+
+        fn subcommand_run_fn(name: &str) -> Option<RunFn> {
             match name {
                 $(
                     stringify!($command) => Some(commands::$command::run),
@@ -133,3 +142,9 @@ 
     root
     config
 }
+pub struct CliInvocation<'a> {
+    ui: &'a Ui,
+    subcommand_args: &'a ArgMatches<'a>,
+    non_repo_config: &'a Config,
+    repo_path: Option<&'a Path>,
+}
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,11 +1,7 @@ 
 use crate::error::CommandError;
-use crate::ui::Ui;
-use clap::ArgMatches;
 use format_bytes::format_bytes;
-use hg::config::Config;
 use hg::repo::Repo;
 use hg::utils::files::get_bytes_from_path;
-use std::path::Path;
 
 pub const HELP_TEXT: &str = "
 Print the root directory of the current repository.
@@ -17,14 +13,11 @@ 
     clap::SubCommand::with_name("root").about(HELP_TEXT)
 }
 
-pub fn run(
-    ui: &Ui,
-    config: &Config,
-    repo_path: Option<&Path>,
-    _args: &ArgMatches,
-) -> Result<(), CommandError> {
-    let repo = Repo::find(config, repo_path)?;
+pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
+    let repo = Repo::find(invocation.non_repo_config, invocation.repo_path)?;
     let bytes = get_bytes_from_path(repo.working_directory_path());
-    ui.write_stdout(&format_bytes!(b"{}\n", bytes.as_slice()))?;
+    invocation
+        .ui
+        .write_stdout(&format_bytes!(b"{}\n", bytes.as_slice()))?;
     Ok(())
 }
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
@@ -1,14 +1,11 @@ 
 use crate::error::CommandError;
 use crate::ui::Ui;
 use clap::Arg;
-use clap::ArgMatches;
-use hg::config::Config;
 use hg::operations::list_rev_tracked_files;
 use hg::operations::Dirstate;
 use hg::repo::Repo;
 use hg::utils::files::{get_bytes_from_path, relativize_path};
 use hg::utils::hg_path::{HgPath, HgPathBuf};
-use std::path::Path;
 
 pub const HELP_TEXT: &str = "
 List tracked files.
@@ -29,23 +26,18 @@ 
         .about(HELP_TEXT)
 }
 
-pub fn run(
-    ui: &Ui,
-    config: &Config,
-    repo_path: Option<&Path>,
-    args: &ArgMatches,
-) -> Result<(), CommandError> {
-    let rev = args.value_of("rev");
+pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
+    let rev = invocation.subcommand_args.value_of("rev");
 
-    let repo = Repo::find(config, repo_path)?;
+    let repo = Repo::find(invocation.non_repo_config, invocation.repo_path)?;
     if let Some(rev) = rev {
         let files =
             list_rev_tracked_files(&repo, rev).map_err(|e| (e, rev))?;
-        display_files(ui, &repo, files.iter())
+        display_files(invocation.ui, &repo, files.iter())
     } else {
         let distate = Dirstate::new(&repo)?;
         let files = distate.tracked_files()?;
-        display_files(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,9 +1,5 @@ 
 use crate::error::CommandError;
-use crate::ui::Ui;
-use clap::ArgMatches;
-use hg::config::Config;
 use hg::repo::Repo;
-use std::path::Path;
 
 pub const HELP_TEXT: &str = "
 Print the current repo requirements.
@@ -13,13 +9,8 @@ 
     clap::SubCommand::with_name("debugrequirements").about(HELP_TEXT)
 }
 
-pub fn run(
-    ui: &Ui,
-    config: &Config,
-    repo_path: Option<&Path>,
-    _args: &ArgMatches,
-) -> Result<(), CommandError> {
-    let repo = Repo::find(config, repo_path)?;
+pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
+    let repo = Repo::find(invocation.non_repo_config, invocation.repo_path)?;
     let mut output = String::new();
     let mut requirements: Vec<_> = repo.requirements().iter().collect();
     requirements.sort();
@@ -27,6 +18,6 @@ 
         output.push_str(req);
         output.push('\n');
     }
-    ui.write_stdout(output.as_bytes())?;
+    invocation.ui.write_stdout(output.as_bytes())?;
     Ok(())
 }
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
@@ -1,13 +1,9 @@ 
 use crate::error::CommandError;
-use crate::ui::Ui;
 use clap::Arg;
 use clap::ArgGroup;
-use clap::ArgMatches;
-use hg::config::Config;
 use hg::operations::{debug_data, DebugDataKind};
 use hg::repo::Repo;
 use micro_timer::timed;
-use std::path::Path;
 
 pub const HELP_TEXT: &str = "
 Dump the contents of a data file revision
@@ -42,12 +38,8 @@ 
 }
 
 #[timed]
-pub fn run(
-    ui: &Ui,
-    config: &Config,
-    repo_path: Option<&Path>,
-    args: &ArgMatches,
-) -> Result<(), CommandError> {
+pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
+    let args = invocation.subcommand_args;
     let rev = args
         .value_of("rev")
         .expect("rev should be a required argument");
@@ -63,10 +55,10 @@ 
             }
         };
 
-    let repo = Repo::find(config, repo_path)?;
+    let repo = Repo::find(invocation.non_repo_config, invocation.repo_path)?;
     let data = debug_data(&repo, rev, kind).map_err(|e| (e, rev))?;
 
-    let mut stdout = ui.stdout_buffer();
+    let mut stdout = invocation.ui.stdout_buffer();
     stdout.write_all(&data)?;
     stdout.flush()?;
 
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
@@ -1,13 +1,9 @@ 
 use crate::error::CommandError;
-use crate::ui::Ui;
 use clap::Arg;
-use clap::ArgMatches;
 use format_bytes::format_bytes;
-use hg::config::Config;
 use hg::errors::HgError;
 use hg::repo::Repo;
 use hg::utils::SliceExt;
-use std::path::Path;
 
 pub const HELP_TEXT: &str = "
 With one argument of the form section.name, print just the value of that config item.
@@ -25,20 +21,16 @@ 
         .about(HELP_TEXT)
 }
 
-pub fn run(
-    ui: &Ui,
-    config: &Config,
-    repo_path: Option<&Path>,
-    args: &ArgMatches,
-) -> Result<(), CommandError> {
-    let opt_repo = Repo::find_optional(config, repo_path)?;
+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 {
-        config
+        invocation.non_repo_config
     };
-
-    let (section, name) = args
+    let (section, name) = invocation
+        .subcommand_args
         .value_of("name")
         .expect("missing required CLI argument")
         .as_bytes()
@@ -47,6 +39,6 @@ 
 
     let value = config.get(section, name).unwrap_or(b"");
 
-    ui.write_stdout(&format_bytes!(b"{}\n", value))?;
+    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,14 +1,10 @@ 
 use crate::error::CommandError;
-use crate::ui::Ui;
 use clap::Arg;
-use clap::ArgMatches;
-use hg::config::Config;
 use hg::operations::cat;
 use hg::repo::Repo;
 use hg::utils::hg_path::HgPathBuf;
 use micro_timer::timed;
 use std::convert::TryFrom;
-use std::path::Path;
 
 pub const HELP_TEXT: &str = "
 Output the current or given revision of files
@@ -36,19 +32,14 @@ 
 }
 
 #[timed]
-pub fn run(
-    ui: &Ui,
-    config: &Config,
-    repo_path: Option<&Path>,
-    args: &ArgMatches,
-) -> Result<(), CommandError> {
-    let rev = args.value_of("rev");
-    let file_args = match args.values_of("files") {
+pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
+    let rev = invocation.subcommand_args.value_of("rev");
+    let file_args = match invocation.subcommand_args.values_of("files") {
         Some(files) => files.collect(),
         None => vec![],
     };
 
-    let repo = Repo::find(config, repo_path)?;
+    let repo = Repo::find(invocation.non_repo_config, invocation.repo_path)?;
     let cwd = hg::utils::current_dir()?;
 
     let mut files = vec![];
@@ -67,7 +58,7 @@ 
     match rev {
         Some(rev) => {
             let data = cat(&repo, rev, &files).map_err(|e| (e, rev))?;
-            ui.write_stdout(&data)?;
+            invocation.ui.write_stdout(&data)?;
             Ok(())
         }
         None => Err(CommandError::Unimplemented.into()),