Structopt: version = expr ignored when using subcommands

[wez]

This sounds similar to https://github.com/TeXitoi/structopt/issues/283 but manifests a little differently.

#[derive(StructOpt)]
#[structopt(version = my_version())]
struct Opt {
    #[structopt(subcommand)]
    cmd: Option<SubCommand>,
}

#[derive(StructOpt)]
struct StartCommand {
}

#[derive(StructOpt)]
enum SubCommand {
   Start(StartCommand),
}

generates code along the lines of:

impl StructOptInternal for Opt {
  fn augment_clap(app) {
      let app = app.version("1.0").version(my_version());
      let app = <SubCommand>::augment_clap(app);
      app
   }
}

impl StructOptInternal for Start {
   fn augment_clap(app) {
      // this overrides the version set on the top level, which is the bug
      let app = app.version("1.0");
      // ...
      app
}

The workaround is to add something like:

#[structopt(no_version, version = my_version())]

on everthing for which the StructOpt derive is used.

[CreepySkeleton]

I really regret the decision to generate version() automatically when not asked, unlike author and about. It's third bug because of that.

[wez]

I was going to suggest only setting it when explicitly referenced, but I haven't been following the changes since 0.2 closely enough to understand the background.

[CreepySkeleton]

You mean, generate .version() call only when explicitly requested by #[structopt(version)]? We were considering it in #217 but we ended up with no_version instead, and I wholeheartedly regret this decision. I'll do my best to change in in clap_derive but, for structopt, it would be a breaking change.

[TeXitoi]

That's a breaking change.

For the memory: https://github.com/TeXitoi/structopt/issues/217

[wez]

My expectation would be that if I didn't explicitly use #[structopt(version)] then I would get a default version for the top level struct only, with the others leaving it unspecified.

I believe that that is the behavior in structopt 0.2 which is what I was attempting to upgrade from; from my perspective 0.2 -> 0.3 is already a breaking change, so I have no problem with a fix being an additional breaking change--it would make it less of a breaking change for me :)

[wez]

FWIW, I'm also totally fine with not setting the default version at all, but that's because I prefer to show versions based off the commit information for executables. So long as overriding the version is not overly laborious, I don't have a strong opinion on what the default behavior is.

[CreepySkeleton]

My expectation would be that if I didn't explicitly use #[structopt(version)] then I would get a default version for the top level struct only, with the others leaving it unspecified.

The question here, how to detect if the struct is top level or not? StructOpt uses only one trait for all the stuff: top level structs (apps), flattened structs, and subcommands.

It would be much easier to develop and support if they were separate traits, like StructOpt, StructFlatten, StructSubcmd or whatever, and this would also allow us to confidently say "yes, this is top level struct since StructOpt is used only on top level structs". But, unfortunately, this is a breaking change too.

(Not to mention that this would simplify the implementing code significantly).

I'm looking forward to change it in clap_derive, we'll see then.

[TeXitoi]

That would be more complicated for the user.

[CreepySkeleton]

I believe that that is the behavior in structopt 0.2 which is what I was attempting to upgrade from

Actually, it was a longstanding bug in 0.2, which was fixed only recently.

The bug in a nutshell - top level attributes (including the auto-generated .version() call) on inner structs (flattened, subcommands) had been ignored up until then.

[CreepySkeleton]

That would be more complicated for the user.

What's complicated with separate traits? It would be like:

if you want to use the struct with flatten, it must derive StructFlatten.

if you want to use a struct wit subcommand, it must derive StructSubcommand

The top level app must derive StructOpt.

Pretty simple, huh?

[TeXitoi]

You'll be surprised by the bug reports that would generate such a change.

[CreepySkeleton]

Er, I'm not proposing to change structopt, heavens no. I'm just thinking aloud about "what clap_derive should look like".

[TeXitoi]

Of course, just don't underestimate the simplicity of your interface, and don't forget that you are biased as a contributor.