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

From: Thomas Rast <trast@student.ethz.ch>
Date: Apr 6, 2010, 09:27 UTC
Message-ID: <201004061127.01471.trast@student.ethz.ch>
In-Reply-To: <20100405115548.GA19971@macbook.lan.lan>

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

[...]

Show 11 quoted lines

> 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