Re: [PATCH 0/7] submodule groups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> I'd suggest not to over-engineer this.  Go back and imagine how
> "/bin/ls" would work is a good sanity check to gauge what complexity
> levels ordinary users would feel comfortable to handle.
>
> "ls a b" would give union of what "ls a" and "ls b" would output,
> there is no negation, and the users won't die from having to say "ls
> [A-Za-qs-z]*" to exclude files whose names begin with 'r'.

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)".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]