-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
Use enum as subcommands in a subcommand #2005
Comments
OK, got it. Some (modified) code from discussions that works: #[derive(Clap)]
pub struct App {
#[clap(long = "verbose")]
verbose: bool,
// ...
#[clap(subcommand)]
subcommand: Subcommand,
}
#[derive(Clap)]
pub enum Subcommand {
Config(Config)
// ...
}
// THIS "SHIM" STRUCT IS MANDATORY
#[derive(Clap)]
pub struct Config {
#[clap(subcommand)]
subcommand: ConfigSubcommand
}
#[derive(Clap, Debug)]
pub enum ConfigSubcommand {
Show,
Edit,
} The thing is, OP wants to omit the shim struct and use the enum directly: #[derive(Clap)]
pub struct App {
#[clap(long = "verbose")]
verbose: bool,
// ...
#[clap(subcommand)]
subcommand: Subcommand,
}
#[derive(Clap)]
pub enum Subcommand {
// NO SHIMS INVOLVED
Config(Config)
// ...
}
#[derive(Clap)]
enum Config {
Show,
Edit,
} That kind of makes sense. But it doesn't work because of this. I'll be working on it once I'm done with my current backlog. |
It used to work on structopt, right? How can we fix this? |
I believe the best way is to change the syntax: enum Foo {
// no attr = flatten, backcompat
Foo(Ext),
// tell derive that ExtFlat is struct and it must be flattened
#[clap(flatten)]
Bar(ExtFlat),
// tell derive that ExtSub is enum and it must be used as subcommands set
#[clap(subcommand)]
Bar(ExtSub),
// synonym for current #[structopt(flatten)]
// see https://github.com/TeXitoi/structopt/issues/327
#[clap(subcommand(flatten))]
Zog(ExtSubFlat)
} |
Your snippet is not exactly giving me the full context. But I can wait for the PR to review it properly. |
Something like this works for me without the extra struct #[derive(Clap)]
pub enum Subcommand {
// NO SHIMS INVOLVED
Config {
#[clap(subcommand)]
sub: Config
}
// ...
} |
My current thinking is that we should be able to get this working without the need for any additional attributes. |
Looking to catch up on this. It sounds we had an undefined case (enum flattened into struct) and a "questionable" case (enum nested in enum) and #1681 removed. I say "questionable" because @CreepySkeleton raised concerns in #1681 but now we are here with a request for it to work. @CreepySkeleton's concerns were:
The known/expected case for nested subcommands is to have a struct inbetween with a #1681 also included a recommendation for handling of the attributes and that has been brought up in this thread as well:
|
I'm going to do an exercise in re-deriving (no pun intended) how all of this works, ignoring implementation. The default case for struct fields is that they are arguments: #[derive(Clap)]
struct Parent {
field: Arg
}
#[derive(Clap)]
struct Parent {
field: Arg,
#[clap(flatten)]
delegate: Child,
}
#[derive(Clap)]
struct Child {
parent_field: Arg
} The default case for variants is that they are sub-commands and any contained value are arguments for that subcommand: #[derive(Clap)]
enum Subcommands {
First {
field: Arg
},
} What gets interesting is the tuple case: #[derive(Clap)]
enum Subcommands {
First(Child)
}
#[derive(Clap)]
struct Child {
field: Arg
} Strictly speaking, I know From that perspective, an So I'd change the previous recommended behavior to:
|
I agree. I do want to point out that we need to allow explicit |
In fact, all of these cases work except for a non flattened enum inside enum which is what this issue is. |
Yes, my intent was to to highlight the proposed principle for how all of these should behave in contrast to the other proposal. |
That PR auto-linked this issue since you had the word |
For completeness sake, I wanted to compare what compiles for every case for #!/usr/bin/env -S cargo eval --
//! ```cargo
//! [package]
//! edition = "2018"
//! [dependencies]
//! structopt = "*"
//! ```
#[derive(structopt::StructOpt)]
struct StructOfArgs {
// Compile error: the trait bound `ChildArgs: FromStr` is not satisfied
//implicit_args: ChildArgs,
#[structopt(flatten)]
flatten_args: ChildArgs,
#[structopt(subcommand)]
subcommand_args: ChildArgs,
}
#[derive(structopt::StructOpt)]
struct StructOfENums {
// Compile error: the trait bound `Subcommand: FromStr` is not satisfied
//implicit_subcmds: Subcommand,
// Compile error: does not impl `ArgGroup`
#[structopt(flatten)]
flatten_subcmds: Subcommand,
#[structopt(subcommand)]
subcmmand_subcmds: Subcommand,
}
#[derive(structopt::StructOpt)]
enum EnumOfStructs {
ImplicitArgs(ChildArgs),
#[structopt(flatten)]
FlattenArgs(ChildArgs),
// Compile error: subcommand is only allowed on fields
//#[structopt(subcommand)]
//SubcommandArgs(ChildArgs),
}
#[derive(structopt::StructOpt)]
enum EnumOfEnums {
ImplicitSubcmds(Subcommand),
#[structopt(flatten)]
FlattenSubcmds(Subcommand),
// Compile error: subcommand is only allowed on fields
//#[structopt(subcommand)]
//SubcmmandSubcmds(Subcommand),
}
#[derive(structopt::StructOpt)]
struct ChildArgs {
}
#[derive(structopt::StructOpt)]
enum Subcommand {
}
fn main() {
println!("Hello world");
} |
And clap today (note: this doesn't capture what cases panic) #!/usr/bin/env -S cargo eval --
//! ```cargo
//! [package]
//! edition = "2018"
//! [dependencies]
//! clap = "3.0.0-beta.2"
//! ```
#[derive(clap::Clap)]
struct StructOfArgs {
// Compile error: the trait bound `ChildArgs: FromStr` is not satisfied
//implicit_args: ChildArgs,
#[clap(flatten)]
flatten_args: ChildArgs,
/* Compile error
error[E0277]: the trait bound `ChildArgs: clap::Subcommand` is not satisfied
--> structopt.rs:8:10
|
8 | #[derive(clap::Clap)]
| ^^^^^^^^^^ the trait `clap::Subcommand` is not implemented for `ChildArgs`
|
= note: required by `augment_subcommands`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0277]: the trait bound `ChildArgs: clap::Subcommand` is not satisfied
--> structopt.rs:16:10
|
16 | #[clap(subcommand)]
| ^^^^^^^^^^ the trait `clap::Subcommand` is not implemented for `ChildArgs`
|
= note: required by `from_subcommand`
*/
//#[clap(subcommand)]
//subcommand_args: ChildArgs,
}
#[derive(clap::Clap)]
struct StructOfENums {
// Compile error: the trait bound `Subcommand: FromStr` is not satisfied
//implicit_subcmds: Subcommand,
#[clap(flatten)]
flatten_subcmds: Subcommand,
#[clap(subcommand)]
subcmmand_subcmds: Subcommand,
}
#[derive(clap::Clap)]
enum EnumOfStructs {
ImplicitArgs(ChildArgs),
/* Compile errors:
error[E0277]: the trait bound `ChildArgs: clap::Subcommand` is not satisfied
--> structopt.rs:33:10
|
33 | #[derive(clap::Clap)]
| ^^^^^^^^^^ the trait `clap::Subcommand` is not implemented for `ChildArgs`
|
= note: required by `augment_subcommands`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0277]: the trait bound `ChildArgs: clap::Subcommand` is not satisfied
--> structopt.rs:33:10
|
33 | #[derive(clap::Clap)]
| ^^^^^^^^^^ the trait `clap::Subcommand` is not implemented for `ChildArgs`
|
= note: required by `from_subcommand`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
*/
//#[clap(flatten)]
//FlattenArgs(ChildArgs),
// Compile error: subcommand is only allowed on fields
//#[clap(subcommand)]
//SubcommandArgs(ChildArgs),
}
#[derive(clap::Clap)]
enum EnumOfEnums {
ImplicitSubcmds(Subcommand),
#[clap(flatten)]
FlattenSubcmds(Subcommand),
// Compile error: subcommand is only allowed on fields
//#[clap(subcommand)]
//SubcmmandSubcmds(Subcommand),
}
#[derive(clap::Clap)]
struct ChildArgs {
}
#[derive(clap::Clap)]
enum Subcommand {
}
fn main() {
println!("Hello world");
} |
This is mostly me thinking aloud to help save the information and in case anyone else has thoughts.
I think the original intent in #1681 was to make these distinct by having a separate This leads us to a situation where there are no compile time errors and, without Possible solutions: Go back to the original behaviorThis requires
With this option, Instead of the original solution for this, my thought is we'd change the traits so:
The advantage of this route is the we isolate all arg group / subcommand knowledge to the trait implementer rather than the caller having to know some. The disadvantage is this doesn't scale with other features but we don't have a clear enough idea of what those are, so YAGNI We could go all in on this and merge Go all in on #1681I feel #1681 only went part way. It created a The division of labor would be
In this case, I would Other notes
And a general observation is that merging ProposalI'm changing my thought on my last proposal to enum Foo {
// no attr, arguments to `foo`
Foo(ChildArgs),
// Shortcut syntax
FooAlt {
#[clap(subcommand)]
sub: Config
}
// Be explicit, subcommands to `bar-alt`
#[clap(subcommand)]
BarAlt(ChildCommands),
// Collapse `DelegatedCommands` into `Foo`
#[clap(flatten)]
Baz(DelegatedCommands),
} The matrix will then be:
This will be done with trait IntoApp: Sized {
fn into_app<'help>(update: bool) -> App<'help>;
}
trait FromArgMatches: Sized {
fn from_arg_matches(matches: &ArgMatches) -> Option<Self>;
fn update_from_arg_matches(&mut self, matches: &ArgMatches);
}
// Also derives FromArgMatches
trait Args: FromArgMatches + Sized {
fn append_args(app: App<'_>, update: bool) -> App<'_>;
}
// Also derives FromArgMatches
trait Subcommand: FromArgMatches + Sized {
fn append_subcmmand(app: App<'_>, update: bool) -> App<'_>;
}
|
In having 1-3 top-level traits, I think it could make sense to give |
With clap-rs#2005, we are looking at having 3 primary derive traits, `Clap`, `Subcommand`, and `Args`. `Clap` isn't very descriptive, so changing it to `Parser` since that is the primary role of that trait, with all of its `*parse*` functions.
With clap-rs#2005, we are looking at having 3 primary derive traits, `Clap`, `Subcommand`, and `Args`. `Clap` isn't very descriptive, so changing it to `Parser` since that is the primary role of that trait, with all of its `*parse*` functions.
With clap-rs#2005, we are looking at having 3 primary derive traits, `Clap`, `Subcommand`, and `Args`. `Clap` isn't very descriptive, so changing it to `Parser` since that is the primary role of that trait, with all of its `*parse*` functions.
I would prefer to go all in on #1681. There were quite a few reasons in previous issues regarding this which happened before I even learnt rust. I completely agree with your proposed matrix.
Can you please expand on that sentence? That said, I strongly disagree with asking the users to derive either Of course, the user always have the flexibility of deriving only those traits. |
They can't derive them all currently. Today, a user would
My proposal is that
The user can then optionally choose to derive Most of this falls out of the work I'm doing for this issue.
Every time I've interacted with a crate with a derive that provides functionality beyond deriving the trait that I named, I've been confused and feel uncertainty on how to use it. In addition, this is giving your types that don't make sense. There is never a time a user would be wanting an Personally, I can go either way on One case for why emphasizing Besides making other traits top-level, part of my suggestion for renaming |
👍
That sounds good to me. I must have misunderstood your original proposal. I still don't see a strong reason on the rename and would prefer
I am okay with
I think this is already the way we do things. If you see
I would say implementing your proposal for the traits would be part of the issue fix here. I think both of us are aligned (except for the rename). Please feel free to go ahead with the implementation. I would prefer if you can send small PRs but I don't mind if that can't be done. |
Before, partial command lines would panic at runtime. Now it'll be a compile error For example: ``` pub enum Opt { Daemon(DaemonCommand), } pub enum DaemonCommand { Start, Stop, } ``` Gives: ``` error[E0277]: the trait bound `DaemonCommand: clap::Args` is not satisfied --> clap_derive/tests/subcommands.rs:297:16 | 297 | Daemon(DaemonCommand), | ^^^^^^^^^^^^^ the trait `clap::Args` is not implemented for `DaemonCommand` | = note: required by `augment_args` ``` To nest this, you currently need `enum -> struct -> enum`. A later change will make it so you can use the `subcommand` attribute within enums to cover this case. This is a part of clap-rs#2005
`structopt` originally allowed ``` pub enum Opt { Daemon(DaemonCommand), } pub enum DaemonCommand { Start, Stop, } ``` This was partially broken in clap-rs#1681 where `$ cmd daemon start` works but `cmd daemon`, panics. Originally, `structopt` relied on exposing the implementation details of a derived type by providing a `is_subcommand` option, so we'd know whether to provide `SubcommandRequiredElseHelp` or not. This was removed in clap-rs#1681 Fixes clap-rs#2005
Before, partial command lines would panic at runtime. Now it'll be a compile error For example: ``` pub enum Opt { Daemon(DaemonCommand), } pub enum DaemonCommand { Start, Stop, } ``` Gives: ``` error[E0277]: the trait bound `DaemonCommand: clap::Args` is not satisfied --> clap_derive/tests/subcommands.rs:297:16 | 297 | Daemon(DaemonCommand), | ^^^^^^^^^^^^^ the trait `clap::Args` is not implemented for `DaemonCommand` | = note: required by `augment_args` ``` To nest this, you currently need `enum -> struct -> enum`. A later change will make it so you can use the `subcommand` attribute within enums to cover this case. This is a part of clap-rs#2005
`structopt` originally allowed ``` pub enum Opt { Daemon(DaemonCommand), } pub enum DaemonCommand { Start, Stop, } ``` This was partially broken in clap-rs#1681 where `$ cmd daemon start` works but `cmd daemon`, panics. Originally, `structopt` relied on exposing the implementation details of a derived type by providing a `is_subcommand` option, so we'd know whether to provide `SubcommandRequiredElseHelp` or not. This was removed in clap-rs#1681 Fixes clap-rs#2005
Before, partial command lines would panic at runtime. Now it'll be a compile error For example: ``` pub enum Opt { Daemon(DaemonCommand), } pub enum DaemonCommand { Start, Stop, } ``` Gives: ``` error[E0277]: the trait bound `DaemonCommand: clap::Args` is not satisfied --> clap_derive/tests/subcommands.rs:297:16 | 297 | Daemon(DaemonCommand), | ^^^^^^^^^^^^^ the trait `clap::Args` is not implemented for `DaemonCommand` | = note: required by `augment_args` ``` To nest this, you currently need `enum -> struct -> enum`. A later change will make it so you can use the `subcommand` attribute within enums to cover this case. This is a part of clap-rs#2005
`structopt` originally allowed ``` pub enum Opt { Daemon(DaemonCommand), } pub enum DaemonCommand { Start, Stop, } ``` This was partially broken in clap-rs#1681 where `$ cmd daemon start` works but `cmd daemon`, panics. Originally, `structopt` relied on exposing the implementation details of a derived type by providing a `is_subcommand` option, so we'd know whether to provide `SubcommandRequiredElseHelp` or not. This was removed in clap-rs#1681 Fixes clap-rs#2005
Before, partial command lines would panic at runtime. Now it'll be a compile error For example: ``` pub enum Opt { Daemon(DaemonCommand), } pub enum DaemonCommand { Start, Stop, } ``` Gives: ``` error[E0277]: the trait bound `DaemonCommand: clap::Args` is not satisfied --> clap_derive/tests/subcommands.rs:297:16 | 297 | Daemon(DaemonCommand), | ^^^^^^^^^^^^^ the trait `clap::Args` is not implemented for `DaemonCommand` | = note: required by `augment_args` ``` To nest this, you currently need `enum -> struct -> enum`. A later change will make it so you can use the `subcommand` attribute within enums to cover this case. This is a part of clap-rs#2005
Before, partial command lines would panic at runtime. Now it'll be a compile error For example: ``` pub enum Opt { Daemon(DaemonCommand), } pub enum DaemonCommand { Start, Stop, } ``` Gives: ``` error[E0277]: the trait bound `DaemonCommand: clap::Args` is not satisfied --> clap_derive/tests/subcommands.rs:297:16 | 297 | Daemon(DaemonCommand), | ^^^^^^^^^^^^^ the trait `clap::Args` is not implemented for `DaemonCommand` | = note: required by `augment_args` ``` To nest this, you currently need `enum -> struct -> enum`. A later change will make it so you can use the `subcommand` attribute within enums to cover this case. This is a part of clap-rs#2005
`structopt` originally allowed ``` pub enum Opt { Daemon(DaemonCommand), } pub enum DaemonCommand { Start, Stop, } ``` This was partially broken in clap-rs#1681 where `$ cmd daemon start` works but `cmd daemon`, panics. Originally, `structopt` relied on exposing the implementation details of a derived type by providing a `is_subcommand` option, so we'd know whether to provide `SubcommandRequiredElseHelp` or not. This was removed in clap-rs#1681 Fixes clap-rs#2005
`structopt` originally allowed ``` pub enum Opt { Daemon(DaemonCommand), } pub enum DaemonCommand { Start, Stop, } ``` This was partially broken in clap-rs#1681 where `$ cmd daemon start` works but `cmd daemon`, panics. Originally, `structopt` relied on exposing the implementation details of a derived type by providing a `is_subcommand` option, so we'd know whether to provide `SubcommandRequiredElseHelp` or not. This was removed in clap-rs#1681 Fixes clap-rs#2005
With clap-rs#2005, we are looking at having 3 primary derive traits, `Clap`, `Subcommand`, and `Args`. `Clap` isn't very descriptive, so changing it to `Parser` since that is the primary role of that trait, with all of its `*parse*` functions.
Before clap-rs#2005, `Clap` was a special trait that derived all clap traits it detected were relevant (including an enum getting both `ArgEnum`, `Clap`, and `Subcommand`). Now, we have elevated `Clap`, `Args`, `Subcommand`, and `ArgEnum` to be user facing but the name `Clap` isn't very descriptive. This also helps further clarify the relationships so a crate providing an item to be `#[clap(flatten)]` or `#[clap(subcommand)]` is more likely to choose the needed trait to derive. Also, my proposed fix fo clap-rs#2785 includes making `App` attributes almost exclusively for `Clap`. Clarifying the names/roles will help communicate this. For prior discussion, see clap-rs#2583
Before clap-rs#2005, `Clap` was a special trait that derived all clap traits it detected were relevant (including an enum getting both `ArgEnum`, `Clap`, and `Subcommand`). Now, we have elevated `Clap`, `Args`, `Subcommand`, and `ArgEnum` to be user facing but the name `Clap` isn't very descriptive. This also helps further clarify the relationships so a crate providing an item to be `#[clap(flatten)]` or `#[clap(subcommand)]` is more likely to choose the needed trait to derive. Also, my proposed fix fo clap-rs#2785 includes making `App` attributes almost exclusively for `Clap`. Clarifying the names/roles will help communicate this. For prior discussion, see clap-rs#2583
Before clap-rs#2005, `Clap` was a special trait that derived all clap traits it detected were relevant (including an enum getting both `ArgEnum`, `Clap`, and `Subcommand`). Now, we have elevated `Clap`, `Args`, `Subcommand`, and `ArgEnum` to be user facing but the name `Clap` isn't very descriptive. This also helps further clarify the relationships so a crate providing an item to be `#[clap(flatten)]` or `#[clap(subcommand)]` is more likely to choose the needed trait to derive. Also, my proposed fix fo clap-rs#2785 includes making `App` attributes almost exclusively for `Clap`. Clarifying the names/roles will help communicate this. For prior discussion, see clap-rs#2583
Before clap-rs#2005, `Clap` was a special trait that derived all clap traits it detected were relevant (including an enum getting both `ArgEnum`, `Clap`, and `Subcommand`). Now, we have elevated `Clap`, `Args`, `Subcommand`, and `ArgEnum` to be user facing but the name `Clap` isn't very descriptive. This also helps further clarify the relationships so a crate providing an item to be `#[clap(flatten)]` or `#[clap(subcommand)]` is more likely to choose the needed trait to derive. Also, my proposed fix fo clap-rs#2785 includes making `App` attributes almost exclusively for `Clap`. Clarifying the names/roles will help communicate this. For prior discussion, see clap-rs#2583
Make sure you completed the following tasks
Code
Output
Steps to reproduce the issue
cargo run --
Version
Actual Behavior Summary
When deriving
Clap
for enums as shown in the code, calling any of theparse
methods while not giving a secondary subcommand to primary subcommand causes a panic.Expected Behavior Summary
I think that instead of panicking, clap should return an error displaying the help message.
Additional context
I have already opened a discussion about this at #2004, because I was unsure if this behavior was unexpected.
@CreepySkeleton agrees that this is a bug.
Debug output
Debug Output
The text was updated successfully, but these errors were encountered: