Re: [PATCH v4 0/8] rev-list: implement object type filter

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

>> I however have to wonder why this need so much code (in other words,
>> why do we need more than just adding something similar to this block
>> in the revision.c machinery:
>> 
>> 	} else if (!strcmp(arg, "--objects")) {
>> 		revs->tag_objects = 1;
>> 		revs->tree_objects = 1;
>> 		revs->blob_objects = 1;
>> 
>> that flips <type>_objects member bits?) though.
>
> So if we make this part of the rev machinery, I guess your idea is that
> we can just put the logic into `show_object()`?

Not necessarily.  I was making a lot more naïve observations.

 * Even though we have 3 independent (tag|tree|blob)_objects bits,
   they are all set to true with "--objects" or they are cleared
   without.  There is no codepath that flips these bits to set some
   but not all of them to be set.

 * But if we look at the the hits from

    $ git grep -C2 -E -e '([.]|->)(tag|tree|blob)_objects' \*.[ch]

   it is clear that the code is (at least trying to be) prepared for
   them to be set independently.  The .tree_objects member is often
   checked without checking others to mark the tree objects on the
   edge of the range uninteresting, for example.

   It of course is unknown how well the code is actually prepared
   for these three bits to be set independently, as nobody can set
   these bits independently.

 * Which makes a naïve reader to wonder if it would be sufficient
   to have a silly option, like this:

 	} else if (!strcmp(arg, "--filter=object:type=tree")) {
 		revs->tag_objects = 0;
 		revs->tree_objects = 1;
 		revs->blob_objects = 0;

   in the same if/else if/... cascade as the above (and other types
   as well), in order to do the same thing as this series.

And, the above led to the question---the patches in your series
apparently do a lot more (even if we discount the option parsing
part), and I was wondering if that is because the independence
between these three bits the existing code aspires to maintain is
broken.

> Anyway, this is assuming that I'm not misinterpreting what you mean by
> your above comment. So please let me know if I misunderstood.