Subject: Re: [PATCH] Initialize notes trees if %N is used and no --show-notes given

From: Thomas Rast <[email-masked]>
Date: Tue, 6 Apr 2010 11:27:01 +0200

[A Cc would have been nice, I nearly missed this but it's clearly my
bug.]

Johannes Gilger wrote:
> this bug bit me when I used 'git log --format="%N"' without adding
> --show-notes, which caused git to fail an assertion:
>  Assertion failed: (display_notes_trees), function format_display_notes, file notes.c, line 1186.
[...]
> diff --git a/notes.c b/notes.c
> index e425e19..83f39ae 100644
> --- a/notes.c
> +++ b/notes.c
> @@ -1183,6 +1183,8 @@ void format_display_notes(const unsigned char *object_sha1,
>  			  struct strbuf *sb, const char *output_encoding, int flags)
>  {
>  	int i;
> +	if (!display_notes_trees)
> +		init_display_notes(NULL);
>  	assert(display_notes_trees);

Thanks for the report.  Unfortunately this returns to the
silently-initialize-with-NULL case that was I explicitly asked to
avoid.

I see three options:
- %N could simply expand to nothing if notes are disabled
- %N could silently initialize as above
- your patch

though for your patch, I'd also remove the assert() since it's
basically there to enforce the requirement of initializing them; the
trees list can never be NULL after init_display_notes().

Currently I think the first option would be the best, since
(notionally; we still don't have all the bits AFAIK) the built-in
formats can then be written with a %N at the right place, without
having to worry about the other command line options.  I haven't had
enough coffee to think about any possible ill side effects, though.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch