Re: [PATCH 5/7] submodule--helper module_list_compute: allow label or name arguments

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Stefan Beller <sbeller@google.com> writes:
>
>> +static void split_argv_pathspec_groups(int argc, const char **argv,
>> +				       const char ***pathspec_argv,
>> +				       struct string_list *group)
>> +{
>> +	int i;
>> +	struct argv_array ps = ARGV_ARRAY_INIT;
>> +	for (i = 0; i < argc; i++) {
>> +		if (starts_with(argv[i], "*")
>> +		    || starts_with(argv[i], ":")) {
>> +			string_list_insert(group, argv[i]);
>> +		} else if (starts_with(argv[i], "./")) {
>> +			argv_array_push(&ps, argv[i]);
>
> I'd suggest stripping "./" when you do this.  That is, make the rule
> to be "*label is a label, :name is a name, and everything else is a
> path.  You can prefix ./ to an unfortunate path that begins with an
> asterisk or a colon and we will strip ./ disambiguator".

To clarify, "./$path and $path are the same so why strip it?" is an
understandable, even though naive, question. The reason is because
you do not want to contaminate the code that parses pathspecs with
the knowledge of this submodule-group specific prefix.  "./$path"
and "$path" may be equivalent for a literal pathspec, but I'd want
to see user be able to say

   git submodule foreach './:(icase)foo'

and find Foo, foo, FOO, etc.

I would also recommend to strip '*' and ':' from group names and
module names, and maintain two separate lists.  You would eventually
want to do "*default*" to name all groups whose name matches with
the pattern "default*", as if it were a pathspec matched against the
available group names, and that will keep the door open for future
extension like  "*:(icase)default*".