clap
clap copied to clipboard
clap-rs
derive: flatten on Option fields
I have a usecase where a bunch of structures are used by both serde_derive and structopt_derive in order to source cli-options and file-options in a uniform way. It works pretty well with #[structopt(flatten)] and other annotations, except when trying to flatten Option<_> fields.
That is, a simplified example is as following:
#[macro_use]
extern crate serde_derive;
#[macro_use]
extern crate structopt_derive;
#[derive(Deserialize, StructOpt)]
struct TopLevel {
#[structopt(flatten)]
section_a: Option<SectionA>,
}
#[derive(Deserialize, StructOpt)]
struct SectionA {
#[structopt(long = "section_a.field" )]
field: Option<String>,
}
This fails with:
the trait `structopt::StructOpt` is not implemented for `std::option::Option<SectionA>`
Approaching this from my own crate, I think I can't solve this alone due to orphan rules (both the StructOpt trait and the Option receiver are not originating in the crate). Thus I tried to add a blanket impl<T: StructOpt> StructOpt for Option<T: StructOpt> in structopt; that pushed the issue a bit forward, but then I ended up stuck in proc-macro logic.
@TeXitoi I'd like to know if you think it generally makes sense for structopt to take care of the Option case above, and if so adapting the derive logic to handle that.
The problem is "what does it mean?"
Here, we have one field that is an option. Should structopt create a TopLevel { section_a: None } or a TopLevel { section_a: Some(SectionA { field: None }) }?
If we have 2 String fields, should the 2 fields be optional, but if you have one, you must have 2?
What about nested flatten?
I just encountered this problem too, and I think my case is more clearcut:
I have an optional filter argument and I want to add an additional second filter that can only be added if the first filter is set. I tried to represent this as following:
struct Params {
// more fields ...
#[structopt(flatten)
range_filter: Option<RangeFilter>
}
#[derive(StructOpt)
struct RangeFilter {
#[structopt(short = "b")
bottom: String,
#[structopt(short = "t")
top: Option<String>
}
@TimoFreiberg How do you expect the top (-t) option should be handled? What this double-Option would mean?
I guess this should work for you just fine
struct Params {
#[structopt(flatten)]
range_filter: RangeFilter
}
#[derive(StructOpt)]
struct RangeFilter {
#[structopt(short = "b")]
bottom: Option<String>,
#[structopt(short = "t")]
top: Option<String>
}
top should not be set unless bottom is set. I handled it similar to your suggestion now, but I panic when only top is set.
@TimoFreiberg You can use clap::Arg::requires instead of panic:
#[derive(StructOpt)]
struct RangeFilter {
#[structopt(short = "b")]
bottom: Option<String>,
#[structopt(short = "t", requires = "bottom")]
top: Option<String>
}
The problem is "what does it mean?"
Here, we have one field that is an option. Should structopt create a
TopLevel { section_a: None }or aTopLevel { section_a: Some(SectionA { field: None }) }?
I propose to implement the first design variant since it would allow us to handle nested flatten quite naturally:
#[derive(StructOpt)]
struct TopLevel {
#[structopt(flatten)]
section_a: Option<SectionA>,
}
#[derive(StructOpt)]
struct SectionA {
#[structopt(flatten)]
inner_sect: Option<InnerSection>,
#[structopt(long = "section_a.field")]
field: String,
}
#[derive(StructOpt)]
struct InnerSection {
#[structopt(long = "inner_section.field")]
field: String,
}
-
app %no args%=>TopLevel { section_a: None } -
// app --section_a.field arg TopLevel { section_a: Some(SectionA { field: "arg", inner_sect: None }) } -
// app --section_a.field arg --inner_section.flag inner TopLevel { section_a: Some(SectionA { field: "arg", inner_sect: Some( InnerSection { field: "inner }) }) }
cc @TeXitoi do you agree?
I'm afraid that implementing this will be very complicated to code, maintain and understand who to use.
In my case:
Given:
external/command1.rs:
#[derive(StructOpt, Debug, Serialize, Deserialize)]
pub struct Command1 {
// Some arguments
}
external/command2.rs:
#[derive(StructOpt, Debug, Serialize, Deserialize)]
pub struct Command2 {
// Some arguments
}
Need:
mycrate/command.rs:
#[derive(StructOpt, Debug, Serialize, Deserialize)]
pub struct MyCommand {
#[serde(flatten)]
#[structopt(flatten)]
com1: Command1,
#[serde(flatten)]
#[structopt(flatten, required_if("arg1"))]
com2: Option<Command2>,
#[structopt(short = "a", long)]
arg1: bool,
}
In other words, in structure MyCommand arguments from Command1 required always, but arguments from Command2 required only if arg1 appears (i.e. true).
Of course we can create two separate structures for two options (with arg1 and without it), but it not consistent with DRY.
But good solution may be use flatten between our structures:
#[derive(StructOpt, Debug, Serialize, Deserialize)]
pub struct MyCommand {
#[serde(flatten)]
#[structopt(flatten)]
com1: Command1,
}
#[derive(StructOpt, Debug, Serialize, Deserialize)]
pub struct MyCommandWithArg1 {
#[serde(flatten)]
#[structopt(flatten)]
my_com: MyCommand,
#[serde(flatten)]
#[structopt(flatten)]
com2: Command2,
}
In this case we can reuse functionality of MyCommand in MyCommandWithArg1.
The problem is "what does it mean?"
My two cents: maybe match whatever serde does? The feature request opened with "I have a usecase where a bunch of structures are used by both serde_derive and structopt_derive", and consistency would help with that. I haven't verified serde has reasonable behavior, but off-hand I don't see any reason the two should behave differently.
fwiw, I ended up here after trying a simpler thing:
#[derive(StructOpt)]
struct Credentials {
#[structopt(long)]
username: String,
#[structopt(long)]
password: String,
}
#[derive(StructOpt)]
struct Opt {
// ...
#[structopt(flatten)]
credentials: Option<Credentials>,
// ...
}
which I'd expect to have similar behavior as the following, with a more structured representation.
#[derive(StructOpt)]
struct Opt {
// ...
#[structopt(long, requires="password")]
username: Option<String>,
#[structopt(long, requires="username")]
password: Option<String>,
// ...
}
Not a must-have; it'd just make my code more readable and avoid having an unreachable error case later on:
let credentials = match (opt.username, opt.password) {
(Some(username), Some(password)) => Some(Credentials {
username,
password,
}),
(None, None) => None,
_ => unreachable!(), // prevented by structopt requires
};
That would be really tricky to do this kind of things.
Since this was discussed, update_from_args was added that makes all fields optional which has the appearance of allowing this to be implemented without requiring extra cross-derive communication that tends to be the bane of derive feature requests (see pretty much any A-derive issue). #3707 experimented with this.
- In general, I feel hesitant of what corner cases might be lurking because that API call was not designed with that purpose in mind and there is a lot of nuanced behavior to
update_from_args. - It is derive-only behavior so the rest of clap doesn't know, like help rendering
- We'd need to make sure it works in all contexts, including when flattening other structs, pulling in subcommands, etc.