From: karthik nayak <karthik.188@gmail.com>
Date: Sat, 28 Mar 2015 18:44:58 GMT
Subject: Re: [RFC/GSoC] Proposal Draft: Unifying git branch -l, git tag -l, and git for-each-ref
Message-ID: <5516F6AA.50103@gmail.com>
In-Reply-To: <20150326163748.GF6564@peff.net>


On 03/26/2015 10:07 PM, Jeff King wrote:
> On Mon, Mar 23, 2015 at 06:39:20PM +0530, karthik nayak wrote:
>
> > All three commands select a subset of the repository’s refs and print the
> > result. There has been an attempt to unify these commands by Jeff King[3]. I
> > plan on continuing his work[4] and using his approach to tackle this
> > project.
>
> I would be cautious about the work in my for-each-ref-contains-wip
> branch. At one point it was reasonably solid, but it's now a year and a
> half old, and I've been rebasing it without paying _too_ much attention
> to correctness. I think some subtle bugs have been introduced as it has
> been carried forward.
>
> Also, the very first patch (factoring out the contains traversal) is
> probably better served by this series:
>
>    http://thread.gmane.org/gmane.comp.version-control.git/252472
>
> I don't remember all of the issues offhand that need to be addressed in
> it, but there were plenty of review comments.
Thanks for the link, will go through that.
>
> > For extended selection behaviour such as ‘--contains’ or ‘--merged’ we could
> > implement these within
> > the library by providing functions which closely mimic the current methods
> > used individually by ‘branch -l’ and ‘tag -l’. For eg to implement
> > ‘--merged’ we implement a ‘compute_merge()’ function, which with the help of
> > the revision API’s will be able to perform the same function as ‘branch -l
> > --merged’.
>
> One trick with making a library-like interface is that some of the
> selection routines can work on a "streaming" list of refs (i.e., as we
> see each ref we can say "yes" or "no") and some must wait until the end
> (e.g., --merged does a single big merge traversal). It's probably not
> the end of the world to just always collect all the refs, then filter
> them, then sort them, then print them. It may delay the start of output
> in some minor cases, but I doubt that's a big deal (and anyway, the
> packed-refs code will load them all into an array anyway, so collecting
> them in a second array is probably not a big deal).
I think I noted this down while going through your implementation also.
You even mentioned this on the mailing list if I'm not wrong.
Will have to work out a design around this and think about it more.
>
> > For formatting functionality provided by ‘for-each-ref’ we replicate the
> > ‘show_ref’ function in ‘for-each-ref.c’ where the format is given to the
> > function and the function uses the format to obtain atom values and prints
> > the corresponding atom values to the screen. This feature would allow us to
> > provide format functionality which could act as a base for the ‘-v’ option
> > also.
>
> Yeah, I'd really like to see "--format" for "git branch", and have "-v"
> just feed that a hard-coded format string (or even a configurable one).
>
> > Although Jeff has built a really good base to build upon, I shall use
> > his work as more of a reference and work on unification of the three
> > commands from scratch.
>
> Good. :)
>
> -Peff
>
Thanks for the Review/Tips.

Regards
-Karthik