Re: [PATCH] Add the --submodule-summary option to the diff option family

Hi,

On Mon, 5 Oct 2009, Johannes Sixt wrote:

Johannes Schindelin schrieb:

quoted

On Mon, 5 Oct 2009, Johannes Sixt wrote:

quoted

Junio C Hamano schrieb:

quoted

Johannes Schindelin [off-list ref] writes:

quoted

+	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