-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
docs: Add subcommand comment #4692
docs: Add subcommand comment #4692
Conversation
examples/cargo-example-derive.rs
Outdated
@@ -4,6 +4,7 @@ use clap::Parser; | |||
#[command(name = "cargo")] | |||
#[command(bin_name = "cargo")] | |||
enum Cargo { | |||
#[command(name = "example-derive")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like overriding the auto-generated name with the same value doesn't help clarify but can also confuse.
I also feel like this example is the wrong place to be teaching how to override a subcommand name. This is more focused on how to integrate with cargo but this change is more about how to use the attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the end of the day I’m looking for something that nudges people toward getting a working cargo sub command. I want to save others the time I spent staring at my code confused.
Part of that process is understanding how to get the command name they want. It’s unclear (to me) what comes from convention versus what comes from configuration.
Having the base Cli given an explicit name of “cargo” hints to me that there are ways to override defaults. I just couldn’t figure out where to do that.
If you don’t like the renaming how about a comment hinting where the other value is coming from.
Maybe something like:
// Subcommand “example-derive” generated from variant name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated with a comment and renamed the args struct to better differentiate where the subcommand name comes from.
Discussed in comment clap-rs#4692 (comment) this adds a comment explaining where the subcommand name "example-derive" comes from. I also modified the `ExampleDerive` struct to be `ExampleDeriveArgs`. This means the variant and its struct no longer have the same name and that it's clearer that the struct contains the args.
b7ebc69
to
28a7d73
Compare
Discussed in comment clap-rs#4692 (comment) on_r1095242457 this adds a comment explaining where the subcommand name "example-derive" comes from. I also modified the `ExampleDerive` struct to be `ExampleDeriveArgs`. Th is means the variant and its struct no longer have the same name and tha t it's clearer that the struct contains the args.
28a7d73
to
2e46d0c
Compare
I've gone ahead and applied the |
Thanks a ton! I appreciate the extra work of changing the struct name in the examples across the board. I think that will help. I still would like a little more of a hint for some just-in-time learning regarding that convention, but certainly understand wanting to limit scope creep and keep examples focused. ❤️🥳💯 |
Discussed in comment #4692 (comment) this adds a comment explaining where the subcommand name "example-derive" comes from.
I also modified the
ExampleDerive
struct to beExampleDeriveArgs
. This means the variant and its struct no longer have the same name and that it's clearer that the struct contains the args.