clap-rs / clap

A full featured, fast Command Line Argument Parser for Rust
docs.rs/clap
Apache License 2.0
14.06k stars 1.03k forks source link

clap_derive Arg methods with flattened #3269

Closed pinkforest closed 2 years ago

pinkforest commented 2 years ago

Please complete the following tasks

Clap Version

3.0.5

Describe your use case

I got two example use-cases

I came across this initially at cargo-clap/issues/27 and since it's use requires the use of flattened this should have some consideration what might be supported and document them for the future reference.

global on Enum Subcommand

Allow using global to reduce amount of code e.g. in enum Subcommand

#[derive(Subcommand)]
#[clap(setting(AppSettings::DeriveDisplayOrder))]
pub enum GeigerCommands {
    /// Geiger with build tree
    Build {
        #[clap(flatten)]
        workspace: clap_cargo::Workspace,
    },
    /// Geiger with test tree
    Test {
        #[clap(flatten)]
        workspace: clap_cargo::Workspace,
    }
}

Override members of pre-flattened arguments

Also f.ex. default_value & co - it would require getting rid of flattened and a way to pass "overrides" against members

e.g. cargo build to set the help that differs between build:

        --test <NAME>...             Build only the specified test target
        --tests                      Build all tests

and vs cargo test:

        --test <NAME>...             Test only the specified test target
        --tests                      Test all tests

Also env variant for this would be cool but I would think these should be hardcoded there - not sure.

Plus it would be nice to group workspace / manifest under their own heading with help_heading

Describe the solution you'd like

First it would be great to get a support statement and document them in the derive reference perhaps

Relevant with flatten might be:

Alternatives, if applicable

It would be nice if the functions get implemented for at least some in the future :)

Additional Context

I am migrating cargo-geiger to clap, clap-cargo and clap-cargo items require using flatten.

epage commented 2 years ago

Allow using global to reduce amount of code e.g. in enum Subcommand ... Relevant with flatten might be:

(focusing on the "apply this method to everything flattened)

The problem with this is we can't really communicate in the derive code between these different derives (on top of people hand-implementing). This means we have to do all of the communication through the trait at runtime. This puts a large burden on the trait implementation (which could be quite brittle from an API stability perspective) and there s a lot of complexity to get these interactions working correctly.

So far we have been rejecting features dependent on this.

Also f.ex. default_value & co - it would require getting rid of flattened and a way to pass "overrides" against members

(focusing on overriding a specific argument from a flatten)

Building this into the flatten construct has an extra level of complexity because of having to specify which argument an override applies to.

We do have the App::mut_arg method that can be used for modifying an argument. struct-level attributes are almost all evaluated after arguments, so the ordering would work. So you can do some level of overrides like

#[derive(clap::Parser)]
#[clap(mut_arg("test", |arg| arg.help("Test only the specified test target"))]
struct Cli {
...
}

One option might be to allow mut_arg on the flatten call so it is "close" to where it is needed. We might allow other app methods in this context to help with issues like https://github.com/clap-rs/clap/issues/1807 though those might be before adding the flattened arguments though if we add a #[clap::app()] attribute, that could allow for controlling ordering.

epage commented 2 years ago

btw customizing clap-cargo is something I have been thinking about. One other thought I had is to use macros to create distinct versions of the struct but that will lead to code bloat and make it hard to interact with them the same way.

taladar commented 2 years ago

It would be nice if this could at least be implemented for things like help_heading so we could easily group everything flattened in one spot under one help heading.

epage commented 2 years ago

help_heading is one of the few exceptions of what works because of how its implemented (App::help_heading). See this test.

The derive reference was previously updated to mention this.

epage commented 2 years ago

It sounds like this issue boils down to

Did I miss anything? If not, I'm inclined to close this issue

epage commented 2 years ago

Without any further updates, I'm moving forward with closing this. If more details become available, we can re-evaluate.