Re: Log message printout cleanups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Linus Torvalds <torvalds@xxxxxxxx> writes:

> There's actually something _wrong_ with "git log --stat". 
>
> What happens is that "git log" will enable "rev.combine_merges" by 
> default, and that means that together with "--stat" ignoring merges by 
> default, it will _instead_ generate a "--cc" diff for that merge.
>
> I'll leave that up to you, I'm getting pretty fed up with "git log" right 
> now ;)

The primary problem is that the code in "next" was before the
"fancy diffstat for combined" work by Johannes; it punts and
always shows the patch output even when stat is asked for.

I think this does what both of us want.  One thing I noticed is
that log-tree-diff-flush does "---\n" under --patch-with-stat
but not under --stat; I matched that here.

-- >8 --
combine-diff: show diffstat with the first parent.

Even under combine-diff, asking for stat (either with --stat or
--patch-with-stat) gives you diffstat for the first parent.

While the combined patch is useful to highlight the complexity
and interaction of the parts touched by all branches when
reviewing a merge commit, diffstat is a tool to assess the
extent of damage the merge brings in, and showing stat with the
first parent is more sensible than clever per-parent diffstat.

Signed-off-by: Junio C Hamano <junkio@xxxxxxx>

---

 combine-diff.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index aef9006..27f6f57 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -831,10 +831,11 @@ void show_combined_diff(struct combine_d
 	case DIFF_FORMAT_NAME:
 		show_raw_diff(p, num_parent, rev);
 		return;
-
-	default:
 	case DIFF_FORMAT_PATCH:
 		show_patch_diff(p, num_parent, dense, rev);
+		return;
+	default:
+		return;
 	}
 }
 
@@ -847,10 +848,13 @@ void diff_tree_combined_merge(const unsi
 	struct commit_list *parents;
 	struct combine_diff_path *p, *paths = NULL;
 	int num_parent, i, num_paths;
+	int do_diffstat;
 
+	do_diffstat = (opt->output_format == DIFF_FORMAT_DIFFSTAT ||
+		       opt->with_stat);
 	diffopts = *opt;
-	diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diffopts.with_raw = 0;
+	diffopts.with_stat = 0;
 	diffopts.recursive = 1;
 
 	/* count parents */
@@ -864,14 +868,24 @@ void diff_tree_combined_merge(const unsi
 	     parents;
 	     parents = parents->next, i++) {
 		struct commit *parent = parents->item;
+		/* show stat against the first parent even
+		 * when doing combined diff.
+		 */
+		if (i == 0 && do_diffstat)
+			diffopts.output_format = DIFF_FORMAT_DIFFSTAT;
+		else
+			diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
 		diff_tree_sha1(parent->object.sha1, commit->object.sha1, "",
 			       &diffopts);
 		diffcore_std(&diffopts);
 		paths = intersect_paths(paths, i, num_parent);
 
-		if (diffopts.with_stat && rev->loginfo)
-			show_log(rev, rev->loginfo, "---\n");
+		if (do_diffstat && rev->loginfo)
+			show_log(rev, rev->loginfo,
+				 opt->with_stat ? "---\n" : "\n");
 		diff_flush(&diffopts);
+		if (opt->with_stat)
+			putchar('\n');
 	}
 
 	/* find out surviving paths */


-
: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]