From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mon, 05 Oct 2009 09:20:06 GMT
Subject: Re: [PATCH] Add the --submodule-summary option to the diff option family
Message-ID: <alpine.DEB.1.00.0910051119260.4985@pacific.mpi-cbg.de>
In-Reply-To: <4AC9B7C3.3090302@viscovery.net>

Hi,

On Mon, 5 Oct 2009, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > On Mon, 5 Oct 2009, Johannes Sixt wrote:
> >> Junio C Hamano schrieb:
> >>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> >>>> +	fwrite(sb.buf, sb.len, 1, f);
> >>>> +
> >>>> +	if (!message) {
> >>>> +		while ((commit = get_revision(&rev))) {
> >>>> +			strbuf_setlen(&sb, 0);
> >>>> +			if (del)
> >>>> +				strbuf_addstr(&sb, commit->object.flags &
> >>>> +						SYMMETRIC_LEFT ? del : add);
> >>>> +			format_commit_message(commit, format, &sb,
> >>>> +					rev.date_mode);
> >>>> +			if (del)
> >>>> +				strbuf_addstr(&sb, reset);
> >>>> +			strbuf_addch(&sb, '\n');
> >>>> +			fwrite(sb.buf, sb.len, 1, f);
> >> Outch! fwrite doesn't interpret color escapes. AFAICS, this sequence is
> >> easy to change such that it uses fprintf().
> > 
> > Good point.  I changed it to
> > 
> >                         fprintf(f, "%s", sb.buf);
> 
> Thanks. But notice how you are constructing the string in sb from pieces.
> I meant to change it to
> 
> 	fprintf(f, "%s%s%s\n",
> 			del ? (commit->object.flags & SYMMETRIC_LEFT
> 					 ? del : add) : "",
> 			format_commit_message(commit, format, &sb,
> 					rev.date_mode),
> 			del ? reset : "");
> 
> or similar. We already use this idiom elsewhere.

And I find it utterly ugly and unreadable there, too. So this is why I did 
not do it.

Ciao,
Dscho