On Mon, Jul 16, 2012 at 10:42:07PM -0700, Junio C Hamano wrote: > > Just like log, the notes are part of the commit information to the right > > of the graph. But this second hunk is for when we are not using the > > pretty-printer at all, and the output looks like this: > > > > $ git rev-list --graph --notes -2 HEAD > > * f6bbb09529a4cc73446c7c115ac1468477bd0cc6 > > > > Notes: > > foobar > > * 31c79549b85c6393be4f40432f5b86ebc097fc7e > > > > which doesn't make sense > > I actually have quite a different feeling about this. As I said in > the separate message, I think --graph, or anything that makes the > output unparsable or harder to parse for machines for that matter, > in rev-list are not something we have because we wanted to support > them, but that which just happen to work because the large part of > rev-list and log can share building blocks to do similar things. > The key phrase is "can share" here; it does not necessarily mean > they "should" [*1*]. Somebody went to the trouble to make "rev-list --graph" work[1] (that is what the call to graph_show_remainder in the else clause of the conditional is about). I agree it seems kind of useless, but it does work now, and we should at least not make it worse (and I think we both agree that the output above is just wrong). So whatever we show for a note, it should look like: * f6bbb095... | the notes thing to show * 31c79549... Because that is how graph output is formatted. Either that, or we should disallow --graph entirely with rev-list (which I'd also be OK with). > I do not mind having an option to show the notes text, but I doubt > it is a sane thing to do to make "rev-list --notes" unconditionally > show the payload of the notes blob. "rev-list --objects" only shows > the object names of trees and blobs, not the payload in these > objects, and this is very much on purpuse. It allows the downstream > process that reads its output from the pipe to easily parse the > output and choose to do whatever it wants to do using them. > > I wonder if we should show the blob object names that store the > notes payload if we were given --notes option in a format that is > easy for readers to mechanically parse its output. 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. -Peff [1] Looking at the code, I do think somebody wanted "rev-list --graph" to work, and it is not an accident. But I think they did not do a very thorough job, as things like "git rev-list --objects --graph" produce nonsensical output. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html