Easy to accidentally use setting when global_setting is intended

[epage]

Issue by epage
Monday Nov 15, 2021 at 18:08 GMT
Originally opened as clap-rs/clap#3028

---

Please complete the following tasks

• I have searched the discussions
• I have searched the existing issues

Rust Version

rustc 1.55.0 (c8dfcfe04 2021-09-06)

Clap Version

2.33

Minimal reproducible code

When supporting a user on TeXitoi/structopt#129, I accidentally gave the following non-functional code, confused as to why it wasn't working:

use structopt::StructOpt;

#[derive(StructOpt)]
#[structopt(about = "Sum one or more lists of ints.")]
#[structopt(settings = &[structopt::clap::AppSettings::AllowNegativeNumbers])]
enum Cli {
    SumOne {
        #[structopt(short, required = true)]
        a: Vec<isize>,
    },
    SumTwo {
        #[structopt(short, required = true)]
        a: Vec<isize>,
        #[structopt(short, required = true)]
        b: Vec<isize>,
    },
}

fn main() {
    fn sum(l: &[isize]) -> isize {
        l.iter().sum()
    }
    match Cli::from_args() {
        Cli::SumOne { a } => println!("{}", sum(&a)),
        Cli::SumTwo { a, b } => println!("{} {}", sum(&a), sum(&b)),
    }
}

when I should have done

use structopt::StructOpt;

#[derive(StructOpt)]
#[structopt(about = "Sum one or more lists of ints.")]
#[structopt(global_setting = structopt::clap::AppSettings::AllowNegativeNumbers)]
enum Cli {
    SumOne {
        #[structopt(short, required = true)]
        a: Vec<isize>,
    },
    SumTwo {
        #[structopt(short, required = true)]
        a: Vec<isize>,
        #[structopt(short, required = true)]
        b: Vec<isize>,
    },
}

fn main() {
    fn sum(l: &[isize]) -> isize {
        l.iter().sum()
    }
    match Cli::from_args() {
        Cli::SumOne { a } => println!("{}", sum(&a)),
        Cli::SumTwo { a, b } => println!("{} {}", sum(&a), sum(&b)),
    }
}

Steps to reproduce the bug with the above code

Run it: cargo run -- sum-one -a 1 2 -3

Actual Behaviour

Fails with unknown -3 arg

Expected Behaviour

Prints 0

Additional Context

I think every place I use an AppSettings in my programs, I've done this wrong.

The problem with this is that clap mixes subcommand logic (e.g. SubcommandRequiredElseHelp) with command logic (e.g. DisabledColoredHelp). Using the command logic without global_setting is likely a bug but conversely, using subcommand logic with global_setting is likely a bug. I have not looked through all of the settings to see if there are any middle ground cases.

EDIT: There is a third category, command display logic, like help_heading. This should not be treated with global.

At minimum, all examples should be updated to use global_setting for command-related settings, even if subcommands aren't used. People will take these examples and apply them to their subcommands or could later extend their command to have subcommands, so we should treat this as the de facto approach.

However, this still requires a lot of communication to the user so they know which to use when. Alternatively

1. #2717
2. Group command functions separate from subcommand functions (or find an API design to have them on separate types)
3. Layer each App's command config below its subcommands, ie propagate all command configuration to subcommands, recursively, if the subcommand does not have an explicit value set (instead that will then get propagated when recursing)

• For regular values, we just wrap them in Option and provide accessor functions that unwrap_or(...) so we can track whether it is explicitly set while being ergonomic. I've found this pattern fairly successful when dealing with layered config (example 1, example 2) and args (example).
• For Settings, we can have a parallel bitflags that tracks whether the value is explicitly set or not.

This approach provides the behavior users expect while giving them the control to do something we didn't predict.

Debug Output

No response