Re: [PATCH] notes: Use get_sha1_committish instead of read_ref in init_notes()

From: Jeff King <peff@peff.net>
Date: Jun 17, 2015, 03:22 UTC
Message-ID: <20150617032231.GA24505@peff.net>
In-Reply-To: <1434503731-26414-1-git-send-email-mh@glandium.org>

On Wed, Jun 17, 2015 at 10:15:31AM +0900, Mike Hommey wrote:

Show 7 quoted lines

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

-Peff