Skip to content

Commit

Permalink
cli: process args to "--help" and "help"
Browse files Browse the repository at this point in the history
Clap bails parsing when an "error" is encountered, e.g. a subcommand is missing,
"--help" is passed, or the "help" subcommand is invoked. This means that the
current approach of parsing args does not handle flags like `--no-pager` or
`--color` when an error is encountered.

Fix this by separating early args into their own struct and preprocessing them
using `ignore_errors` (per clap-rs/clap#1880).

The early args are in a new `EarlyArgs` struct because of a known bug where
`ignore_errors` causes default values not to be respected
(clap-rs/clap#4391 specifically calls out bool, but
strings may also be ignored), so when `ignore_errors` is given, the default
values will be missing and parsing will fail unless the right arg types are used
(e.g`. Option`). By parsing only early args (using the new struct) we only need
to adjust `no_pager`, instead of adjusting all args with a default value.
  • Loading branch information
chooglen committed Dec 1, 2022
1 parent 6fe17c0 commit bb3cbc1
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 20 deletions.
53 changes: 34 additions & 19 deletions src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use std::sync::Arc;

use clap;
use clap::builder::{NonEmptyStringValueParser, TypedValueParser, ValueParserFactory};
use clap::{Arg, ArgMatches, Command, Error, FromArgMatches};
use clap::{Arg, ArgAction, ArgMatches, Command, Error, FromArgMatches};
use git2::{Oid, Repository};
use itertools::Itertools;
use jujutsu_lib::backend::{BackendError, CommitId, TreeId};
Expand Down Expand Up @@ -1177,6 +1177,8 @@ pub fn short_operation_hash(operation_id: &OperationId) -> String {
pub struct Args {
#[command(flatten)]
pub global_args: GlobalArgs,
#[command(flatten)]
pub early_args: EarlyArgs,
}

#[derive(clap::Args, Clone, Debug)]
Expand All @@ -1201,7 +1203,7 @@ pub struct GlobalArgs {
/// stale working copy commit, you can use `--no-commit-working-copy`.
/// This may be useful e.g. in a command prompt, especially if you have
/// another process that commits the working copy.
#[arg(long, global = true, help_heading = "Global Options")]
#[arg(long, global = true, help_heading = "Global Options", action = ArgAction::SetTrue)]
pub no_commit_working_copy: bool,
/// Operation to load the repo at
///
Expand Down Expand Up @@ -1229,6 +1231,13 @@ pub struct GlobalArgs {
default_value = "@"
)]
pub at_operation: String,
/// Enable verbose logging
#[arg(long, short = 'v', global = true, help_heading = "Global Options")]
pub verbose: bool,
}

#[derive(clap::Args, Clone, Debug)]
pub struct EarlyArgs {
/// When to colorize output (always, never, auto)
#[arg(
long,
Expand All @@ -1242,9 +1251,10 @@ pub struct GlobalArgs {
long,
value_name = "WHEN",
global = true,
help_heading = "Global Options"
help_heading = "Global Options",
action = ArgAction::SetTrue
)]
pub no_pager: bool,
pub no_pager: Option<bool>,
/// Additional configuration options
// TODO: Introduce a `--config` option with simpler syntax for simple
// cases, designed so that `--config ui.color=auto` works
Expand All @@ -1255,9 +1265,6 @@ pub struct GlobalArgs {
help_heading = "Global Options"
)]
pub config_toml: Vec<String>,
/// Enable verbose logging
#[arg(long, short = 'v', global = true, help_heading = "Global Options")]
pub verbose: bool,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -1396,6 +1403,21 @@ fn resolve_aliases(
}
}

/// Parse args that must be interpreted early, e.g. before printing help.
fn handle_early_args(ui: &mut Ui, args: &mut EarlyArgs) -> Result<(), CommandError> {
if let Some(choice) = args.color {
args.config_toml
.push(format!("ui.color=\"{}\"", choice.to_string()));
}
if args.no_pager.unwrap_or_default() {
ui.set_pagination(crate::ui::PaginationChoice::No);
}
if !args.config_toml.is_empty() {
ui.extra_toml_settings(&args.config_toml)?;
}
Ok(())
}

pub fn parse_args(
ui: &mut Ui,
app: clap::Command,
Expand All @@ -1409,22 +1431,15 @@ pub fn parse_args(
return Err(CommandError::CliError("Non-utf8 argument".to_string()));
}
}
// ignore_errors() bypasses errors like "--help" or missing subcommand
let early_matches = app.clone().ignore_errors(true).get_matches();
let mut toplevel_args: EarlyArgs = EarlyArgs::from_arg_matches(&early_matches).unwrap();
handle_early_args(ui, &mut toplevel_args)?;

let string_args = resolve_aliases(ui.settings(), &app, &string_args)?;
let matches = app.clone().try_get_matches_from(&string_args)?;

let mut args: Args = Args::from_arg_matches(&matches).unwrap();
if let Some(choice) = args.global_args.color {
args.global_args
.config_toml
.push(format!("ui.color=\"{}\"", choice.to_string()));
}
if args.global_args.no_pager {
ui.set_pagination(crate::ui::PaginationChoice::No);
}
if !args.global_args.config_toml.is_empty() {
ui.extra_toml_settings(&args.global_args.config_toml)?;
}
let args: Args = Args::from_arg_matches(&matches).unwrap();
let command_helper = CommandHelper::new(app, string_args, args.global_args);
Ok((command_helper, matches))
}
Expand Down
2 changes: 1 addition & 1 deletion tests/test_global_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,10 @@ fn test_help() {
-R, --repository <REPOSITORY> Path to repository to operate on
--no-commit-working-copy Don't commit the working copy
--at-operation <AT_OPERATION> Operation to load the repo at [default: @] [aliases: at-op]
-v, --verbose Enable verbose logging
--color <WHEN> When to colorize output (always, never, auto)
--no-pager Disable the pager
--config-toml <TOML> Additional configuration options
-v, --verbose Enable verbose logging
"###);
}

Expand Down

0 comments on commit bb3cbc1

Please sign in to comment.