git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: karthik nayak <karthik.188@gmail.com>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [RFC/GSoC] Proposal Draft: Unifying git branch -l, git tag -l, and git for-each-ref
Date: Thu, 26 Mar 2015 12:37:49 -0400	[thread overview]
Message-ID: <20150326163748.GF6564@peff.net> (raw)
In-Reply-To: <55101080.90805@gmail.com>

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.

> 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).

> 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

  reply	other threads:[~2015-03-26 16:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-23 13:09 [RFC/GSoC] Proposal Draft: Unifying git branch -l, git tag -l, and git for-each-ref karthik nayak
2015-03-26 16:37 ` Jeff King [this message]
2015-03-28 18:44   ` karthik nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150326163748.GF6564@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=karthik.188@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).