Re: [PATCH] notes: Use get_sha1_committish instead of read_ref in init_notes()
- From
Mike Hommey <mh@glandium.org>
- Date
- Jun 17, 2015, 09:02 UTC
- Message-ID
- <20150617090246.GC30948@glandium.org>
- In-Reply-To
- <20150617032231.GA24505@peff.net>
On Tue, Jun 16, 2015 at 11:22:31PM -0400, Jeff King wrote:
Show 21 quoted lines
> On Wed, Jun 17, 2015 at 10:15:31AM +0900, Mike Hommey wrote:
>
> > init_notes() is essentially the only point of entry to the notes API.
> > It is an arbitrary restriction that all it allows as input is a strict
> > ref name, when callers may want to give an arbitrary committish.
> >
> > This has the side effect of enabling the use of committish as notes refs
> > in commands allowing them, e.g. git log --notes=foo@{1}, although
> > I haven't checked whether that's the case for all of them.
>
> What about expand_notes_ref? We call that on the argument to "--notes".
> I guess it is OK to expand "foo@{1}" into "refs/notes/foo@{1}", but what
> about other more arcane syntaxes, like ":/"?
>
> In a sense that is weirdly broken already:
>
> $ git log --notes=:/foo >/dev/null
> warning: notes ref refs/notes/:/foo is invalid
>
> but I wonder if we should be making expand_notes_ref a little more
> careful as part of the same topic.Interestingly, now that I look, there's also this: https://github.com/git/git/blob/master/notes-cache.c#L40
that doesn't use expand_notes_ref, but it's apparently only used in userdiff_get_textconv, and I'm not sure why.
Mike