Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Derive doc clap ordering for multiple Clap #2531

Merged
merged 1 commit into from
Aug 12, 2021

Conversation

pickfire
Copy link
Contributor

Fix #2527

Copy link
Member

@pksunkara pksunkara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write a separate test and test the full help output instead of this partial assertion in another test.

@pickfire pickfire force-pushed the fix-flatten branch 2 times, most recently from 1c1cf5a to b0fabd4 Compare June 10, 2021 15:44
@pickfire
Copy link
Contributor Author

@pksunkara Fixed

@epage
Copy link
Member

epage commented Jul 21, 2021

btw I think this will break GlobalVersion because it has to be set before everything else

Personally, I would recommend we move all app settings set via attributes to be in IntoApp only, not allowing inheriting them through flatten. I personally find that confusing that pulling in args is also pulling in settings.

@pksunkara
Copy link
Member

Can you please expand on what you mean? I am a little bit confused.

@epage
Copy link
Member

epage commented Aug 11, 2021

Can you please expand on what you mean? I am a little bit confused.

When we derive Args, we put all settings in augment_app rather than into_app, so when an Args is flattened into an Args, settings are applied from both with the last flattened Args winning. This PR changes the evaluation order.

Example: Originally clap_cargo::Features had a doc string saying how to flatten it into your Args. A structopt update came out and suddenly its developer-focused doc-string started winning, being exposed in --help and I had to remove it so the real about would get passed to clap and then the user.

imo we should instead move some of these to into_app and not allow inheriting certain settings. This is especially true with clap3 where clap_cargo will be updated to derive Args and not Clap. In this case, conceptually, an Args shouldn't be providing application settings (subcommand handling is an example exception and is an artifact of clap's design)

@pksunkara pksunkara added this to the 3.0 milestone Aug 11, 2021
@pksunkara
Copy link
Member

Can you please create an issue with an example failure and proposal on how to fix it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#[clap(flatten)] takes top-level docstring from the flattened struct, not the struct being flattened into
3 participants