Re: [PATCH] bisect: print abbrev sha1 for first bad commit

On Sun, May 10, 2015 at 07:12:45PM -0400, Trevor Saunders wrote:

quoted

Yeah, I have always found bisect's output somewhat silly. It prints the
"--raw" diff output, which is not incredibly useful. And then to top it
off, it does not feed the "--recursive" switch to the diff, so you don't
even get to see the real list of changed files.

 So, fun fact it doesn't actually always print the raw diffoutput if
 there is no diff, for example a merge where both sides only touched
 different files as in test 40 in t6030.

Ah, that makes sense. It's basically just feeding the commit to
"diff-tree" (except doing it internally rather than running it as a
separate program). And the defaults there do not show anything for merge
commits. It could do the equivalent of "--cc" (i.e., set the
dense_combined_merges flag in the "struct rev_info").

quoted

(Actually, it looks like all this is generated in bisect.c:show_diff_tree,
so it would have to be written in C; but it should be pretty easy to
tweak the display options).

yeah, that seems pretty straight forward, but I'm not really sure what
to do about this case where no diff is printed, I guess I should figure
out what bits need to be set for the commit to be shown anyway.

I'd argue for simply never showing the diff (dropping the "opt.diff = 1"
line from bisect.c:show_diff_tree), but that is mostly my personal
opinion. If we are going to show a diff, perhaps "--recursive
--name-status" would be the most friendly, with "--cc" for the merge
commits.

Translated into C, something like (this is completely untested):

diff --git a/bisect.c b/bisect.c
index 10f5e57..62786cf 100644
--- a/bisect.c
+++ b/bisect.c

@@ -876,6 +876,8 @@ static void show_diff_tree(const char *prefix, struct commit *commit)
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	opt.abbrev = 0;
 	opt.diff = 1;
+	opt.combine_merges = 1;
+	opt.dense_combined_merges = 1;
 
 	/* This is what "--pretty" does */
 	opt.verbose_header = 1;

@@ -884,7 +886,8 @@ static void show_diff_tree(const char *prefix, struct commit *commit)
 
 	/* diff-tree init */
 	if (!opt.diffopt.output_format)
-		opt.diffopt.output_format = DIFF_FORMAT_RAW;
+		opt.diffopt.output_format = DIFF_FORMAT_NAME_STATUS;
+	DIFF_OPT_SET(&opt.diffopt, RECURSIVE);
 
 	log_tree_commit(&opt, commit);
 }