Re: Git notes list/show <revision-range>

From: Jeff King <peff@peff.net>
Date: Apr 20, 2011, 07:35 UTC
Message-ID: <20110420073517.GB1971@sigill.intra.peff.net>
In-Reply-To: <4DAE8867.8010704@drmicha.warpmail.net>

On Wed, Apr 20, 2011 at 09:16:55AM +0200, Michael J Gruber wrote:

Show 9 quoted lines

> > So you could write:
> > 
> >   # long form, just as you can do with "git log"
> >   git notes log notes/commits -- ":(note):`git rev-parse HEAD`"
> 
> I don't think "notes log" should take a notes ref argument like that. It
> already has "--ref" for that purpose, so I would suggest
> 
> git notes [--ref <notesref>] log [<rev>]

Hmm. You are probably right. I was modeling it after "log", but it is a notes subcommand. I was thinking you could also do something like:

  git notes log commits some-other-notes $sha1

or even

  # see what any notes ref has to say about this commit
  git notes log --all $sha1

but it is probably a better idea to keep the interface more consistent with the other notes subcommands.

Show 5 quoted lines

> git notes [--ref <notesref>] log [<rev>]
>
> with <rev> being mapped to :(notes):<rev> automatically, <rev>
> defaulting to HEAD. That is much more in line with the other notes
> subcommands. (We may or may note check for ":(notes):" being there already.)

I think it should be more like:

  git notes [--ref <notesref] log [<rev options>] [<rev> ...]

That is:

  1. There may be arbitrary rev options. Because you might want "log
     -p", or "log --format".

  2. We may want to allow multiple revs (e.g., if you have a patch
     series with 5 commits, you may want to see the history of any notes
     that refer to any of those commits).

I think (1) means we may need to feed the result to the revision options-parser.

Show 8 quoted lines

> > I think it would need a little refactoring of setup_revisions() to be
> > more flexible, but most of the hard work is already done by the usual
> > revision traversal mechanism.
> 
> ? I haven't checked in detail, but I think all we need is:
> 
> - Make "git notes --ref <notesref> log <rev>" call "git log
> <resolvednotesref> -- :(notes):<rev>" (and pass on log options).

I was worried about how to get the log options. If you see:

  git notes log --foo bar

is "bar" a revision, or an argument to --foo? I think right now the answer is always "revision", but that is only because the revision options do not currently use parse-options, which would allow "--format=foo" to be written as "--format foo". So I'd rather not rely on that.

> - Make the pathspec ":(notes):<rev>" resolve <rev> to <sha1> and match
> with all possible breakdowns of <sha1> which we accept for a notes tree.

I was assuming that :(notes): would only accept a sha1. But it probably makes sense for it to handle resolution itself.

-Peff