Args/Subcommand app settings shouldn't impact what they are flattened into

epage commented 2 years ago

Issue by epage Saturday Oct 16, 2021 at 15:03 GMT Originally opened as https://github.com/clap-rs/clap/issues/2894

Please complete the following tasks

[X] I have searched the discussions
[X] I have searched the existing issues

Rust Version

rustc 1.55.0 (c8dfcfe04 2021-09-06)

Clap Version

master

Minimal reproducible code

use clap::{Args, Parser};

#[derive(Parser, PartialEq, Debug)]
struct Opt {
    #[clap(flatten)]
    common: Common,
}

#[derive(Args, PartialEq, Debug)]
#[clap(version = "3.0.0")]
struct Common {
    arg: i32,
}

fn main() {
    Opt::parse();
}

Steps to reproduce the bug with the above code

cargo run -- --help

Actual Behaviour

test-clap 3.0.0

USAGE:
    test-clap <ARG>

ARGS:
    <ARG>    

OPTIONS:
    -h, --help       Print help information
    -V, --version    Print version information

Expected Behaviour

test-clap 

USAGE:
    test-clap <ARG>

ARGS:
    <ARG>    

OPTIONS:
    -h, --help    Print help information

Additional Context

With the vocabulary we are providing users, we are flattening an Args. This says nothing about flattening an App as well.

This has also led to a series of bugs like #2527. We've worked around this by carefully setting the precedence (https://github.com/TeXitoi/structopt/pull/325, #2531 and #2819) which has lead to other bugs like #2785 which required more precedence tweaks (#2882).

By distinguishing what app settings are tightly associated with flattening (help_heading, the implicit subcommand required else help) and making everything else only apply when creating an App, we can simplify, clarify, and make less brittle these relationships.

Debug Output

No response

epage commented 2 years ago

Comment by epage Saturday Oct 16, 2021 at 15:05 GMT

Note: we can't just move the app method calls to IntoApp because that will break subcommands because Args and Subcommand are also implicitly sub-parsers in subcommands and we expect them to be able to initialize the subcommand's app settings.

Options

Have Args and Subcommand also derive IntoApp
Have users explicitly derive IntoApp when they want to be used as a subcommand
Split the Args and Subcommand augment_app methods into augment_app and add_args.

epage commented 2 years ago

Comment by epage Tuesday Nov 02, 2021 at 17:57 GMT

In thinking about this, it might make sense for subcommand to pull in some appsettings. We already implicitly add one we'd need to keep and the user might want to do other related ones like UseLongFormatForHelpSubcommand, InferSubcommands,

epage / clapng