Re: [PATCH 1/3] traverse_trees(): allow pruning with pathspec

On Wed, Aug 31, 2011 at 12:44 AM, Junio C Hamano [off-list ref] wrote:

Nguyen Thai Ngoc Duy [off-list ref] writes:

quoted

quoted

@@ -376,16 +396,22 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)

                       mask |= 1ul << i;
                       if (S_ISDIR(entry[i].mode))
                               dirmask |= 1ul << i;
+                       e = &entry[i];
               }

Why? "e" is not used in that loop or anywhere after that.

This is trying to find _a_ surviving entry to be fed to prune_traversal()
which in turn uses tree_entry_interesting(). At this point in the code, we
are stuffing the entries of the same name from the input trees (and if one
tree is missing an entry of the chosen name, it will have NULL there), so
any non-empty entry would do. It corresponds to "first" but that is just a
simple string and not a name_entry tree_entry_interesting() wants.

Ah yes. I only searched in old code, "e" is used in the new
prune_traversal() call.

quoted

quoted

               if (!mask)
                       break;
-               ret = info->fn(n, mask, dirmask, entry, info);
-               if (ret < 0) {
-                       error = ret;
-                       if (!info->show_all_errors)
-                               break;
+               interesting = prune_traversal(e, info, &base, interesting);
+               if (interesting < 0)
+                       break;

I don't really understand this function to comment. But I guess when
interesting < 0, we only skip info->fn() and assume it returns "mask"
(its user unpack_callback() only returns either "mask" or -1).

We consume the entries we have used in merging (which is actually
"everything in entry[] array" as info->fn() returns "mask" itself) by
saying "update_extended_entry()" and the purpose of doing so is to prepare
to process the next entry of the tree we are traversing.

When tree_entry_interesting() returns negative, it tells us "no, and no
subsequent entries will be either", meaning "we are done with this tree".
As we are done, there is nothing to prepare for the next round; we are not
walking the remaining entries in the trees we are looking at. Is there any
point in calling update_extended_entry() I am missing?

No you're right again. Somehow I thought there would be another round
in the loop (ie. continue, not break). My bad.
-- 
Duy