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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 16, 2010, 23:31 UTC
Message-ID: <7vmxo5l2g4.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1292425376-14550-11-git-send-email-pclouds@gmail.com>

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

Show 8 quoted lines

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