Re: [PATCH 0/7] submodule groups

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

Having said all that, there is one thing we may want to consider.

For negative pathspecs, we deliberately defined its semantics to "see if the path matches with any positive pathspec element (and it is a non-match if no positive one matches), and then exclude if it matches with any negative pathspec element".

That is, when you are matching 'x.o' against three-element pathspec 

	'*.c' ':(exclude)*.o' 'x.?'

you do not say "x.o does not match *.c, but it matches *.o so it is to be excluded, ah, wait, x.? matches to revive it so it is a match". Instead "*.c does not and x.? does match, but *.o matches so it is excluded". IOW, the order of the pathspec elements given on the command line does not matter.

Now we are adding two different criteria to specify inclusion based on labels and names.

As implemented in the patch series under discussion, we are saying that a submodule is included if

    - its path matches the pathspec (using the "matches one or more
      positive pathspec elements, and does not match any negaative
      pathspec element" as the definition of "matches"), OR

    - it is included in the specified set of *labels, OR

    - its name is specified by :name

There is no reason not to consider future enhancements to specify exclusion base on these two new criretia. A naive and easier to implement enhancement would be to rewrite the above to (the latter two item changes):

    - its path matches the pathspec (using the "matches one or more
      positive pathspec elements, and does not match any negaative
      pathspec element" as the definition of "matches"), OR

    - it is included in the specified set of *labels, and does not
      have any excluded *!labels, OR

    - its name is specified by :name, and is not among any excluded
      :!name.

but it does not have to be that way. I suspect that it may make it easier to understand and use if we OR'ed together only the positive ones from three classes of criteria together and require at least one of them to match, and then requiring none of the negative ones to match. That is, a module-spec with three elements:

     'sub*' '*label0' './:(exclude)*0'

with the implemented algorithm would choose submodules whose name begins with 'sub' except the ones whose name ends with '0', OR those with label0, but if we redefine the behaviour to "positive ones together, and then exclude negative ones", then we would choose ones whose name begins with 'sub' or ones with label0, and among these, exclude those whose name ends with '0' (which is what your "test" wanted to express).

The implementation of match_pathspec() does the "first positive ones only and then negative ones" two-step process already, so I think you could update its "int is_dir" argument to "unsigned flags", introduce a "negative only" flag, and then do something like:

        for each path
                if (!(match_pathspec(ps, name, ..., 0) ||
                      has a "positive" label specified ||
                      has its name specified as a "postiive")
                        /* does not match any positive criteria */
                        continue;
                if (match_pathspec(ps, name, ..., NEGATIVE_ONLY) ||
                    has a "negative" label specified ||
                    has its name specified as a "negative")
                        /* explicitly excluded */
                        continue;
                /* included! */

That would open door to something like

     'sub*' '*label0' './:(exclude)*0' '*!label1'

to allow you to say "(those with label0 or whose path begins with sub) minus (those with label1 or whose path ends with 0)".