Re: [PATCH] pretty.c: add %z specifier.

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 21, 2008, 05:09 UTC
Message-ID: <7vtzj0slx4.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080321045137.GA5563@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 15 quoted lines

> On Thu, Mar 20, 2008 at 09:48:16PM -0700, Junio C Hamano wrote:
>
>> > +	case 'z':		/* null */
>> > +		strbuf_addch(sb, '\0');
>> > +		return 1;
>> >  	}
>> >
>> >  	/* these depend on the commit */
>> 
>> I do not like this at all.  Why aren't we doing %XX (2 hexadecimal digits
>> for an octet)?
>
> Because %ad is already taken? :)
>
> %x* is still available, though, so maybe %x00?

Perhaps, but before I forget.

My much bigger niggle about the "--pretty=format:<>" code I have is that the "log" machinery does not change the usual record "delimiter" to record "terminator" when --pretty=format:<> is in effect.

The "log" family generally treats LF/NUL as record delimiter, not terminator, and it is by a very good conscious design. When you are looking at the output from "git log -2", you would want to have a delimiting LF between the first commit and the second commit, but you do not want an extra LF after the second commit.

However, when "--pretty=format:<>" is in effect, it is inconvenient that the machinery inserts a LF between each record but not at the end.

    $ git log -2 --pretty=format:%s

may look sane when the pager immediately returns the control to you, but it is not really. To view it:

    $ git log -2 --pretty=format:%s | cat

This would show that there is no LF after the final output, which is quite bad.