Skip to content
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

Nested updates implemented have bugs around subcommands #2605

Closed
epage opened this issue Jul 19, 2021 Discussed in #2591 · 0 comments
Closed

Nested updates implemented have bugs around subcommands #2605

epage opened this issue Jul 19, 2021 Discussed in #2591 · 0 comments
Labels
A-derive Area: #[derive]` macro API
Milestone

Comments

@epage
Copy link
Member

epage commented Jul 19, 2021

Discussed in #2591

Originally posted by epage July 14, 2021
This was implemented in #1878 for #1837 based on #1836.

I've not dig into all of #1878's comments but at one point the thought was:

We don't know which fields the flattened struct consists of, so we delegate to the flattened type itself - it must know how to update itself.

This isn't always the case. From inspection

  • SubCmd{ field: i32 } generates an augment update
  • SubCmd(Args) is hard coded to call augment
  • #[flatten] Subcommand(MoreCmds) is hard coded to call augment
  • #[subcmmand] field: SubCmd calls augment update
  • #[flatten] field: MoreArgs is hard coded to call augment

Example generated code from clap_derive/tests/flatten.rs where there is a mismatch in augment vs from

        fn augment_clap_for_update<'b>(app: clap::App<'b>) -> clap::App<'b> {
            {
                let app = app;
                let app = <Common as clap::IntoApp>::augment_clap(app);
                app
            }
        }
...
        fn update_from_arg_matches(&mut self, arg_matches: &clap::ArgMatches) {
            {
                #[allow(non_snake_case)]
                let common = &mut self.common;
                clap::FromArgMatches::update_from_arg_matches(common, arg_matches);
            }
        }
```</div>
epage added a commit to epage/clap that referenced this issue Jul 19, 2021
Before, when doing an `update` involving subcommands, we generated
parsing rules for the `from` case instead, requiring all arguments to be
present.

This switches us to descending into `update` code and adds tests to
verify it works.

This is a part of clap-rs#2605
@pksunkara pksunkara added this to the 3.0 milestone Jul 25, 2021
@pksunkara pksunkara added the A-derive Area: #[derive]` macro API label Jul 25, 2021
epage added a commit to epage/clap that referenced this issue Jul 26, 2021
When using `#[clap(subcommand)]` inside of a Subcommand, we didn't
properly do an `update` but a `from`.

This is a part of clap-rs#2605
@epage epage closed this as completed Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-derive Area: #[derive]` macro API
Projects
None yet
Development

No branches or pull requests

2 participants