fix(derive): Ensure App help_heading is applied #2882
Conversation
We normally set all app attributes at the end. This can be changed but will require some work to ensure - Top-level item's doc cmment ins our over flattened - We still support `Args` / `Subcommand` be used to initialize an `App` when creating a subcommand In the mean time, this special cases `help_heading` to happen first. We'll need this special casing anyways to address clap-rs#2803 since we'll need to capture the old help heading before addings args and then restore it after. I guess we could unconditionally do that but its extra work / boilerplate for when people have to dig into their what the derives do. Fixes clap-rs#2785
|
I know I've not given much time for people to review this (especially since derives can be a bit rougher to review) but the high level goal is not controversial and I have some other changes I'm wanting to get this in tonight to get several other changes blocked on this in tomorrow morning. bors r+ |
|
Build succeeded: |
|
I have not reviewed this because I wanted to see if there's a way we can avoid doing it like this. I didn't like the fact that we divided these methods into first and last. With the recent fixes to help_heading, wouldn't applying all the methods first (like it was before) fix it? Don't think this PR is really needed. |
For now, we need to apply the methods last so that when we flatten, the higher level struct's app settings win (#2531). In parallel, I've been working towards changing how we model app settings and would split those out so that a flattened struct will never impact the higher level struct. That is a bigger, more experimental of a change that I think is a lower priority for 3.0 than this change and wanted to make progress on this bug rather than block a fix on that experiment. |
To confirm, that is tracked in #2894, right? |
| #( #subcommands )*; | ||
| #app_var #app_methods | ||
| #app_var #final_app_methods |
There was a problem hiding this comment.
We don't have tests for some of the changes in these files.
Now it is, yes. |
We normally set all app attributes at the end. This can be changed but
will require some work to ensure
Args/Subcommandbe used to initialize anAppwhencreating a subcommand
In the mean time, this special cases
help_headingto happen first.We'll need this special casing anyways to address #2803 since we'll need
to capture the old help heading before addings args and then restore it
after. I guess we could unconditionally do that but its extra work /
boilerplate for when people have to dig into their what the derives do.
Fixes #2785