Flatten on enums

[cecton]

This is probably wrong but it works!

Fixes #327

[CreepySkeleton]

Thanks for showing interest!

Your current approach is a no go. We don't want black magic in structopt so we are not accepting code relying on private implementation details of other crates. We consider #[doc(hidden)] pub private, even though clap v2 branch is kind of frozen; just for the sake of all the holy beauty in the universe. We don't want to desecrate this beauty, do we?

But fear not (OK, maybe fear a little)! There's a way to implement it safe and totally sound.

structopt uses dynamic dispatch for inter-maco-expansion communication, better seen with an example

// this is the first derive-macro expansion
// but not in a sense it gets expanded first. 
// the expansion order is undefined, can't rely on it! 
#[derive(StructOpt)] 
struct Opt {
    // here we need to know which fields `FlattenedStruct`
    // contains so we could generate corresponding `app.arg()` calls
    // (and others). But we can't pass this info from there on
    // compile time since the expansion order is undefined :(
    // What are we gonna do? Dynamic dispatch to rescue!
    //
    // There's the hidden `StructOptInternal` trait that contains 
    // a number of such dispatching methods, `augment_clap` is among them.
    #[structopt(flatten)]
    field: FlattenedStruct
}

// the second expansion
#[derive(StructOpt)]
struct FlattenedStruct {
    opt: String
}

// This is what the FIRST derive will generate

impl StructOpt for Opt {
    fn clap() -> App {
        // create "bare" app
        let app = App::new("name");
        // augment it by calling `App::*` methods
        <Opt as StructoptInternal>::augment_clap(app)
    }

    fn from_clap(matches: &ArgMatches) -> Self {
        Opt {
            // we simply delegate to the flattened type **at runtime**!
            // this is what flattening is, in essence. 
            field: <FlattenedStruct as StructoptInternal>::from_clap(matches)
        }
    }
}


impl StructOptInternal for Opt {
    fn augment_clap(app: App) -> App {
        // We delegate to `FlattenedStruct::augment_clap`
        // so it will augment the app for us!
        <FlattenedStruct as StructoptInternal>::augment_clap(app)
    }

    // other stuff, who cares about it...
}

// This is what the SECOND derive will generate

impl StructOpt for FlattenedStruct {
    // the same as above 
    fn clap() -> App {
        let app = App::new("name");
        <Opt as StructoptInternal>::augment_clap(app)
    }

    // we delegated to it from `Opt`
    fn from_clap(matches: &ArgMatches) -> Self {
        Opt {
            opt: // the code extracting it from `matches`
        }
    }
}

impl StructOptInternal for Opt {
    fn augment_clap(app: App) -> App {
        app.arg(Arg::with_name("opt")) 
        /* other opts/fields */
    }

    // other stuff, who cares about it...
}

You want to tweak augment_clap for enums that is generated at
https://github.com/TeXitoi/structopt/blob/master/structopt-derive/src/lib.rs#L465..L481

You also want to tweak from_subcommand generation at
https://github.com/TeXitoi/structopt/blob/master/structopt-derive/src/lib.rs#L535..L549
it takes role of the constructor for enums.

You also want (I must admit you're a person with a lot of wants) to add a test in tests/subcommnds.rs to make sure it works as intended.

The last thing you will need is to document the usege in src/lib.rs, somewhere in #Subcommands section. I know, nobody wants it, but this is necessary.

Any questions?

[cecton]

@CreepySkeleton OK with all 😁 I was kinda expecting it's no good.

Just one small question: in all your explanations, did you keep in mind that the structopt BaseCli and ExtendedCli (see #327) are in 2 different crates? It's very important.

[CreepySkeleton]

@cecton It doesn't matter what crate they from as long as they both derive(StructOpt). Dynamic dispatch doesn't care about it, AFAIK, feel free to correct me.

And both of this crates must depend on the same version of StructOpt, it's very important :)

[cecton]

@CreepySkeleton what I wish I could do is:

Instead of calling augment_clap() on a subcommand freshly created, I would call augment_clap() on app (given in argument) instead.

impl ::structopt::StructOptInternal for GroupCli {
    fn augment_clap<'a, 'b>(app: ::structopt::clap::App<'a, 'b>) -> ::structopt::clap::App<'a, 'b> {
        app.subcommand({
            let subcommand = ::structopt::clap::SubCommand::with_name("base-cli");
            let subcommand = {
                let kind = "FlattenStruct";
                let subcommand =
                    <BaseCli as ::structopt::StructOptInternal>::augment_clap(subcommand); // <------- HERE
                if <BaseCli as ::structopt::StructOptInternal>::is_subcommand() {
                    subcommand.setting(::structopt::clap::AppSettings::SubcommandRequiredElseHelp)
                } else {
                    subcommand
                }
            };
            subcommand.version("0.1.0")
        })
        .subcommand({
            let subcommand = ::structopt::clap::SubCommand::with_name("command3");
            let subcommand = {
                let kind = "other";
                let subcommand =
                    <Command3 as ::structopt::StructOptInternal>::augment_clap(subcommand);
                if <Command3 as ::structopt::StructOptInternal>::is_subcommand() {
                    subcommand.setting(::structopt::clap::AppSettings::SubcommandRequiredElseHelp)
                } else {
                    subcommand
                }
            };
            subcommand.version("0.1.0")
        })
        .version("0.1.0")
    }

But I don't think this is possible because .subcommand() moves self and return a new App with the new subcommands.

The only way I think it would be possible is to change this code:

    quote! {
        fn augment_clap<'a, 'b>(
            app: ::structopt::clap::App<'a, 'b>
        ) -> ::structopt::clap::App<'a, 'b> {
            app #app_methods #( #subcommands )* #version
        }
    }

And add at the end #augments that would be a list of augments to apply on app.

What do you think?

[CreepySkeleton]

@cecton I think your current approach is what stalling you here.

Current implementation generates a chain of .subcommand().subcommand() //... calls. What you're trying to do is putting the augment_clap() delegation somewhere inside this invocation, but augment_clap appends (potentially) multiple subcommands rather than only one. It can't live inside a .subcommand() call.

(I admit the previous subcommands() hack would work, but still no way we're accepting this 😋)

What you want to do is to generate either .subcommand() call (for normal subcommands) or augment_clap() call (for flattened subcommands) on every iteration.

I recommend you to abandon the .subcommand().subcommand() approach here and generate this instead:

impl ::structopt::StructOptInternal for GroupCli {
    fn augment_clap<'a, 'b>(app: ::structopt::clap::App<'a, 'b>) -> ::structopt::clap::App<'a, 'b> {
        // normal subcommand
        let app = app.subcommand({
            // already implemented, no need to touch
        });
 
        // flattened subcommand
        let app = <Sybty as StructOptInternal>::augment_clap(app);
     
        // and so on
    }

[cecton]

@CreepySkeleton I think I did it! It's probably still wrong at other levels and needs quite a clean-up but it works!

impl ::structopt::StructOptInternal for GroupCli {
    fn augment_clap<'a, 'b>(app: ::structopt::clap::App<'a, 'b>) -> ::structopt::clap::App<'a, 'b> {
        let app = app
            .subcommand({
                let subcommand = ::structopt::clap::SubCommand::with_name("command3");
                let subcommand = {
                    let kind = "other";
                    let subcommand =
                        <Command3 as ::structopt::StructOptInternal>::augment_clap(subcommand);
                    if <Command3 as ::structopt::StructOptInternal>::is_subcommand() {
                        subcommand
                            .setting(::structopt::clap::AppSettings::SubcommandRequiredElseHelp)
                    } else {
                        subcommand
                    }
                };
                subcommand.version("0.1.0")
            })
            .version("0.1.0");
        let app = <BaseCli as ::structopt::StructOptInternal>::augment_clap(app);
        app
    }
    fn from_subcommand<'a, 'b>(
        sub: (&'b str, Option<&'b ::structopt::clap::ArgMatches<'a>>),
    ) -> Option<Self> {
        if let Some(res) = <BaseCli as ::structopt::StructOptInternal>::from_subcommand(sub) {
            return Some(GroupCli::BaseCli(res));
        };
        match sub {
            ("command3", Some(matches)) => {
                let kind = "other";
                Some(GroupCli::Command3(
                    <Command3 as ::structopt::StructOpt>::from_clap(matches),
                ))
            }
            _ => None,
        }
    }
    fn is_subcommand() -> bool {
        true
    }
}

[CreepySkeleton]

@cecton Congrats! The first time is the most exciting one (in programming at least)!

I would like to highly encourage you to add a test so you can see if it works for youself.

[cecton]

And what do I do with the failing test? Is there a way to distinguish a struct from an enum?

[cecton]

I would like to highly encourage you to add a test so you can see if it works for youself.

Of course. I added one already. I think only the documentation is missing at this point.

[CreepySkeleton]

Yes, and adjust the .stderr file

[CreepySkeleton]

And add an subentry to the table of contents

[CreepySkeleton]

You need cargo fmt --all

[CreepySkeleton]

hey @TeXitoi mind to take a look?

[CreepySkeleton]

Fuck, I should have waited for this to get merged before merging my own PR.

OK, once @TeXitoi has it reviewed, I'll deal with the conflicts myself as part of my apologies to @cecton , sorry 😢

[TeXitoi]

Great changes! Only minor remarks.

Thanks @cecton for your contribution.

Thanks @CreepySkeleton for this great review.