On Fri, Aug 14, 2015 at 9:26 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> On Thu, Aug 13, 2015 at 5:05 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote: >>> On Thu, Aug 13, 2015 at 4:21 PM, Karthik Nayak <karthik.188@xxxxxxxxx> 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}? > Well okay I could something on those lines. > - 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? I was under the idea that since we're dealing with do_for_each_entry() eventually and in that we set the loose_dir value based on the given 'base' hence when the caller asks for something like "--branches --tags", it would be better to just iterate through the refs in the directory of "--branches --tags" rather than go through the whole list of refs and drop ones which don't belong to "--branches --tags". but this was an over the top look at how do_for_each_entry() works, I could be totally off the mark. -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html