Subject: Re: [PATCH] submodule--helper.c: add only-active to foreach

From: Shourya Shukla <[email-masked]>
Date: Sun, 10 May 2020 22:14:24 +0530

Hey Guillaume,

> On repository with some submodule not active, it could be needed to run
> a command only for active submodule. Today it can be achived with the
> command:

Spelling: achive -> achieve

Maybe we can keep the commit message a bit more imperative?
Something like:
-------------------------
On a repository with some submodules not active, one may need to run a
command only for an active submodule or vice-versa. To achieve this,
one may use:
git submodule foreach 'git -C $toplevel submodule--helper is-active \
$sm_path && pwd || :'

Simplify this expression to make it more readable and easy-to-use by
adding the flag `--is-active` to subcommand `foreach` of `git
submodule`. Thus, simplifying the above command to:
git submodule--helper foreach --is-active pwd
-------------------------
Yes, maybe renaming the flag to `--is-active` would make it a tad bit
simpler? This commit message may not be perfect but it seems like an
improvement over the previous one?

To me this option seems good. It may have some good utility in the
future. Similarly, we may change the struct to:
	struct foreach_cb {
 	const char *prefix;
 	int quiet;
 	int recursive;
	int is_active;
	 };

Therefore, the if-statement becomes:
	if (info->is_active && !is_submodule_active(the_repository, path))
		return;

BTW what do we return here, could you please be more specific?
Again, the change here as well:
		OPT_BOOL(0, "is-active", &info.is_active,

Here, too:
		N_("git submodule--helper foreach [--quiet] [--recursive] [--is-active] [--] <command>"),

And,
	test_expect_success 'test "submodule--helper foreach --is-active" usage' '

Finally,
	git submodule--helper foreach --is-active "echo \$toplevel-\$name-\$path-\$sha1" > ../actual

What do you think?

Regards,
Shourya Shukla