Allow groups of groups

[epage]

A blocker for #4697 which is important for #3123 and #2621 is the ability to have a group own another group.

API-wise, I suspect we'd change ArgGroup::arg to ArgGroup::member or ArgGroup::child to generalize it for taking either an arg or a group ID.

• To meet our compatibility goals,. this "change" would be a deprecation and a new function.
  • I'd recommend doing this with a PR that (1) adds the new function, (2) updates existing uses to the new function, (3) adds the deprecation notice
  • We might need to soft-deprecate it and not apply the attribute because a true deprecation must be done on a point release

To unblock this, we'd need to update the debug assertions to make sure the Id is a valid Arg or Group Id.

There is also the validation work

[ysndr]

I've been looking into this a bit more.

Are you sure there are still "debug assertions to make sure the Id is a valid Arg or Group Id" in the project?

Grepping for debug_assert didn't yield anything looking close to that.
The assertions i found are apparently only checking for Id but don;t assert what that Id represents: [1].
Also it seems to check arg in group.args against both arg_ids and group_ids: [2]
That is to say, I dont think "groups of groups" are particularly disallowed.

Variable naming is a bit biased, "arg" is everywhere around those methods even if it is explicitly checked against group_ids as well.

Renaming the method(s) on ArgGroup is relatively straight forward (sans all the doc tests..), i figure this should actually go further a bit, also changing "arg" at the usage side to "member".
Since i'm particularly interested in the conflict side tho i might work on that first before hunting down umpteen "arg" variables that should be "member" instead.

[1] https://github.com/ysndr/clap/blob/6539ab81c4858e051dfd2d76a984faec75c7b88d/clap_builder/src/builder/debug_asserts.rs#L307-L327

[2] https://github.com/ysndr/clap/blob/6539ab81c4858e051dfd2d76a984faec75c7b88d/clap_builder/src/builder/debug_asserts.rs#L290-L305

Edit:
Turns out the handling for groups in [1] were prior changes of mine that snuck into the renaming commit.

[epage]

I'm a bit confused. You pointed at code in debug_asserts.rs in 6539ab8 to show that child groups are checked but for 6539ab8 github is reporting "This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.".

If I look in

clap/clap_builder/src/builder/debug_asserts.rs

Lines 290 to 299 in 64e3790

	for arg in &group.args {
	// Args listed inside groups should exist
	assert!(
	cmd.get_arguments().any(|x| x.get_id() == arg),
	"Command {}: Argument group '{}' contains non-existent argument '{}'",
	cmd.get_name(),
	group.get_id(),
	arg
	);
	}

which is the latest commit for master
https://github.com/clap-rs/clap/blob/master/clap_builder/src/builder/debug_asserts.rs#L290-L299
that code is not there.

This matches the behavior you saw in #5700 (comment)

[ysndr]

Oh my that happens when you start off with existing work and mess up your rebase.

So those lines 290 - 300 were my changes from investigating the original subgroups approach.