Re: [PATCH 01/10] ref-filter: add option to filter only branches

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 14, 2015, 15:56 UTC
Message-ID: <xmqq614hkgnd.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAOLa=ZQSVCgcuUQFsF2Mq-m+wS8im3t4Z3PSEgoAHcV0gGuvPA@mail.gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

Show 23 quoted lines

> On Thu, Aug 13, 2015 at 5:05 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> On Thu, Aug 13, 2015 at 4:21 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>>>
>>> This was taken from branch.c, I thought of using an enum instead but that
>>> would again require most of branch.c, hence it's been carried over
>>> without changing
>>> I'm thinking of changing it, any suggestions?
>>>
>>
>> What I was thinking was of having:
>>
>> #define FILTER_REFS_INCLUDE_BROKEN 0x1
>> #define FILTER_REFS_TAGS 0x2
>> #define FILTER_REFS_BRANCHES 0x4
>> #define FILTER_REFS_REMOTES 0x8
>> #define FILTER_REFS_DETACHED_HEAD 0x16
>>
>> and using these for showing ref kind also instead of separately
>> having 'REF_DETACHED_HEAD' and so on.
>>
>
> Something like this:
> https://github.com/KarthikNayak/git/commit/0ec5381420dcdfe7c62000b56168e2842d5d0063

I notice a few things in ref-filter.c in that commit (a web interface including GitHub one is horrible in showing the things in wider context across files, so I'll base my discussion by guessing what the caller of this function and helpers this function calls do):

 - Your "ALL" silently overrides others.  Is that sensible?  Perhaps
   you would instead want to define FILTER_REFS_OTHER (not needed to
   be exposed to UI) and then define FILTER_REFS_ALL as the ORed
   value of FILTER_REFS_{BRANCHES,...,OTHER}?

 - When the caller asks for "--branches --tags", you run
   ref-filter-handler twice on ref_cbdata.  Does that make sense?
   Shouldn't you iterate over all the available refs just once,
   rejecting ones that aren't in either refs/{heads,tags}/ instead?