( )⚙ D10080 rhg: Use clap’s support for global CLI arguments

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10080

rhg: Use clap’s support for global CLI arguments
ClosedPublic

Authored by SimonSapin on Feb 26 2021, 6:17 AM.

Details

Reviewers
martinvonz
Group Reviewers
hg-reviewers
Commits
rHG4e4c70401028: rhg: Use clap’s support for global CLI arguments
Summary

By default, clap only accepts app-level arguments (as opposed to sub-command
level) to be specified before a sub-command: rhg -R ./foo log. Specifying
them after would be rejected: rhg log -R ./foo.

Previously we worked around that by registering global arguments both
at the app level and on each sub-command, but that required looking
for their value in two places. It turns out that Clap has built-in support
for what we want to do, so let’s use it.

Also, Clap "settings" turn out to be either global or not too.
Let’s make AllowInvalidUtf8 apply to sub-commands too.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

SimonSapin created this revision.Feb 26 2021, 6:17 AM
baymax edited the summary of this revision. (Show Details)Feb 26 2021, 7:11 AM
baymax updated this revision to Diff 25977.

✅ refresh by Heptapod after a successful CI run (🐙 💚)

martinvonz accepted this revision.Feb 26 2021, 1:31 PM
martinvonz added a subscriber: martinvonz.

Good find! :)

This revision is now accepted and ready to land.Feb 26 2021, 1:31 PM
SimonSapin added a commit: rHG4e4c70401028: rhg: Use clap’s support for global CLI arguments.Feb 26 2021, 1:34 PM
Closed by commit rHG4e4c70401028: rhg: Use clap’s support for global CLI arguments (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/rhg/src/main.rs (70 lines)

Diff 25974

rust/rhg/src/main.rs

use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
mod blackbox; mod blackbox;
mod error; mod error;
mod exitcode; mod exitcode;
mod ui; mod ui;
use error::CommandError; use error::CommandError;
fn add_global_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> { fn main_with_result(
app.arg( ui: &ui::Ui,
process_start_time: &blackbox::ProcessStartTime,
) -> Result<(), CommandError> {
env_logger::init();
let app = App::new("rhg")
.setting(AppSettings::AllowInvalidUtf8)
.setting(AppSettings::SubcommandRequired)
.setting(AppSettings::VersionlessSubcommands)
.arg(
Arg::with_name("repository") Arg::with_name("repository")
.help("repository root directory") .help("repository root directory")
.short("-R") .short("-R")
.long("--repository") .long("--repository")
.value_name("REPO") .value_name("REPO")
.takes_value(true), .takes_value(true)
// Both ok: `hg -R ./foo log` or `hg log -R ./foo`
.global(true),
) )
.arg( .arg(
Arg::with_name("config") Arg::with_name("config")
.help("set/override config option (use 'section.name=value')") .help("set/override config option (use 'section.name=value')")
.long("--config") .long("--config")
.value_name("CONFIG") .value_name("CONFIG")
.takes_value(true) .takes_value(true)
.global(true)
// Ok: `--config section.key1=val --config section.key2=val2` // Ok: `--config section.key1=val --config section.key2=val2`
.multiple(true) .multiple(true)
// Not ok: `--config section.key1=val section.key2=val2` // Not ok: `--config section.key1=val section.key2=val2`
.number_of_values(1), .number_of_values(1),
) )
}
fn main_with_result(
ui: &ui::Ui,
process_start_time: &blackbox::ProcessStartTime,
) -> Result<(), CommandError> {
env_logger::init();
let app = App::new("rhg")
.setting(AppSettings::AllowInvalidUtf8)
.setting(AppSettings::SubcommandRequired)
.setting(AppSettings::VersionlessSubcommands)
.version("0.0.1"); .version("0.0.1");
let app = add_global_args(app);
let app = add_subcommand_args(app); let app = add_subcommand_args(app);
let matches = app.clone().get_matches_safe()?; let matches = app.clone().get_matches_safe()?;
let (subcommand_name, subcommand_matches) = matches.subcommand(); let (subcommand_name, subcommand_matches) = matches.subcommand();
let run = subcommand_run_fn(subcommand_name) let run = subcommand_run_fn(subcommand_name)
.expect("unknown subcommand name from clap despite AppSettings::SubcommandRequired"); .expect("unknown subcommand name from clap despite AppSettings::SubcommandRequired");
let subcommand_args = subcommand_matches let subcommand_args = subcommand_matches
.expect("no subcommand arguments from clap despite AppSettings::SubcommandRequired"); .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. let config_args = matches
// `hg log -R ./foo` .values_of_os("config")
let value_of_global_arg = |name| { // Turn `Option::None` into an empty iterator:
subcommand_args .into_iter()
.value_of_os(name) .flatten()
.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 = subcommand_args.values_of_os(name).into_iter().flatten();
a.chain(b)
};
let config_args = values_of_global_arg("config")
.map(hg::utils::files::get_bytes_from_os_str); .map(hg::utils::files::get_bytes_from_os_str);
let non_repo_config = &hg::config::Config::load(config_args)?; let non_repo_config = &hg::config::Config::load(config_args)?;
let repo_path = value_of_global_arg("repository").map(Path::new); let repo_path = matches.value_of_os("repository").map(Path::new);
let repo = match Repo::find(non_repo_config, repo_path) { let repo = match Repo::find(non_repo_config, repo_path) {
Ok(repo) => Ok(repo), Ok(repo) => Ok(repo),
Err(RepoError::NotFound { at }) if repo_path.is_none() => { Err(RepoError::NotFound { at }) if repo_path.is_none() => {
// Not finding a repo is not fatal yet, if `-R` was not given // Not finding a repo is not fatal yet, if `-R` was not given
Err(NoRepoInCwdError { cwd: at }) Err(NoRepoInCwdError { cwd: at })
} }
Err(error) => return Err(error.into()), Err(error) => return Err(error.into()),
}; };
$( $(
pub mod $command; pub mod $command;
)+ )+
} }
fn add_subcommand_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> { fn add_subcommand_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
app app
$( $(
.subcommand(add_global_args(commands::$command::args())) .subcommand(commands::$command::args())
)+ )+
} }
pub type RunFn = fn(&CliInvocation) -> Result<(), CommandError>; pub type RunFn = fn(&CliInvocation) -> Result<(), CommandError>;
fn subcommand_run_fn(name: &str) -> Option<RunFn> { fn subcommand_run_fn(name: &str) -> Option<RunFn> {
match name { match name {
$( $(