Re: [RFC/GSoC] Proposal Draft: Unifying git branch -l, git tag -l, and git for-each-ref
- From
karthik nayak <karthik.188@gmail.com>
- Date
- Mar 28, 2015, 18:44 UTC
- Message-ID
- <5516F6AA.50103@gmail.com>
- In-Reply-To
- <20150326163748.GF6564@peff.net>
On 03/26/2015 10:07 PM, Jeff King wrote:
Show 20 quoted lines
> 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.
Show 18 quoted lines
> > > 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.
Show 19 quoted lines
> > > 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