Log message printout cleanups (#2)

Linus Torvalds · Junio C Hamano · commit eab144ac49c1 · 2006-04-17T21:43:15.000-07:00
Here's a further patch on top of the previous one with cosmetic
improvements (no "real" code changes, just trivial updates):

 - it gets the "---" before a diffstat right, including for the combined
   merge case. Righ now the logic is that we always use "---" when we have
   a diffstat, and an empty line otherwise. That's how I visually prefer
   it, but hey, it can be tweaked later.

 - I made "diff --cc/combined" add the "---/+++" header lines too. The
   thing won't be mistaken for a valid diff, since the "@@" lines have too
   many "@" characters (three or more), but it just makes it visually
   match a real diff, which at least to me makes a big difference in
   readability. Without them, it just looks very "wrong".

   I guess I should have taken the filename from each individual entry
   (and had one "---" file per parent), but I didn't even bother to try to
   see how that works, so this was the simple thing.

With this, doing a

	git log --cc --patch-with-stat

looks quite readable, I think. The only nagging issue - as far as I'm
concerned - is that diffstats for merges are pretty questionable the way
they are done now. I suspect it would be better to just have the _first_
diffstat, and always make the merge diffstat be the one for "result
against first parent".

Signed-off-by: Junio C Hamano &lt;junkio@cox.net&gt;
diff --git a/combine-diff.c b/combine-diff.c
@@ -585,6 +585,16 @@ static void reuse_combine_diff(struct sline *sline, unsigned long cnt,
 	sline->p_lno[i] = sline->p_lno[j];
 }
 
+static void dump_quoted_path(const char *prefix, const char *path)
+{
+	fputs(prefix, stdout);
+	if (quote_c_style(path, NULL, NULL, 0))
+		quote_c_style(path, NULL, stdout, 0);
+	else
+		printf("%s", path);
+	putchar('\n');
+}
+
 static int show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			   int dense, struct rev_info *rev)
 {
@@ -692,12 +702,7 @@ static int show_patch_diff(struct combine_diff_path *elem, int num_parent,
 
 		if (rev->loginfo)
 			show_log(rev, rev->loginfo, "\n");
-		printf("diff --%s ", dense ? "cc" : "combined");
-		if (quote_c_style(elem->path, NULL, NULL, 0))
-			quote_c_style(elem->path, NULL, stdout, 0);
-		else
-			printf("%s", elem->path);
-		putchar('\n');
+		dump_quoted_path(dense ? "diff --cc " : "diff --combined ", elem->path);
 		printf("index ");
 		for (i = 0; i < num_parent; i++) {
 			abb = find_unique_abbrev(elem->parent[i].sha1,
@@ -728,6 +733,8 @@ static int show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			}
 			putchar('\n');
 		}
+		dump_quoted_path("--- a/", elem->path);
+		dump_quoted_path("+++ b/", elem->path);
 		dump_sline(sline, cnt, num_parent);
 	}
 	free(result);
@@ -861,6 +868,9 @@ void diff_tree_combined_merge(const unsigned char *sha1,
 			       &diffopts);
 		diffcore_std(&diffopts);
 		paths = intersect_paths(paths, i, num_parent);
+
+		if (diffopts.with_stat && rev->loginfo)
+			show_log(rev, rev->loginfo, "---\n");
 		diff_flush(&diffopts);
 	}
 
diff --git a/log-tree.c b/log-tree.c
@@ -1,4 +1,3 @@
-
 #include "cache.h"
 #include "diff.h"
 #include "commit.h"
@@ -55,7 +54,7 @@ int log_tree_diff_flush(struct rev_info *opt)
 	}
 
 	if (opt->loginfo && !opt->no_commit_id)
-		show_log(opt, opt->loginfo, "\n");
+		show_log(opt, opt->loginfo, opt->diffopt.with_stat ? "---\n" : "\n");
 	diff_flush(&opt->diffopt);
 	return 1;
 }

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-`
`2`	`1`	`#include "cache.h"`
`3`	`2`	`#include "diff.h"`
`4`	`3`	`#include "commit.h"`
`@@ -55,7 +54,7 @@ int log_tree_diff_flush(struct rev_info *opt)`
`55`	`54`	`}`
`56`	`55`
`57`	`56`	`if (opt->loginfo && !opt->no_commit_id)`
`58`		`- show_log(opt, opt->loginfo, "\n");`
	`57`	`+ show_log(opt, opt->loginfo, opt->diffopt.with_stat ? "---\n" : "\n");`
`59`	`58`	`diff_flush(&opt->diffopt);`
`60`	`59`	`return 1;`
`61`	`60`	`}`