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

Hi,

On Mon, 5 Oct 2009, Johannes Sixt wrote:

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);

 - In the "ANSI-terminal only" world view, adding colors to strbuf and
   writing it out together with the actual strings is an easy thing to do.
   Don't Windows folks have trouble converting this kind of code to their
   color control call that is separate from writing strings out?  If it is
   not a problem, I do not have any objection to it, but otherwise I'd
   suggest not to add any more code that stores color escape sequence in
   strbuf, so that we would not make later conversion by Windows folks
   harder than necessary.

Thanks for noticing this! To store color escapes in strbuf is not a
problem as long as the string is finally written using printf, fprintf, or
fputs.

quoted

quoted

+			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);

BTW we probably need to remove the "TODO: write" from compat/winansi.c...

Ciao,
Dscho