cli: return an errors if testing

[amorenoz]

Fixes: #259

Instead of aborting the program, return the error so that tests can check against them.

[atenart]

I'm wondering why the initial issue was not seen on release v1.1. Do you know what introduced this behavior?

[amorenoz]

You mean that on v1.1 "--help" and "--version" were not raised as errors?

[atenart]

You mean that on v1.1 "--help" and "--version" were not raised as errors?

I'm not 100% sure of which versions are impacted, but testing a few builds on my machine,

v1.0:

$ ./target/release/retis --version
retis 1.0.0 ("pizza")

(IIRC the packaged v1.0 had not the issue).

v1.1:

$ ./target/release/retis --version
retis 1.1.0 ("pizza margherita")

(But the packaged v1.1 had the issue).

main w/ 3c651d7 reverted:

$ ./target/release/retis --version
Error: retis 1.1.0 ("pizza margherita")

[amorenoz]

It seems clap's behavior changed. The following code:

use clap::Command;

fn main()  {
   	let arg_vec = vec!["my_prog", "--help"];
	let matches = Command::new("myprog")
        .ignore_errors(true)
        .try_get_matches_from(arg_vec);

	println!("Matches {:?}", matches);
}

Yields different results when built against clap 3.2.25 (used in retis 1.1.0) and 4.3.19 (used by retis's main).

3.2.25:

Matches Ok(ArgMatches { valid_args: [help], valid_subcommands: [], disable_asserts: false, args: {}, subcommand: None })

4.3.19:

Matches Err(ErrorInner { kind: DisplayHelp, context: FlatMap { keys: [], values: [] }, message: Some(Formatted(StyledStr("\u{1b}[1m\u{1b}[4mUsage:\u{1b}[0m \u{1b}[1mmy_prog\u{1b}[0m\n\n\u{1b}[1m\u{1b}[4mOptions:\u{1b}[0m\n  \u{1b}[1m-h\u{1b}[0m, \u{1b}[1m--help\u{1b}[0m  Print help\n"))), source: None, help_flag: Some("--help"), styles: Styles { header: Style { fg: None, bg: None, underline: None, effects: Effects(BOLD | UNDERLINE) }, error: Style { fg: Some(Ansi(Red)), bg: None, underline: None, effects: Effects(BOLD) }, usage: Style { fg: None, bg: None, underline: None, effects: Effects(BOLD | UNDERLINE) }, literal: Style { fg: None, bg: None, underline: None, effects: Effects(BOLD) }, placeholder: Style { fg: None, bg: None, underline: None, effects: Effects() }, valid: Style { fg: Some(Ansi(Green)), bg: None, underline: None, effects: Effects() }, invalid: Style { fg: Some(Ansi(Yellow)), bg: None, underline: None, effects: Effects() } }, color_when: Auto, color_help_when: Auto, backtrace: None })

[amorenoz]

I think the previous behavior is the buggy one so they must have fixed it.

[amorenoz]

Actually: clap-rs/clap#4988

[atenart]

Thanks for the links. I'm still wondering why my local build of v1.1.0 did not had the issue while the build in copr had it, but I'm assuming the version of clap used might have been slightly different for some reason.

[atenart]

"build_from() should only be used directly by unit tests." <- could we add this in the function comments?

[atenart]

LGTM, thanks!