2010/12/14 Junio C Hamano <gitster@xxxxxxxxx>:
>> +int within_depth(const char *name, int namelen,
>> + Â Â Â Â Â Â Â Â Â Â int depth, int max_depth)
>> +{
>> + Â Â const char *cp = name, *cpe = name + namelen;
>> +
>> + Â Â while (cp < cpe) {
>> + Â Â Â Â Â Â if (*cp++ != '/')
>> + Â Â Â Â Â Â Â Â Â Â continue;
>> + Â Â Â Â Â Â depth++;
>> + Â Â Â Â Â Â if (depth > max_depth)
>> + Â Â Â Â Â Â Â Â Â Â return 0;
>> + Â Â }
>> + Â Â return 1;
>> +}
>
> Makes me almost suspect that it may make more sense to keep track of the
> "depth" in a similar way as "base" and "baselen" as "traversal state" on
> the side of the caller so that you do not have to scan the string for
> slashes over and over again. ÂBut given the codeflow, doing so might make
> the result look too ugly, so I won't recommend that without thinking,
> though.
Moreover, this function is also used by match_pathspec_depth().
>> - Â Â if (!ps || !ps->nr)
>> + Â Â if (!ps)
>> Â Â Â Â Â Â Â return 1;
>>
>> + Â Â if (!ps->nr) {
>> + Â Â Â Â Â Â if (!ps->recursive || ps->max_depth == -1)
>> + Â Â Â Â Â Â Â Â Â Â return 1;
>> + Â Â Â Â Â Â return !!within_depth(base, baselen,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â !!S_ISDIR(entry->mode),
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ps->max_depth);
>> + Â Â }
>
> This gives different behaviour to between callers that give you NULL as
> pathspec and callers that give you a pathspec with zero elements. ÂIs this
> intended? ÂWhat is the use case (iow, what does an end user give from the
> command line to experience this difference)?
Old pathspec type's legacy. When pathspec is of "const char **", it
can be NULL. When struct pathspec is used, I don't think we need to
support NULL pathspec. Will remove that "if (!ps)" clause.
>> @@ -571,7 +580,13 @@ int tree_entry_interesting(const struct name_entry *entry,
>> Â Â Â Â Â Â Â Â Â Â Â if (!match_dir_prefix(base, baselen, match, matchlen))
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â /* Just a random prefix match */
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â continue;
>> - Â Â Â Â Â Â Â Â Â Â return 2;
>> +
>> + Â Â Â Â Â Â Â Â Â Â if (!ps->recursive || ps->max_depth == -1)
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â return 2;
>> +
>> + Â Â Â Â Â Â Â Â Â Â return !!within_depth(base+matchlen+1, baselen-matchlen-1,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â !!S_ISDIR(entry->mode),
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ps->max_depth);
>> Â Â Â Â Â Â Â }
>
> If two pathspecs that overlap with each other (e.g. "Documentation/" and
> "Documentation/technical") are given, and if the shorter one comes before
> the longer one in ps[], wouldn't this give you an unexpected result? ÂWhen
> inspecting "Documentation/technical/api/foo.txt" with depth limit of 2, if
> you didn't have "Documentation/" pathspec, you count "api/foo.txt"
> relative to "Documentation/technical", declare that the path is within
> limit, and show it. ÂBut if you have "Documentation/" in ps[], you look at
> it, decide "technical/api/foo.txt" is too deep and return false without
> even looking at "Documentation/technical" that may appear later in ps[],
> no?
Right. Hmm.. grep's pathspec_matches() probably has this problem too.
--
Duy
--
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