On 06/04/10 11:27, Thomas Rast wrote: > [A Cc would have been nice, I nearly missed this but it's clearly my > bug.] Sorry for that, haven't mailed to git-ml for quite a while ;) > I see three options: > - %N could simply expand to nothing if notes are disabled > - %N could silently initialize as above > - your patch The first option would be confusing. I, for one, would simply put %N in my log and never really know that existing notes aren't displayed. I wasn't even sure my git.git checkout had notes, so I created one myself. A better behaviour would be to not expand %N if notes are disabled, so a user gets some kind of feedback that %N isn't working. I'd really like %N to do the initialization. There is no other placeholder which requires an extra option to work, if I see it correctly. As for the builtin formats I was under the impressions that they worked completely outside the parser for placeholders, so one would not use '%N' in a builtin format, and %N initializing the notes would not conflict with --no-notes and builtin formats. > 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(). Sure. Greetings, Jojo -- Johannes Gilger <heipei@xxxxxxxxxxxx> http://heipei.net GPG-Key: 0xD47A7FFC GPG-Fingerprint: 5441 D425 6D4A BD33 B580 618C 3CDC C4D0 D47A 7FFC -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html