feat: Builder macro to assist with App/Arg/Group/SubCommand building

[WildCryptoFox]

#217

[Vinatorul]

I am the last fan of macros, but it looks very cool to me! Thank you!
Let's wait until @kbknapp check this.
And have you planned to add integration test for this feature?

[WildCryptoFox]

Due to the dynamics of the macro literally translating to the builder. The example is in itself, a test case. Compilation is the only thing that can really be tested. Past that, any tests are actually testing the builder itself - not the macro.

---

@Vinatorul "I am the last fan of macros" did you mean you are "now a fan of macros", don't like, or don't yet understand them ? ;-)

[WildCryptoFox]

Additional note: The examples/18_builder_macro.rs should reach every macro pattern matching arm. If not; that is a bug in the example and I'm sorry. :-)

Any other possible tests would be implementation stress tests. I.e. An app with 64 arguments will not compile.

However crazy, such SHOULD be broken down into subcommands at that point and the consumer should repeat over their list of subcommands - which if still more than 64 subcommands... They can always implement a macro like this one and then it's only limited to the AST token tree of rustc)

---

Alternatively the consumer may also just go with the cheats way out with #![recursion_limit = "128"] to double the recursion limit. The correct method is to just write sane code

[Vinatorul]

The example is in itself, a test case.

Yes it tests that macro builds but it can't claim that in works.

I am the last fan of macros

I meant don't like, there was a lot of problems in C++ with macros. In rust macros are pretty better, but I still have some mistrusts to them :)

[WildCryptoFox]

@Vinatorul I'm not against a test case, but now we're limited to the input that is given to the macro. The complex example covers all features of the macro and I've manually verified the generated code.

Bad input to the macro will generate bad code. This will cause a compile time error.

• Typo in an identifier used for setting booleans, iterating over lists or running functions will cause the expected compile time error.
• Failing to meet the requirements of the macro's matching arms will cause a headache and require the consumer to do the normal macro debugging. -Z trace-macros is the most suitable for this context.

[WildCryptoFox]

Only testcases that come to mind is to use the macro to generate an App to some variable, then do an equality test against the manually written builder. Again, anything deeper is testing the capabilities of the builder itself, not the macro.

[Vinatorul]

@james-darkfox This test can save once live when developer made copy-paste or misspell error, for example:

 (@arg ($arg:expr) $modes:tt #{$n:expr, $m:expr} $($tail:tt)*) => {
        clap_app!{ @arg ($arg) $modes min_values($n) min_values($m) $($tail)* }
 };

[WildCryptoFox]

You successfully got me re-checking my code and generated code. =D

Again... Such suitable test would need to be an equality test against two instances of App, one built manually and one aided with the macro.

[Vinatorul]

Maybe I don't understand something, but why can't it be like this one, but with checks: https://github.com/kbknapp/clap-rs/blob/master/tests/yaml.rs

[WildCryptoFox]

I have a feeling that test gets optimized out to nothing anyway... Just typechecks.

Nothing is stopping a test from existing. My original point was simply, that the example code stresses all features of the macro; and has been manually verified to generate the correct code.

To programmatically confirm the integrity of the generated builder, is to simply check equality against a manually built and equivalent instance of App.

[Vinatorul]

Yes, example code stresses all features of the macro.
I meant test not for current macro, but test for future changes to avoid regression caused by programmer misspel or copy-paste or smth like that.
May be I'll implement it sometime 😉

---

Travis build finally ended, so let's wait for @kbknapp approvement.

[WildCryptoFox]

Future regression is indeed very important. I'm unsure without diving back into the code, if App implements Eq; which if it does, the test would just need to compare the generated builder by the macro and the known-good state of the builder as proven here.

[WildCryptoFox]

Also. As an extract from the checklist on #217

• Future proof macro design.
  • App/@subcommand depend on: with_name arg_group subcommand setting arg.
  • @arg depends on: with_name long short takes_value required multiple value_name min_values max_values.
  • @group depends on: with_name add.

---

Sidenote, I feel that App and Subcommand have some things that need to be separated (I.e. subcommands don't have authors, settings, and possibly other things).

[kbknapp]

Looks great, I'm excited for this merge!

@james-darkfox could you add a commit adding yourself to the CONTRIBUTORS.md and this'll be ready for merge. We can also do it after the fact, but this gives you a chance to put whatever info you want in there.

Sidenote, I feel that App and Subcommand have some things that need to be separated (I.e. subcommands don't have authors, settings, and possibly other things).

Yes, and no. "Yes" in that they typically don't. "No" in that they can have authors and settings and such (think third party cargo subcommands, but on steroids...i.e. all inside the single cargo application which helps with call times. Ever notice it takes a good minute to call a third part subcommand while it searches your $PATH?

The settings piece is a little more subtle. While normally it doesn't matter, there can be times when it's nice to give a subcommand the ability to have different settings from the parent commands or application in general (such as the need for mandatory args or further subcommands).
The settings

[kbknapp]

@james-darkfox not sure which timezone you're in, so I'll give you a bit to put in the contributors commit. After a few hours, if nothing myself or @Vinatorul will merge and we'll just add your name in the next PR to update the versions and readme 😉

[WildCryptoFox]

As for that subcommand search; no I haven't noticed such delay. I also think that should be settings gated as the consumer would opt for this if they wanted such subcommands from third parties.

My timezone is AEST:Eastern Australia

[kbknapp]

Perhaps, but nothing is lost leaving those settings un-gated except maybe compile time and polluting of API space? As for the API space, I'd actually prefer leaving it un-gated so all possible options are displayed in the docs without confusing consumers.

I would say those settings are there if they need they want them, but aren't by any means required. If there is a detriment to leaving them un-gated I'm open for suggestions :)

As for this PR, I'm happy to say Merged!