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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
