Re: [PATCH v4 6/6] list-objects-filter: implement filter tree:0

On Tue, Aug 14, 2018 at 1:01 PM Jeff King [off-list ref] wrote:

On Tue, Aug 14, 2018 at 10:28:13AM -0700, Matthew DeVore wrote:

quoted

The name "tree:0" allows later filtering based on depth, i.e. "tree:1"
would filter out all but the root tree and blobs. In order to avoid
confusion between 0 and capital O, the documentation was worded in a
somewhat round-about way that also hints at this future improvement to
the feature.

I'm OK with this as a name, since we're explicitly not supporting deeper
depths. But I'd note that "depth" is actually a tricky characteristic,
as it's not a property of the object itself, but rather who refers to
it. So:

  - it's expensive to compute, because you have to actually walk all of
    the possible commits and trees that could refer to it. This
    prohibits a lot of other optimizations like reachability bitmaps
    (though with some complexity you could cache the depths, too).

I think what the user likely wants is to use the minimum depth based
on the commits in the traversal, not every commit in the repo - is
this what you mean?

  - you have to define it as something like "the minimum depth at which
    this object is found", since there may be multiple depths

I think you can read that second definition between the lines of:

quoted

+The form '--filter=tree:<depth>' omits all blobs and trees deeper than
+<depth> from the root tree. Currently, only <depth>=0 is supported.

But I wonder if we should be more precise. It doesn't matter now, but it
may help set expectations if the feature does come later.

Makes sense. I changed it like this -

diff --git a/Documentation/rev-list-options.txt

b/Documentation/rev-list-options.txt
index 0b5f77ad3..5f1672913 100644

--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt

@@ -732,8 +732,10 @@ the requested refs.
 The form '--filter=sparse:path=<path>' similarly uses a sparse-checkout
 specification contained in <path>.
 +
-The form '--filter=tree:<depth>' omits all blobs and trees deeper than
-<depth> from the root tree. Currently, only <depth>=0 is supported.
+The form '--filter=tree:<depth>' omits all blobs and trees whose depth
+from the root tree is >= <depth> (minimum depth if an object is located
+at multiple depths in the commits traversed). Currently, only <depth>=0
+is supported, which omits all blobs and trees.

 --no-filter::
  Turn off any previous `--filter=` argument.

-Peff