Git notes list/show <revision-range>

Git notes list/show <revision-range>

[Vincent van Ravesteijn]

Hi all,

Considering a work-flow using topic branches just as the git development uses itself, I do miss a certain feature.

When a feature/patch is proposed, it will "cook" for a while in "pu" and "next". During this period people can comment on the patch. These comments can be stored in git notes added to the commits on which is commented.

At a certain point I want to decide whether the feature can be merged into "next" or "master", so I want to collect all notes for a certain branch to see what still has to be done before this feature can be merged. One can think about this of automatically generating a "What's cooking in X" listing
the topic branches and the comments for the commits in each branch.

That's why I want to (try to) implement the feature that 'git notes list' and 'git notes show' accept a revision range and collect all notes for the commits in this range.

Do you think this will be a useful feature and that it has chances to be merged into git ?

Vincent

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

[Jeff King]

On Mon, Apr 18, 2011 at 08:19:59PM +0200, Vincent van Ravesteijn wrote:

> When a feature/patch is proposed, it will "cook" for a while in "pu"
> and "next". During this period people can comment on the patch. These
> comments can be stored in git notes added to the commits on which is
> commented.

If you haven't already seen it, you might find the "notes/*" refs here:

  git://repo.or.cz/git/trast.git

They reference the messages discussing the patch on the mailing list by
message-id and gmane link.

> That's why I want to (try to) implement the feature that 'git notes
> list' and 'git notes show' accept a revision range and collect all
> notes for the commits in this range.

You can do more-or-less what you want with something like:

  git log --format="%h %s%n%N"

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

[Michael J Gruber]

Jeff King venit, vidit, dixit 18.04.2011 20:27:
> On Mon, Apr 18, 2011 at 08:19:59PM +0200, Vincent van Ravesteijn wrote:
> 
>> When a feature/patch is proposed, it will "cook" for a while in "pu"
>> and "next". During this period people can comment on the patch. These
>> comments can be stored in git notes added to the commits on which is
>> commented.
> 
> If you haven't already seen it, you might find the "notes/*" refs here:
> 
>   git://repo.or.cz/git/trast.git
> 
> They reference the messages discussing the patch on the mailing list by
> message-id and gmane link.
> 
>> That's why I want to (try to) implement the feature that 'git notes
>> list' and 'git notes show' accept a revision range and collect all
>> notes for the commits in this range.
> 
> You can do more-or-less what you want with something like:
> 
>   git log --format="%h %s%n%N"

Exactly, the log family does that and more, since it makes the whole
revision walk machinery available.

But since the OP is volunteering to code for notes :-)
We could need a feature which allows to log the history of a note. The alias

`git noteslog' is aliased to `!sh -c 'git log $(git notes get-ref) "$@"' -'

gives you the history of the notes tree (try it with "-p"), but
sometimes I would like the history of the notes to a specific commit, and in

git noteslog -p -- $(commit)

I would have to use for $(commit) all possible breakdowns of the sha1 of
the commit for all possible notes tree structures. It feels as of the
revision walker needs to learn another pathspec, say

":(note):<sha1>"

in line with our magic pathspec discussion.

Michael

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

[Jeff King]

On Tue, Apr 19, 2011 at 09:17:51AM +0200, Michael J Gruber wrote:

> But since the OP is volunteering to code for notes :-)
> We could need a feature which allows to log the history of a note. The alias
> 
> `git noteslog' is aliased to `!sh -c 'git log $(git notes get-ref) "$@"' -'
> 
> gives you the history of the notes tree (try it with "-p"),

Hmm, I just use "git log notes/<whatever>", which works fine. It does
help if you know that the default ref is "notes/commits", though.

It's not something I do often, though (most of my notes use has been
things that automatically make notes, so the history tends to be
uninteresting and useful only for debugging the note-making code).

> sometimes I would like the history of the notes to a specific commit, and in
> 
> git noteslog -p -- $(commit)
> 
> I would have to use for $(commit) all possible breakdowns of the sha1 of
> the commit for all possible notes tree structures. It feels as of the
> revision walker needs to learn another pathspec, say
> 
> ":(note):<sha1>"
> 
> in line with our magic pathspec discussion.

That's a clever solution. It is a little non-intuitive for a user to
need to know about notes storage, though. Maybe you were already
thinking this, but we could have something like:

  git notes log [revs] [--] [pathspec]

where "[revs]" are checked for in refs/notes/*, defaulting to
"refs/notes/commits". And each element of the pathspec gets the
":(note):" magic automatically. I wonder if we could even resolve the
pathspec bits as regular refs.

So you could write:

  # long form, just as you can do with "git log"
  git notes log notes/commits -- ":(note):`git rev-parse HEAD`"

  # or with automagic ref lookup for pathspec
  git notes log notes/commits -- HEAD

  # and automagic default ref
  git notes log -- HEAD

  # and I think you should be able to write a disambiguator similar to
  # what we use for the revs/paths distinction, but this time for
  # notes-refs versus regular refs. And then drop the "--":
  git notes log HEAD

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.

-Peff

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

[Michael J Gruber]

Jeff King venit, vidit, dixit 19.04.2011 22:32:
> On Tue, Apr 19, 2011 at 09:17:51AM +0200, Michael J Gruber wrote:
> 
>> But since the OP is volunteering to code for notes :-)
>> We could need a feature which allows to log the history of a note. The alias
>>
>> `git noteslog' is aliased to `!sh -c 'git log $(git notes get-ref) "$@"' -'
>>
>> gives you the history of the notes tree (try it with "-p"),
> 
> Hmm, I just use "git log notes/<whatever>", which works fine. It does
> help if you know that the default ref is "notes/commits", though.

Sure. That's what the alias does.

> It's not something I do often, though (most of my notes use has been
> things that automatically make notes, so the history tends to be
> uninteresting and useful only for debugging the note-making code).

In particular, the log messages "Notes added by..." are useless. I use
notes for patch boiler plates and branch annotations, and in these cases
history can be interesting (between versions of a patch, e.g.).

>> sometimes I would like the history of the notes to a specific commit, and in
>>
>> git noteslog -p -- $(commit)
>>
>> I would have to use for $(commit) all possible breakdowns of the sha1 of
>> the commit for all possible notes tree structures. It feels as of the
>> revision walker needs to learn another pathspec, say
>>
>> ":(note):<sha1>"
>>
>> in line with our magic pathspec discussion.
> 
> That's a clever solution. It is a little non-intuitive for a user to
> need to know about notes storage, though. Maybe you were already
> thinking this, but we could have something like:
> 
>   git notes log [revs] [--] [pathspec]
> 
> where "[revs]" are checked for in refs/notes/*, defaulting to
> "refs/notes/commits". And each element of the pathspec gets the
> ":(note):" magic automatically. I wonder if we could even resolve the

Here we are back at my alias is a first step in that direction... "git
notes log" should do all that, of course.

> pathspec bits as regular refs.
> 
> 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>]

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

> 
>   # or with automagic ref lookup for pathspec
>   git notes log notes/commits -- HEAD
> 
>   # and automagic default ref
>   git notes log -- HEAD
> 
>   # and I think you should be able to write a disambiguator similar to
>   # what we use for the revs/paths distinction, but this time for
>   # notes-refs versus regular refs. And then drop the "--":
>   git notes log HEAD
> 
> 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).

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

Michael

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

[Jeff King]

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

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

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

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