Nguyán ThÃi Ngác Duy <pclouds@xxxxxxxxx> writes:
> +static int read_tree_1(struct tree *tree, struct strbuf *base,
> + int stage, struct pathspec *pathspecs,
> + read_tree_fn_t fn, void *context)
Micronit: call the variable pathspec, not pathspecs.
An instance of the structure contains a specification to limit the set of
paths to operate on as a set of pathspec elements, and this single set is
collectively called a "struct pathspec"; you are not passing an array that
has one or more pathspecs in it here (i.e. pathspecs[2] in this function
does not make sense).
It looks like the result is easier to follow, perhaps mostly thanks to the
(re)use of strbuf instead of allocating a new path when going deeper.
> @@ -108,10 +60,22 @@ int read_tree_recursive(struct tree *tree,
> init_tree_desc(&desc, tree->buffer, tree->size);
>
> while (tree_entry(&desc, &entry)) {
> - if (!match_tree_entry(base, baselen, entry.path, entry.mode, match))
> + if (all_interesting)
> + retval = all_interesting > 0;
> + else {
> + retval = tree_entry_interesting(&entry, base, 0, pathspecs);
> + if (retval == 2)
> + all_interesting = 1;
> + else if (retval == -1) {
> + all_interesting = -1;
> + retval = 0;
> + }
> + }
> + if (!retval)
> continue;
This "if all-interesting then avoid calling into an expensive matcher"
logic looks familiar but is not something that came from the parts deleted
by this patch. Where did I see it? Is that something we can share in a
common helper function? Is such a sharing worth pursuing, considering that
the above is a reasonably trivial logic in a tight loop?
"That's how the return value of tree-entry-interesting is designed to be
used, and it is unsurprising that all the callers will fall into that
pattern" is a perfectly acceptable answer, I guess.
Thanks.
--
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