Merge branch 'th/diff' into next

* th/diff:
  t4013: note improvements brought by the new output code.
  format-patch: fix diff format option implementation
  t4013 test updates for new output code.
  combine-diff.c: type sanity.
  Fix some more diff options changes.
  Fix diff-tree -s
  log --raw: Don't descend into subdirectories by default
  diff-tree: Use ---\n as a message separator
  Print empty line between raw, stat, summary and patch
  whatchanged: Default to DIFF_FORMAT_RAW
  Don't xcalloc() struct diffstat_t
  Add msg_sep to diff_options
  DIFF_FORMAT_RAW is not default anymore
  Set default diff output format after parsing command line
  Make --raw option available for all diff commands
  Merge with_raw, with_stat and summary variables to output_format

builtin-diff-files.c

@@ -36,6 +36,9 @@ int cmd_diff_files(int argc, const char **argv, char **envp)
 			usage(diff_files_usage);
 		argv++; argc--;
 	}
+	if (!rev.diffopt.output_format)
+		rev.diffopt.output_format = DIFF_FORMAT_RAW;
+
 	/*
 	 * Make sure there are NO revision (i.e. pending object) parameter,
 	 * rev.max_count is reasonable (0 <= n <= 3),

builtin-diff-index.c

@@ -28,6 +28,9 @@ int cmd_diff_index(int argc, const char **argv, char **envp)
 		else
 			usage(diff_cache_usage);
 	}
+	if (!rev.diffopt.output_format)
+		rev.diffopt.output_format = DIFF_FORMAT_RAW;
+
 	/*
 	 * Make sure there is one revision (i.e. pending object),
 	 * and there is no revision filtering parameters.

builtin-diff-stages.c

@@ -85,6 +85,9 @@ int cmd_diff_stages(int ac, const char **av, char **envp)
 		ac--; av++;
 	}
 
+	if (!diff_options.output_format)
+		diff_options.output_format = DIFF_FORMAT_RAW;
+
 	if (ac < 3 ||
 	    sscanf(av[1], "%d", &stage1) != 1 ||
 	    ! (0 <= stage1 && stage1 <= 3) ||

builtin-diff-tree.c

@@ -84,6 +84,9 @@ int cmd_diff_tree(int argc, const char **argv, char **envp)
 		usage(diff_tree_usage);
 	}
 
+	if (!opt->diffopt.output_format)
+		opt->diffopt.output_format = DIFF_FORMAT_RAW;
+
 	/*
 	 * NOTE! We expect "a ^b" to be equal to "a..b", so we
 	 * reverse the order of the objects if the second one

builtin-diff.c

@@ -39,8 +39,6 @@ static int builtin_diff_files(struct rev_info *revs,
 			revs->max_count = 3;
 		else if (!strcmp(arg, "-q"))
 			silent = 1;
-		else if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
 		else
 			usage(builtin_diff_usage);
 		argv++; argc--;
@@ -56,7 +54,7 @@ static int builtin_diff_files(struct rev_info *revs,
 	    3 < revs->max_count)
 		usage(builtin_diff_usage);
 	if (revs->max_count < 0 &&
-	    (revs->diffopt.output_format == DIFF_FORMAT_PATCH))
+	    (revs->diffopt.output_format & DIFF_FORMAT_PATCH))
 		revs->combine_merges = revs->dense_combined_merges = 1;
 	/*
 	 * Backward compatibility wart - "diff-files -s" used to
@@ -107,14 +105,9 @@ static int builtin_diff_b_f(struct rev_info *revs,
 	/* Blob vs file in the working tree*/
 	struct stat st;
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc > 1)
+		usage(builtin_diff_usage);
+
 	if (lstat(path, &st))
 		die("'%s': %s", path, strerror(errno));
 	if (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))
@@ -137,14 +130,9 @@ static int builtin_diff_blobs(struct rev_info *revs,
 	 */
 	unsigned mode = canon_mode(S_IFREG | 0644);
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc > 1)
+		usage(builtin_diff_usage);
+
 	stuff_change(&revs->diffopt,
 		     mode, mode,
 		     blob[1].sha1, blob[0].sha1,
@@ -162,8 +150,6 @@ static int builtin_diff_index(struct rev_info *revs,
 		const char *arg = argv[1];
 		if (!strcmp(arg, "--cached"))
 			cached = 1;
-		else if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
 		else
 			usage(builtin_diff_usage);
 		argv++; argc--;
@@ -185,14 +171,9 @@ static int builtin_diff_tree(struct rev_info *revs,
 {
 	const unsigned char *(sha1[2]);
 	int swap = 0;
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+
+	if (argc > 1)
+		usage(builtin_diff_usage);
 
 	/* We saw two trees, ent[0] and ent[1].
 	 * if ent[1] is unintesting, they are swapped
@@ -214,14 +195,9 @@ static int builtin_diff_combined(struct rev_info *revs,
 	const unsigned char (*parent)[20];
 	int i;
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc > 1)
+		usage(builtin_diff_usage);
+
 	if (!revs->dense_combined_merges && !revs->combine_merges)
 		revs->dense_combined_merges = revs->combine_merges = 1;
 	parent = xmalloc(ents * sizeof(*parent));
@@ -276,9 +252,11 @@ int cmd_diff(int argc, const char **argv, char **envp)
 
 	git_config(git_diff_config);
 	init_revisions(&rev);
-	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 
 	argc = setup_revisions(argc, argv, &rev, NULL);
+	if (!rev.diffopt.output_format)
+		rev.diffopt.output_format = DIFF_FORMAT_PATCH;
+
 	/* Do we have --cached and not have a pending object, then
 	 * default to HEAD by hand.  Eek.
 	 */

builtin-log.c

@@ -14,25 +14,22 @@
 /* this is in builtin-diff.c */
 void add_head(struct rev_info *revs);
 
-static int cmd_log_wc(int argc, const char **argv, char **envp,
+static void cmd_log_init(int argc, const char **argv, char **envp,
 		      struct rev_info *rev)
 {
-	struct commit *commit;
-
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
 	rev->verbose_header = 1;
 	argc = setup_revisions(argc, argv, rev, "HEAD");
-	if (rev->always_show_header) {
-		if (rev->diffopt.pickaxe || rev->diffopt.filter) {
-			rev->always_show_header = 0;
-			if (rev->diffopt.output_format == DIFF_FORMAT_RAW)
-				rev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
-		}
-	}
-
+	if (rev->diffopt.pickaxe || rev->diffopt.filter)
+		rev->always_show_header = 0;
 	if (argc > 1)
 		die("unrecognized argument: %s", argv[1]);
+}
+
+static int cmd_log_walk(struct rev_info *rev)
+{
+	struct commit *commit;
 
 	prepare_revision_walk(rev);
 	setup_pager();
@@ -54,7 +51,10 @@ int cmd_whatchanged(int argc, const char **argv, char **envp)
 	rev.diff = 1;
 	rev.diffopt.recursive = 1;
 	rev.simplify_history = 0;
-	return cmd_log_wc(argc, argv, envp, &rev);
+	cmd_log_init(argc, argv, envp, &rev);
+	if (!rev.diffopt.output_format)
+		rev.diffopt.output_format = DIFF_FORMAT_RAW;
+	return cmd_log_walk(&rev);
 }
 
 int cmd_show(int argc, const char **argv, char **envp)
@@ -69,7 +69,8 @@ int cmd_show(int argc, const char **argv, char **envp)
 	rev.always_show_header = 1;
 	rev.ignore_merges = 0;
 	rev.no_walk = 1;
-	return cmd_log_wc(argc, argv, envp, &rev);
+	cmd_log_init(argc, argv, envp, &rev);
+	return cmd_log_walk(&rev);
 }
 
 int cmd_log(int argc, const char **argv, char **envp)
@@ -78,8 +79,8 @@ int cmd_log(int argc, const char **argv, char **envp)
 
 	init_revisions(&rev);
 	rev.always_show_header = 1;
-	rev.diffopt.recursive = 1;
-	return cmd_log_wc(argc, argv, envp, &rev);
+	cmd_log_init(argc, argv, envp, &rev);
+	return cmd_log_walk(&rev);
 }
 
 static int istitlechar(char c)
@@ -237,11 +238,10 @@ int cmd_format_patch(int argc, const char **argv, char **envp)
 	rev.commit_format = CMIT_FMT_EMAIL;
 	rev.verbose_header = 1;
 	rev.diff = 1;
-	rev.diffopt.with_raw = 0;
-	rev.diffopt.with_stat = 1;
 	rev.combine_merges = 0;
 	rev.ignore_merges = 1;
-	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
+	rev.diffopt.msg_sep = "";
+	rev.diffopt.recursive = 1;
 
 	git_config(git_format_config);
 	rev.extra_headers = extra_headers;
@@ -312,6 +312,9 @@ int cmd_format_patch(int argc, const char **argv, char **envp)
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
+	if (!rev.diffopt.output_format)
+		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
+
 	if (output_directory) {
 		if (use_stdout)
 			die("standard output, or directory, which one?");

combine-diff.c

@@ -702,7 +702,7 @@ static int show_patch_diff(struct combine_diff_path *elem, int num_parent,
 		const char *abb;
 
 		if (rev->loginfo)
-			show_log(rev, rev->loginfo, "\n");
+			show_log(rev, opt->msg_sep);
 		dump_quoted_path(dense ? "diff --cc " : "diff --combined ", elem->path);
 		printf("index ");
 		for (i = 0; i < num_parent; i++) {
@@ -770,9 +770,9 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
 		inter_name_termination = 0;
 
 	if (rev->loginfo)
-		show_log(rev, rev->loginfo, "\n");
+		show_log(rev, opt->msg_sep);
 
-	if (opt->output_format == DIFF_FORMAT_RAW) {
+	if (opt->output_format & DIFF_FORMAT_RAW) {
 		offset = strlen(COLONS) - num_parent;
 		if (offset < 0)
 			offset = 0;
@@ -792,8 +792,7 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
 		printf(" %s ", diff_unique_abbrev(p->sha1, opt->abbrev));
 	}
 
-	if (opt->output_format == DIFF_FORMAT_RAW ||
-	    opt->output_format == DIFF_FORMAT_NAME_STATUS) {
+	if (opt->output_format & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS)) {
 		for (i = 0; i < num_parent; i++)
 			putchar(p->parent[i].status);
 		putchar(inter_name_termination);
@@ -819,17 +818,12 @@ void show_combined_diff(struct combine_diff_path *p,
 	struct diff_options *opt = &rev->diffopt;
 	if (!p->len)
 		return;
-	switch (opt->output_format) {
-	case DIFF_FORMAT_RAW:
-	case DIFF_FORMAT_NAME_STATUS:
-	case DIFF_FORMAT_NAME:
+	if (opt->output_format & (DIFF_FORMAT_RAW |
+				  DIFF_FORMAT_NAME |
+				  DIFF_FORMAT_NAME_STATUS)) {
 		show_raw_diff(p, num_parent, rev);
-		return;
-	case DIFF_FORMAT_PATCH:
+	} else if (opt->output_format & DIFF_FORMAT_PATCH) {
 		show_patch_diff(p, num_parent, dense, rev);
-		return;
-	default:
-		return;
 	}
 }
 
@@ -842,22 +836,20 @@ void diff_tree_combined(const unsigned char *sha1,
 	struct diff_options *opt = &rev->diffopt;
 	struct diff_options diffopts;
 	struct combine_diff_path *p, *paths = NULL;
-	int i, num_paths;
-	int do_diffstat;
+	int i, num_paths, needsep, show_log_first;
 
-	do_diffstat = (opt->output_format == DIFF_FORMAT_DIFFSTAT ||
-		       opt->with_stat);
 	diffopts = *opt;
-	diffopts.with_raw = 0;
-	diffopts.with_stat = 0;
+	diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diffopts.recursive = 1;
 
+	show_log_first = !!rev->loginfo;
+	needsep = 0;
 	/* find set of paths that everybody touches */
 	for (i = 0; i < num_parent; i++) {
 		/* show stat against the first parent even
 		 * when doing combined diff.
 		 */
-		if (i == 0 && do_diffstat)
+		if (i == 0 && opt->output_format & DIFF_FORMAT_DIFFSTAT)
 			diffopts.output_format = DIFF_FORMAT_DIFFSTAT;
 		else
 			diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
@@ -865,12 +857,12 @@ void diff_tree_combined(const unsigned char *sha1,
 		diffcore_std(&diffopts);
 		paths = intersect_paths(paths, i, num_parent);
 
-		if (do_diffstat && rev->loginfo)
-			show_log(rev, rev->loginfo,
-				 opt->with_stat ? "---\n" : "\n");
+		if (show_log_first && i == 0) {
+			show_log(rev, opt->msg_sep);
+			if (rev->verbose_header && opt->output_format)
+				putchar(opt->line_termination);
+		}
 		diff_flush(&diffopts);
-		if (opt->with_stat)
-			putchar('\n');
 	}
 
 	/* find out surviving paths */
@@ -879,17 +871,25 @@ void diff_tree_combined(const unsigned char *sha1,
 			num_paths++;
 	}
 	if (num_paths) {
-		if (opt->with_raw) {
-			int saved_format = opt->output_format;
-			opt->output_format = DIFF_FORMAT_RAW;
+		if (opt->output_format & (DIFF_FORMAT_RAW |
+					  DIFF_FORMAT_NAME |
+					  DIFF_FORMAT_NAME_STATUS)) {
 			for (p = paths; p; p = p->next) {
-				show_combined_diff(p, num_parent, dense, rev);
+				if (p->len)
+					show_raw_diff(p, num_parent, rev);
 			}
-			opt->output_format = saved_format;
-			putchar(opt->line_termination);
+			needsep = 1;
 		}
-		for (p = paths; p; p = p->next) {
-			show_combined_diff(p, num_parent, dense, rev);
+		else if (opt->output_format & DIFF_FORMAT_DIFFSTAT)
+			needsep = 1;
+		if (opt->output_format & DIFF_FORMAT_PATCH) {
+			if (needsep)
+				putchar(opt->line_termination);
+			for (p = paths; p; p = p->next) {
+				if (p->len)
+					show_patch_diff(p, num_parent, dense,
+							rev);
+			}
 		}
 	}
 

diff.c

@@ -1420,11 +1420,11 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
 void diff_setup(struct diff_options *options)
 {
 	memset(options, 0, sizeof(*options));
-	options->output_format = DIFF_FORMAT_RAW;
 	options->line_termination = '\n';
 	options->break_opt = -1;
 	options->rename_limit = -1;
 	options->context = 3;
+	options->msg_sep = "";
 
 	options->change = diff_change;
 	options->add_remove = diff_addremove;
@@ -1438,22 +1438,28 @@ int diff_setup_done(struct diff_options *options)
 	    (0 <= options->rename_limit && !options->detect_rename))
 		return -1;
 
+	if (options->output_format & (DIFF_FORMAT_NAME |
+				      DIFF_FORMAT_NAME_STATUS |
+				      DIFF_FORMAT_CHECKDIFF |
+				      DIFF_FORMAT_NO_OUTPUT))
+		options->output_format &= ~(DIFF_FORMAT_RAW |
+					    DIFF_FORMAT_DIFFSTAT |
+					    DIFF_FORMAT_SUMMARY |
+					    DIFF_FORMAT_PATCH);
+
 	/*
 	 * These cases always need recursive; we do not drop caller-supplied
 	 * recursive bits for other formats here.
 	 */
-	if ((options->output_format == DIFF_FORMAT_PATCH) ||
-	    (options->output_format == DIFF_FORMAT_DIFFSTAT) ||
-	    (options->output_format == DIFF_FORMAT_CHECKDIFF))
+	if (options->output_format & (DIFF_FORMAT_PATCH |
+				      DIFF_FORMAT_DIFFSTAT |
+				      DIFF_FORMAT_CHECKDIFF))
 		options->recursive = 1;
-
 	/*
-	 * These combinations do not make sense.
+	 * Also pickaxe would not work very well if you do not say recursive
 	 */
-	if (options->output_format == DIFF_FORMAT_RAW)
-		options->with_raw = 0;
-	if (options->output_format == DIFF_FORMAT_DIFFSTAT)
-		options->with_stat  = 0;
+	if (options->pickaxe)
+		options->recursive = 1;
 
 	if (options->detect_rename && options->rename_limit < 0)
 		options->rename_limit = diff_rename_limit_default;
@@ -1526,22 +1532,22 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 {
 	const char *arg = av[0];
 	if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
-		options->output_format = DIFF_FORMAT_PATCH;
+		options->output_format |= DIFF_FORMAT_PATCH;
 	else if (opt_arg(arg, 'U', "unified", &options->context))
-		options->output_format = DIFF_FORMAT_PATCH;
+		options->output_format |= DIFF_FORMAT_PATCH;
+	else if (!strcmp(arg, "--raw"))
+		options->output_format |= DIFF_FORMAT_RAW;
 	else if (!strcmp(arg, "--patch-with-raw")) {
-		options->output_format = DIFF_FORMAT_PATCH;
-		options->with_raw = 1;
+		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;
 	}
 	else if (!strcmp(arg, "--stat"))
-		options->output_format = DIFF_FORMAT_DIFFSTAT;
+		options->output_format |= DIFF_FORMAT_DIFFSTAT;
 	else if (!strcmp(arg, "--check"))
-		options->output_format = DIFF_FORMAT_CHECKDIFF;
+		options->output_format |= DIFF_FORMAT_CHECKDIFF;
 	else if (!strcmp(arg, "--summary"))
-		options->summary = 1;
+		options->output_format |= DIFF_FORMAT_SUMMARY;
 	else if (!strcmp(arg, "--patch-with-stat")) {
-		options->output_format = DIFF_FORMAT_PATCH;
-		options->with_stat = 1;
+		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
 	}
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
@@ -1550,19 +1556,20 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	else if (!strcmp(arg, "--full-index"))
 		options->full_index = 1;
 	else if (!strcmp(arg, "--binary")) {
-		options->output_format = DIFF_FORMAT_PATCH;
+		options->output_format |= DIFF_FORMAT_PATCH;
 		options->full_index = options->binary = 1;
 	}
 	else if (!strcmp(arg, "--name-only"))
-		options->output_format = DIFF_FORMAT_NAME;
+		options->output_format |= DIFF_FORMAT_NAME;
 	else if (!strcmp(arg, "--name-status"))
-		options->output_format = DIFF_FORMAT_NAME_STATUS;
+		options->output_format |= DIFF_FORMAT_NAME_STATUS;
 	else if (!strcmp(arg, "-R"))
 		options->reverse_diff = 1;
 	else if (!strncmp(arg, "-S", 2))
 		options->pickaxe = arg + 2;
-	else if (!strcmp(arg, "-s"))
-		options->output_format = DIFF_FORMAT_NO_OUTPUT;
+	else if (!strcmp(arg, "-s")) {
+		options->output_format |= DIFF_FORMAT_NO_OUTPUT;
+	}
 	else if (!strncmp(arg, "-O", 2))
 		options->orderfile = arg + 2;
 	else if (!strncmp(arg, "--diff-filter=", 14))
@@ -1737,15 +1744,17 @@ const char *diff_unique_abbrev(const unsigned char *sha1, int len)
 }
 
 static void diff_flush_raw(struct diff_filepair *p,
-			   int line_termination,
-			   int inter_name_termination,
-			   struct diff_options *options,
-			   int output_format)
+			   struct diff_options *options)
 {
 	int two_paths;
 	char status[10];
 	int abbrev = options->abbrev;
 	const char *path_one, *path_two;
+	int inter_name_termination = '\t';
+	int line_termination = options->line_termination;
+
+	if (!line_termination)
+		inter_name_termination = 0;
 
 	path_one = p->one->path;
 	path_two = p->two->path;
@@ -1774,7 +1783,7 @@ static void diff_flush_raw(struct diff_filepair *p,
 		two_paths = 0;
 		break;
 	}
-	if (output_format != DIFF_FORMAT_NAME_STATUS) {
+	if (!(options->output_format & DIFF_FORMAT_NAME_STATUS)) {
 		printf(":%06o %06o %s ",
 		       p->one->mode, p->two->mode,
 		       diff_unique_abbrev(p->one->sha1, abbrev));
@@ -1983,48 +1992,30 @@ static void diff_resolve_rename_copy(void)
 	diff_debug_queue("resolve-rename-copy done", q);
 }
 
-static void flush_one_pair(struct diff_filepair *p,
-			   int diff_output_format,
-			   struct diff_options *options,
-			   struct diffstat_t *diffstat)
+static int check_pair_status(struct diff_filepair *p)
 {
-	int inter_name_termination = '\t';
-	int line_termination = options->line_termination;
-	if (!line_termination)
-		inter_name_termination = 0;
-
 	switch (p->status) {
 	case DIFF_STATUS_UNKNOWN:
-		break;
+		return 0;
 	case 0:
 		die("internal error in diff-resolve-rename-copy");
-		break;
 	default:
-		switch (diff_output_format) {
-		case DIFF_FORMAT_DIFFSTAT:
-			diff_flush_stat(p, options, diffstat);
-			break;
-		case DIFF_FORMAT_CHECKDIFF:
-			diff_flush_checkdiff(p, options);
-			break;
-		case DIFF_FORMAT_PATCH:
-			diff_flush_patch(p, options);
-			break;
-		case DIFF_FORMAT_RAW:
-		case DIFF_FORMAT_NAME_STATUS:
-			diff_flush_raw(p, line_termination,
-				       inter_name_termination,
-				       options, diff_output_format);
-			break;
-		case DIFF_FORMAT_NAME:
-			diff_flush_name(p, line_termination);
-			break;
-		case DIFF_FORMAT_NO_OUTPUT:
-			break;
-		}
+		return 1;
 	}
 }
 
+static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)
+{
+	int fmt = opt->output_format;
+
+	if (fmt & DIFF_FORMAT_CHECKDIFF)
+		diff_flush_checkdiff(p, opt);
+	else if (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS))
+		diff_flush_raw(p, opt);
+	else if (fmt & DIFF_FORMAT_NAME)
+		diff_flush_name(p, opt->line_termination);
+}
+
 static void show_file_mode_name(const char *newdelete, struct diff_filespec *fs)
 {
 	if (fs->mode)
@@ -2243,58 +2234,96 @@ int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1)
 	return result;
 }
 
+static int is_summary_empty(const struct diff_queue_struct *q)
+{
+	int i;
+
+	for (i = 0; i < q->nr; i++) {
+		const struct diff_filepair *p = q->queue[i];
+
+		switch (p->status) {
+		case DIFF_STATUS_DELETED:
+		case DIFF_STATUS_ADDED:
+		case DIFF_STATUS_COPIED:
+		case DIFF_STATUS_RENAMED:
+			return 0;
+		default:
+			if (p->score)
+				return 0;
+			if (p->one->mode && p->two->mode &&
+			    p->one->mode != p->two->mode)
+				return 0;
+			break;
+		}
+	}
+	return 1;
+}
+
 void diff_flush(struct diff_options *options)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
-	int i;
-	int diff_output_format = options->output_format;
-	struct diffstat_t *diffstat = NULL;
+	int i, output_format = options->output_format;
+	int separator = 0;
 
-	if (diff_output_format == DIFF_FORMAT_DIFFSTAT || options->with_stat) {
-		diffstat = xcalloc(sizeof (struct diffstat_t), 1);
-		diffstat->xm.consume = diffstat_consume;
-	}
+	/*
+	 * Order: raw, stat, summary, patch
+	 * or:    name/name-status/checkdiff (other bits clear)
+	 */
+	if (!q->nr)
+		goto free_queue;
 
-	if (options->with_raw) {
+	if (output_format & (DIFF_FORMAT_RAW |
+			     DIFF_FORMAT_NAME |
+			     DIFF_FORMAT_NAME_STATUS |
+			     DIFF_FORMAT_CHECKDIFF)) {
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
-			flush_one_pair(p, DIFF_FORMAT_RAW, options, NULL);
+			if (check_pair_status(p))
+				flush_one_pair(p, options);
 		}
-		putchar(options->line_termination);
+		separator++;
 	}
-	if (options->with_stat) {
+
+	if (output_format & DIFF_FORMAT_DIFFSTAT) {
+		struct diffstat_t diffstat;
+
+		memset(&diffstat, 0, sizeof(struct diffstat_t));
+		diffstat.xm.consume = diffstat_consume;
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
-			flush_one_pair(p, DIFF_FORMAT_DIFFSTAT, options,
-				       diffstat);
+			if (check_pair_status(p))
+				diff_flush_stat(p, options, &diffstat);
 		}
-		show_stats(diffstat);
-		free(diffstat);
-		diffstat = NULL;
-		if (options->summary)
-			for (i = 0; i < q->nr; i++)
-				diff_summary(q->queue[i]);
-		if (options->stat_sep)
-			fputs(options->stat_sep, stdout);
-		else
-			putchar(options->line_termination);
-	}
-	for (i = 0; i < q->nr; i++) {
-		struct diff_filepair *p = q->queue[i];
-		flush_one_pair(p, diff_output_format, options, diffstat);
+		show_stats(&diffstat);
+		separator++;
 	}
 
-	if (diffstat) {
-		show_stats(diffstat);
-		free(diffstat);
-	}
-
-	for (i = 0; i < q->nr; i++) {
-		if (diffstat && options->summary)
+	if (output_format & DIFF_FORMAT_SUMMARY && !is_summary_empty(q)) {
+		for (i = 0; i < q->nr; i++)
 			diff_summary(q->queue[i]);
-		diff_free_filepair(q->queue[i]);
+		separator++;
 	}
 
+	if (output_format & DIFF_FORMAT_PATCH) {
+		if (separator) {
+			if (options->stat_sep) {
+				/* attach patch instead of inline */
+				fputs(options->stat_sep, stdout);
+			} else {
+				putchar(options->line_termination);
+			}
+		}
+
+		for (i = 0; i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			if (check_pair_status(p))
+				diff_flush_patch(p, options);
+		}
+	}
+
+	for (i = 0; i < q->nr; i++)
+		diff_free_filepair(q->queue[i]);
+free_queue:
 	free(q->queue);
 	q->queue = NULL;
 	q->nr = q->alloc = 0;

diff.h

@@ -20,19 +20,31 @@ typedef void (*add_remove_fn_t)(struct diff_options *options,
 		    const unsigned char *sha1,
 		    const char *base, const char *path);
 
+#define DIFF_FORMAT_RAW		0x0001
+#define DIFF_FORMAT_DIFFSTAT	0x0002
+#define DIFF_FORMAT_SUMMARY	0x0004
+#define DIFF_FORMAT_PATCH	0x0008
+
+/* These override all above */
+#define DIFF_FORMAT_NAME	0x0010
+#define DIFF_FORMAT_NAME_STATUS	0x0020
+#define DIFF_FORMAT_CHECKDIFF	0x0040
+
+/* Same as output_format = 0 but we know that -s flag was given
+ * and we should not give default value to output_format.
+ */
+#define DIFF_FORMAT_NO_OUTPUT	0x0080
+
 struct diff_options {
 	const char *filter;
 	const char *orderfile;
 	const char *pickaxe;
 	unsigned recursive:1,
-		 with_raw:1,
-		 with_stat:1,
 		 tree_in_recursive:1,
 		 binary:1,
 		 full_index:1,
 		 silent_on_remove:1,
 		 find_copies_harder:1,
-		 summary:1,
 		 color_diff:1;
 	int context;
 	int break_opt;
@@ -45,6 +57,7 @@ struct diff_options {
 	int rename_limit;
 	int setup;
 	int abbrev;
+	const char *msg_sep;
 	const char *stat_sep;
 	long xdl_opts;
 
@@ -151,15 +164,6 @@ extern void diffcore_std_no_resolve(struct diff_options *);
 "                show all files diff when -S is used and hit is found.\n"
 
 extern int diff_queue_is_empty(void);
-
-#define DIFF_FORMAT_RAW		1
-#define DIFF_FORMAT_PATCH	2
-#define DIFF_FORMAT_NO_OUTPUT	3
-#define DIFF_FORMAT_NAME	4
-#define DIFF_FORMAT_NAME_STATUS	5
-#define DIFF_FORMAT_DIFFSTAT	6
-#define DIFF_FORMAT_CHECKDIFF	7
-
 extern void diff_flush(struct diff_options*);
 
 /* diff-raw status letters */

log-tree.c

@@ -43,9 +43,10 @@ static int append_signoff(char *buf, int buf_sz, int at, const char *signoff)
 	return at;
 }
 
-void show_log(struct rev_info *opt, struct log_info *log, const char *sep)
+void show_log(struct rev_info *opt, const char *sep)
 {
 	static char this_header[16384];
+	struct log_info *log = opt->loginfo;
 	struct commit *commit = log->commit, *parent = log->parent;
 	int abbrev = opt->diffopt.abbrev;
 	int abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;
@@ -163,8 +164,22 @@ int log_tree_diff_flush(struct rev_info *opt)
 		return 0;
 	}
 
-	if (opt->loginfo && !opt->no_commit_id)
-		show_log(opt, opt->loginfo, opt->diffopt.with_stat ? "---\n" : "\n");
+	if (opt->loginfo && !opt->no_commit_id) {
+		/* When showing a verbose header (i.e. log message),
+		 * and not in --pretty=oneline format, we would want
+		 * an extra newline between the end of log and the
+		 * output for readability.
+		 */
+		show_log(opt, opt->diffopt.msg_sep);
+		if (opt->verbose_header &&
+		    opt->commit_format != CMIT_FMT_ONELINE) {
+			int pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
+			if ((pch & opt->diffopt.output_format) == pch)
+				printf("---%c", opt->diffopt.line_termination);
+			else
+				putchar(opt->diffopt.line_termination);
+		}
+	}
 	diff_flush(&opt->diffopt);
 	return 1;
 }
@@ -261,7 +276,7 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
 		log.parent = NULL;
-		show_log(opt, opt->loginfo, "");
+		show_log(opt, "");
 		shown = 1;
 	}
 	opt->loginfo = NULL;

log-tree.h

@@ -11,6 +11,6 @@ void init_log_tree_opt(struct rev_info *);
 int log_tree_diff_flush(struct rev_info *);
 int log_tree_commit(struct rev_info *, struct commit *);
 int log_tree_opt_parse(struct rev_info *, const char **, int);
-void show_log(struct rev_info *opt, struct log_info *log, const char *sep);
+void show_log(struct rev_info *opt, const char *sep);
 
 #endif

revision.c

@@ -851,8 +851,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 	}
 	if (revs->combine_merges) {
 		revs->ignore_merges = 0;
-		if (revs->dense_combined_merges &&
-		    (revs->diffopt.output_format != DIFF_FORMAT_DIFFSTAT))
+		if (revs->dense_combined_merges && !revs->diffopt.output_format)
 			revs->diffopt.output_format = DIFF_FORMAT_PATCH;
 	}
 	revs->diffopt.abbrev = revs->abbrev;

t/t4013-diff-various.sh

@@ -138,7 +138,10 @@ diff-tree --pretty --summary initial
 diff-tree --pretty --stat --summary initial
 diff-tree --pretty --root -p initial
 diff-tree --pretty --root --stat initial
-#diff-tree --pretty --root --summary initial
+# improved by Timo's patch
+diff-tree --pretty --root --summary initial
+# improved by Timo's patch
+diff-tree --pretty --root --summary -r initial
 diff-tree --pretty --root --stat --summary initial
 diff-tree --pretty --patch-with-stat initial
 diff-tree --pretty --root --patch-with-stat initial
@@ -150,6 +153,7 @@ diff-tree --pretty=oneline --root initial
 diff-tree --pretty=oneline -p initial
 diff-tree --pretty=oneline --root -p initial
 diff-tree --pretty=oneline --patch-with-stat initial
+# improved by Timo's patch
 diff-tree --pretty=oneline --root --patch-with-stat initial
 diff-tree --pretty=oneline --patch-with-raw initial
 diff-tree --pretty=oneline --root --patch-with-raw initial
@@ -172,11 +176,9 @@ diff-tree --cc --stat --summary master
 # stat summary should show the diffstat and summary with the first parent
 diff-tree -c --stat --summary side
 diff-tree --cc --stat --summary side
-# this one gives an extra newline after stat, which should be removed
-# diff-tree --cc --patch-with-stat master
-# this one gives an extra newline after stat, which should be removed
-# other than that it shows the correct example -- stat and summary are
-# against the first parent, and patch-looking combined diff follows.
+# improved by Timo's patch
+diff-tree --cc --patch-with-stat master
+# improved by Timo's patch
 diff-tree --cc --patch-with-stat --summary master
 # this is correct
 diff-tree --cc --patch-with-stat --summary side
@@ -188,7 +190,9 @@ log --root -p master
 log --patch-with-stat master
 log --root --patch-with-stat master
 log --root --patch-with-stat --summary master
+# improved by Timo's patch
 log --root -c --patch-with-stat --summary master
+# improved by Timo's patch
 log --root --cc --patch-with-stat --summary master
 log -SF master
 log -SF -p master
@@ -200,7 +204,9 @@ whatchanged --root -p master
 whatchanged --patch-with-stat master
 whatchanged --root --patch-with-stat master
 whatchanged --root --patch-with-stat --summary master
+# improved by Timo's patch
 whatchanged --root -c --patch-with-stat --summary master
+# improved by Timo's patch
 whatchanged --root --cc --patch-with-stat --summary master
 whatchanged -SF master
 whatchanged -SF -p master

t/t4013/diff.diff-tree_--cc_--patch-with-stat_--summary_master

@@ -4,7 +4,6 @@ $ git diff-tree --cc --patch-with-stat --summary master
  file0   |    3 +++
  2 files changed, 5 insertions(+), 0 deletions(-)
 
-
 diff --cc dir/sub
 index cead32e,7289e35..992913c
 --- a/dir/sub

t/t4013/diff.diff-tree_--cc_--patch-with-stat_master

@@ -0,0 +1,34 @@
+$ git diff-tree --cc --patch-with-stat master
+176b998f5d647cbd77a9d8acf4531e930754d16d
+ dir/sub |    2 ++
+ file0   |    3 +++
+ 2 files changed, 5 insertions(+), 0 deletions(-)
+
+diff --cc dir/sub
+index cead32e,7289e35..992913c
+--- a/dir/sub
++++ b/dir/sub
+@@@ -1,6 -1,4 +1,8 @@@
+  A
+  B
+ +C
+ +D
+ +E
+ +F
++ 1
++ 2
+diff --cc file0
+index b414108,f4615da..10a8a9f
+--- a/file0
++++ b/file0
+@@@ -1,6 -1,6 +1,9 @@@
+  1
+  2
+  3
+ +4
+ +5
+ +6
++ A
++ B
++ C
+$

t/t4013/diff.diff-tree_--pretty=oneline_--root_--patch-with-stat_initial

@@ -1,6 +1,5 @@
 $ git diff-tree --pretty=oneline --root --patch-with-stat initial
 444ac553ac7612cc88969031b02b3767fb8a353a Initial
----
  dir/sub |    2 ++
  file0   |    3 +++
  file2   |    3 +++

t/t4013/diff.diff-tree_--pretty_--root_--summary_-r_initial

@@ -0,0 +1,11 @@
+$ git diff-tree --pretty --root --summary -r initial
+commit 444ac553ac7612cc88969031b02b3767fb8a353a
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+
+ create mode 100644 dir/sub
+ create mode 100644 file0
+ create mode 100644 file2
+$

t/t4013/diff.diff-tree_--pretty_--root_--summary_initial

@@ -0,0 +1,11 @@
+$ git diff-tree --pretty --root --summary initial
+commit 444ac553ac7612cc88969031b02b3767fb8a353a
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+
+ create mode 040000 dir
+ create mode 100644 file0
+ create mode 100644 file2
+$

t/t4013/diff.log_--root_--cc_--patch-with-stat_--summary_master

@@ -5,12 +5,11 @@ Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:04:00 2006 +0000
 
     Merge branch 'side'
----
+
  dir/sub |    2 ++
  file0   |    3 +++
  2 files changed, 5 insertions(+), 0 deletions(-)
 
-
 diff --cc dir/sub
 index cead32e,7289e35..992913c
 --- a/dir/sub

t/t4013/diff.log_--root_-c_--patch-with-stat_--summary_master

@@ -5,12 +5,11 @@ Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:04:00 2006 +0000
 
     Merge branch 'side'
----
+
  dir/sub |    2 ++
  file0   |    3 +++
  2 files changed, 5 insertions(+), 0 deletions(-)
 
-
 diff --combined dir/sub
 index cead32e,7289e35..992913c
 --- a/dir/sub

t/t4013/diff.whatchanged_--root_--cc_--patch-with-stat_--summary_master

@@ -5,12 +5,11 @@ Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:04:00 2006 +0000
 
     Merge branch 'side'
----
+
  dir/sub |    2 ++
  file0   |    3 +++
  2 files changed, 5 insertions(+), 0 deletions(-)
 
-
 diff --cc dir/sub
 index cead32e,7289e35..992913c
 --- a/dir/sub

t/t4013/diff.whatchanged_--root_-c_--patch-with-stat_--summary_master

@@ -5,12 +5,11 @@ Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:04:00 2006 +0000
 
     Merge branch 'side'
----
+
  dir/sub |    2 ++
  file0   |    3 +++
  2 files changed, 5 insertions(+), 0 deletions(-)
 
-
 diff --combined dir/sub
 index cead32e,7289e35..992913c
 --- a/dir/sub