Re: [PATCH] rev-list: fix place holder %N (notes) in user format

From: Jeff King <peff@peff.net>
Date: Mar 25, 2012, 00:55 UTC
Message-ID: <20120325005504.GA27651@sigill.intra.peff.net>
In-Reply-To: <1332617911-8281-1-git-send-email-jukka.lehtniemi@gmail.com>

On Sat, Mar 24, 2012 at 09:38:31PM +0200, Jukka Lehtniemi wrote:

Show 7 quoted lines

> Signed-off-by: Jukka Lehtniemi <jukka.lehtniemi@gmail.com>
> ---
> 
>  Fixes a bug where the place holder for notes (%N) was not expanded 
>  in rev-list. To reproduce the bug:
>    $ git notes add -m foo
>    $ git rev-list --notes --format=format:%N HEAD ^HEAD^

This explanation should probably go in the commit message.

I am not sure the behavior afterwards is right, though. With git-log, I see the following behaviors:

  1. With --notes, notes are displayed.

  3. With --no-notes, notes are not displayed.

  3. With no options, notes are displayed (i.e., we default to --notes).

  4. With --format=%N, notes are displayed (automatically, without
     having to use "--notes").

Shouldn't rev-list behave exactly the same way, except for (3)? That is, it should respect the options in the same way, but default to not showing notes?

Even with your patch, we seem to be violating points (1) and (4). "--notes" does nothing by itself, and you need to use it to make "%N" work.

For (4), you need to use userformat_find_requirement, as git-log does. I'm not sure without digging how to make (1) work.

-Peff