Re: [PATCH v2] Fix notes handling in rev-list

[Jeff King <peff@peff.net>]

On Wed, Jul 18, 2012 at 03:39:34PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > So leaving aside the --graph issues, we would need to decide what to
> > show in the non-graph case. And I think your suggestion is good; there
> > is no real need to dereference the blob (if you want that, you can turn
> > on the pretty-printer).
> >
> > I'm just not sure what the output should be. I guess:
> >
> >   <commit_sha1> <notes sha1s>
> >
> > is probably the most sensible (it's sort of like --parents). And that
> > solves the --graph issue, too, since it continues to take only a single
> > line.
> 
> Surely.  "rev-list --parents --notes" would still be usable that
> way, as a reader that requests such a combination is prepared to
> tell commits (i.e. parents) and blobs (i.e. notes) apart.

I don't think we forbid non-blob values in notes trees, so technically
there could be some ambiguity. I doubt it is a big problem in practice
(especially since I haven't even heard of a good use case yet for "git
rev-list --notes", let alone "git rev-list --notes --parents"). But now
would be the time to avoid a crappy format that we will be stuck with
later.

Unlike elements of the commit object itself, like --parents or
--timestamp, notes do not really gain any efficiency by being printed as
part of the traversal. So modulo the cost of piping the list of commits,
it would not really be any more efficient than "git rev-list | git notes
list --stdin" (except that the latter does not take a --stdin argument,
but could easily do so). And the latter is way more flexible.

So for plumbing, I think this is the wrong direction, anyway. The real
value of this patch is that the pretty-printed code path would work more
like git-log (especially the "%N" format, which lets callers make their
own micro-format for specifying all the bits they are interested in).

Maybe the best thing is to simply disallow --notes when not using a
pretty-printed format.

-Peff