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

[Matthew DeVore <matvore@google.com>]

On Tue, Aug 14, 2018 at 1:01 PM Jeff King <peff@peff.net> wrote:
>
> On Tue, Aug 14, 2018 at 10:28:13AM -0700, Matthew DeVore wrote:
>
> > 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:
>
> > +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