Re: [PATCH 10/21] tree_entry_interesting(): fix depth limit with overlapping pathspecs

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

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Suppose we have two pathspecs 'a' and 'a/b' (both are dirs) and depth
> limit 1. In current code, pathspecs are checked in input order. When
> 'a/b' is checked against pathspec 'a', it fails depth limit and
> therefore is excluded, although it should match 'a/b' pathspec.
>
> This patch reorders all pathspecs alphabetically, then teaches
> tree_entry_interesting() to check against the deepest pathspec first,
> so depth limit of a shallower pathspec won't affect a deeper one.

I am quite happy to see where this new round of this series is going so
far.  I however doubt this patch is a right approach for the problem you
are solving, especially because your longer-term (i.e. toward the rest of
the series to patch 21) plan is to allow wildcards [*1*].

One thing I am not clear is what it means to limit the recursion level
when you have wildcards.

One possible definition of interaction between limit and wildcard may be
to count the number of slashes in the part of the path that matches the
wildcarded part of the pathspec, add the number of path components
appended due to the leading directory match, and then subtract the number
of literal slashes in the wildcarded part of the pattern from the above,
and declare that a match is found if the difference is less than the
limit.

E.g. a pathspec element "a/*/x" would match "a/b/c/x", "a/b/c/d/e/x",
"a/b/x/y" and "a/b/x/y/z" without limit, and with the limit of 1:

    a/b/c/x        matches ('*' expands to "b/c")
    a/b/c/d/e/x    no ('*' has to expand to "c/d/e" and needs 2 levels)
    a/b/x/y        matches ('*' expands to "b" costing zero, "/y" needs 1)
    a/b/x/y/z      does not match

Another definition could be to count _only_ the part that is appended by
recursion (i.e. we do not count how many slashes has to match '*' in the
above examples), and as the option is called --depth, it might make more
sense.

In either case, I am not sure if "if it matches the longest pathspec, we
have the answer without looking at shorter ones" would be a good rule to
use.


[Footnote]

*1* In addition, perhaps you may later want to introduce some "negative"
match operators to pathspecs; while I am not particularly fond of that
direction at this moment, I would like to leave the door open for that
possibility, in case it turns out to be a good thing to have.