On Thu, Feb 2, 2012 at 9:25 PM, Albert Yale <surfingalbert@xxxxxxxxx> wrote:
> I added a "struct pathspec_set" as you suggested
> in your previous review. It had the side effect
> of forcing me to update a few more files than was
> previously necessary.
Please make it a separate patch, it's hard to follow an all-in-one
patch. Although those changes might be unnecessary (see below).
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -566,6 +566,10 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
> while (tree_entry(tree, &entry)) {
> int te_len = tree_entry_len(&entry);
>
> + if (!match_pathspec_depth(pathspec,
> + entry.path, strlen(entry.path),
> + 0, NULL))
> + continue;
> if (match != all_entries_interesting) {
> match = tree_entry_interesting(&entry, base, tn_len, pathspec);
> if (match == all_entries_not_interesting)
tree_entry_interesting() is equivalent to match_pathspec_depth(). The
only difference is that the former is designed to match on trees why
the latter a list. And tree_entry_interesting() is more efficient than
match_pathspec_depth(). As you can see there's
tree_entry_interesting() call above already. You should make the
tree_entry_interesting() understand exclude pathspec instead of adding
match_pathspec_depth() in.
> @@ -295,6 +298,25 @@ int match_pathspec_depth(const struct pathspec *ps,
> return retval;
> }
>
> +int match_pathspec_depth(const struct pathspec *ps,
> + const char *name, int namelen,
> + int prefix, char *seen)
> +{
> + int retval = match_pathspec_set_depth(&ps->include,
> + ps->recursive, ps->max_depth,
> + name, namelen, prefix, seen);
> +
> + if (retval && ps->exclude.nr)
> + {
> + if (match_pathspec_set_depth(&ps->exclude,
> + ps->recursive, ps->max_depth,
> + name, namelen, prefix, seen))
> + return 0;
> + }
> +
> + return retval;
> +}
> +
> static int no_wildcard(const char *string)
> {
> return string[strcspn(string, "*?[{\\")] == '\0';
It makes me wonder, why not add match_pathspec_with_exclusion(const
struct pathspec *include_ps, const struct pathspec *exclude_ps,...),
use the new function in grep.c and revert struct pathspec back to
original? The same can be applied to tree_entry_interesting() (i.e.
add a new one that takes two pathspec sets, which supports exclusion)
I think you may make less changes that way. I'm bad at naming. It's up
to you to rename match_pathspec_with_exclusion to something meaningful
and short enough.
--
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