Log message printout cleanups

On Sun, 16 Apr 2006, Junio C Hamano wrote:
>
> In the mid-term, I am hoping we can drop the generate_header()
> callchain _and_ the custom code that formats commit log in-core,
> found in cmd_log_wc().

Ok, this was nastier than expected, just because the dependencies between
the different log-printing stuff were absolutely _everywhere_, but here's
a patch that does exactly that.

The patch is not very easy to read, and the "--patch-with-stat" thing is
still broken (it does not call the "show_log()" thing properly for
merges). That's not a new bug. In the new world order it _should_ do
something like

	if (rev->logopt)
		show_log(rev, rev->logopt, "---\n");

but it doesn't. I haven't looked at the --with-stat logic, so I left it
alone.

That said, this patch removes more lines than it adds, and in particular,
the "cmd_log_wc()" loop is now a very clean:

	while ((commit = get_revision(rev)) != NULL) {
		log_tree_commit(rev, commit);
		free(commit->buffer);
		commit->buffer = NULL;
	}

so it doesn't get much prettier than this. All the complexity is entirely
hidden in log-tree.c, and any code that needs to flush the log literally
just needs to do the "if (rev->logopt) show_log(...)" incantation.

I had to make the combined_diff() logic take a "struct rev_info" instead
of just a "struct diff_options", but that part is pretty clean.

This does change "git whatchanged" from using "diff-tree" as the commit
descriptor to "commit", and I changed one of the tests to reflect that new
reality. Otherwise everything still passes, and my other tests look fine
too.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>

combine-diff.c

@@ -5,6 +5,7 @@
 #include "diffcore.h"
 #include "quote.h"
 #include "xdiff-interface.h"
+#include "log-tree.h"
 
 static int uninteresting(struct diff_filepair *p)
 {
@@ -585,9 +586,9 @@ static void reuse_combine_diff(struct sline *sline, unsigned long cnt,
 }
 
 static int show_patch_diff(struct combine_diff_path *elem, int num_parent,
-			   int dense, const char *header,
-			   struct diff_options *opt)
+			   int dense, struct rev_info *rev)
 {
+	struct diff_options *opt = &rev->diffopt;
 	unsigned long result_size, cnt, lno;
 	char *result, *cp, *ep;
 	struct sline *sline; /* survived lines */
@@ -689,10 +690,8 @@ static int show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	if (show_hunks || mode_differs || working_tree_file) {
 		const char *abb;
 
-		if (header) {
-			shown_header++;
-			printf("%s%c", header, opt->line_termination);
-		}
+		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);
@@ -750,8 +749,9 @@ static int show_patch_diff(struct combine_diff_path *elem, int num_parent,
 
 #define COLONS "::::::::::::::::::::::::::::::::"
 
-static void show_raw_diff(struct combine_diff_path *p, int num_parent, const char *header, struct diff_options *opt)
+static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct rev_info *rev)
 {
+	struct diff_options *opt = &rev->diffopt;
 	int i, offset, mod_type = 'A';
 	const char *prefix;
 	int line_termination, inter_name_termination;
@@ -761,8 +761,8 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, const cha
 	if (!line_termination)
 		inter_name_termination = 0;
 
-	if (header)
-		printf("%s%c", header, line_termination);
+	if (rev->loginfo)
+		show_log(rev, rev->loginfo, "\n");
 
 	for (i = 0; i < num_parent; i++) {
 		if (p->parent[i].mode)
@@ -810,31 +810,31 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, const cha
 	}
 }
 
-int show_combined_diff(struct combine_diff_path *p,
+void show_combined_diff(struct combine_diff_path *p,
 		       int num_parent,
 		       int dense,
-		       const char *header,
-		       struct diff_options *opt)
+		       struct rev_info *rev)
 {
+	struct diff_options *opt = &rev->diffopt;
 	if (!p->len)
-		return 0;
+		return;
 	switch (opt->output_format) {
 	case DIFF_FORMAT_RAW:
 	case DIFF_FORMAT_NAME_STATUS:
 	case DIFF_FORMAT_NAME:
-		show_raw_diff(p, num_parent, header, opt);
-		return 1;
+		show_raw_diff(p, num_parent, rev);
+		return;
 
 	default:
 	case DIFF_FORMAT_PATCH:
-		return show_patch_diff(p, num_parent, dense, header, opt);
+		show_patch_diff(p, num_parent, dense, rev);
 	}
 }
 
-const char *diff_tree_combined_merge(const unsigned char *sha1,
-			     const char *header, int dense,
-			     struct diff_options *opt)
+void diff_tree_combined_merge(const unsigned char *sha1,
+			     int dense, struct rev_info *rev)
 {
+	struct diff_options *opt = &rev->diffopt;
 	struct commit *commit = lookup_commit(sha1);
 	struct diff_options diffopts;
 	struct commit_list *parents;
@@ -874,17 +874,13 @@ const char *diff_tree_combined_merge(const unsigned char *sha1,
 			int saved_format = opt->output_format;
 			opt->output_format = DIFF_FORMAT_RAW;
 			for (p = paths; p; p = p->next) {
-				if (show_combined_diff(p, num_parent, dense,
-						       header, opt))
-					header = NULL;
+				show_combined_diff(p, num_parent, dense, rev);
 			}
 			opt->output_format = saved_format;
 			putchar(opt->line_termination);
 		}
 		for (p = paths; p; p = p->next) {
-			if (show_combined_diff(p, num_parent, dense,
-					       header, opt))
-				header = NULL;
+			show_combined_diff(p, num_parent, dense, rev);
 		}
 	}
 
@@ -894,5 +890,4 @@ const char *diff_tree_combined_merge(const unsigned char *sha1,
 		paths = paths->next;
 		free(tmp);
 	}
-	return header;
 }

diff-files.c

@@ -5,33 +5,35 @@
  */
 #include "cache.h"
 #include "diff.h"
+#include "commit.h"
+#include "revision.h"
 
 static const char diff_files_usage[] =
 "git-diff-files [-q] [-0/-1/2/3 |-c|--cc] [<common diff options>] [<path>...]"
 COMMON_DIFF_OPTIONS_HELP;
 
-static struct diff_options diff_options;
+static struct rev_info rev;
 static int silent = 0;
 static int diff_unmerged_stage = 2;
 static int combine_merges = 0;
 static int dense_combined_merges = 0;
 
 static void show_unmerge(const char *path)
 {
-	diff_unmerge(&diff_options, path);
+	diff_unmerge(&rev.diffopt, path);
 }
 
 static void show_file(int pfx, struct cache_entry *ce)
 {
-	diff_addremove(&diff_options, pfx, ntohl(ce->ce_mode),
+	diff_addremove(&rev.diffopt, pfx, ntohl(ce->ce_mode),
 		       ce->sha1, ce->name, NULL);
 }
 
 static void show_modified(int oldmode, int mode,
 			  const unsigned char *old_sha1, const unsigned char *sha1,
 			  char *path)
 {
-	diff_change(&diff_options, oldmode, mode, old_sha1, sha1, path, NULL);
+	diff_change(&rev.diffopt, oldmode, mode, old_sha1, sha1, path, NULL);
 }
 
 int main(int argc, const char **argv)
@@ -41,7 +43,7 @@ int main(int argc, const char **argv)
 	int entries, i;
 
 	git_config(git_diff_config);
-	diff_setup(&diff_options);
+	diff_setup(&rev.diffopt);
 	while (1 < argc && argv[1][0] == '-') {
 		if (!strcmp(argv[1], "--")) {
 			argv++;
@@ -74,7 +76,7 @@ int main(int argc, const char **argv)
 			dense_combined_merges = combine_merges = 1;
 		else {
 			int diff_opt_cnt;
-			diff_opt_cnt = diff_opt_parse(&diff_options,
+			diff_opt_cnt = diff_opt_parse(&rev.diffopt,
 						      argv+1, argc-1);
 			if (diff_opt_cnt < 0)
 				usage(diff_files_usage);
@@ -89,13 +91,13 @@ int main(int argc, const char **argv)
 		argv++; argc--;
 	}
 	if (dense_combined_merges)
-		diff_options.output_format = DIFF_FORMAT_PATCH;
+		rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 
 	/* Find the directory, and set up the pathspec */
 	pathspec = get_pathspec(prefix, argv + 1);
 	entries = read_cache();
 
-	if (diff_setup_done(&diff_options) < 0)
+	if (diff_setup_done(&rev.diffopt) < 0)
 		usage(diff_files_usage);
 
 	/* At this point, if argc == 1, then we are doing everything.
@@ -167,8 +169,7 @@ int main(int argc, const char **argv)
 			if (combine_merges && num_compare_stages == 2) {
 				show_combined_diff(&combine.p, 2,
 						   dense_combined_merges,
-						   NULL,
-						   &diff_options);
+						   &rev);
 				free(combine.p.path);
 				continue;
 			}
@@ -194,7 +195,7 @@ int main(int argc, const char **argv)
 			continue;
 		}
 		changed = ce_match_stat(ce, &st, 0);
-		if (!changed && !diff_options.find_copies_harder)
+		if (!changed && !rev.diffopt.find_copies_harder)
 			continue;
 		oldmode = ntohl(ce->ce_mode);
 
@@ -207,7 +208,7 @@ int main(int argc, const char **argv)
 			      ce->sha1, (changed ? null_sha1 : ce->sha1),
 			      ce->name);
 	}
-	diffcore_std(&diff_options);
-	diff_flush(&diff_options);
+	diffcore_std(&rev.diffopt);
+	diff_flush(&rev.diffopt);
 	return 0;
 }

diff-tree.c

@@ -71,6 +71,7 @@ int main(int argc, const char **argv)
 	nr_sha1 = 0;
 	init_revisions(opt);
 	opt->abbrev = 0;
+	opt->diff = 1;
 	argc = setup_revisions(argc, argv, opt, NULL);
 
 	while (--argc > 0) {

diff.h

@@ -6,6 +6,7 @@
 
 #include "tree-walk.h"
 
+struct rev_info;
 struct diff_options;
 
 typedef void (*change_fn_t)(struct diff_options *options,
@@ -70,11 +71,10 @@ struct combine_diff_path {
 	(sizeof(struct combine_diff_path) + \
 	 sizeof(struct combine_diff_parent) * (n) + (l) + 1)
 
-extern int show_combined_diff(struct combine_diff_path *elem, int num_parent,
-			      int dense, const char *header,
-			      struct diff_options *);
+extern void show_combined_diff(struct combine_diff_path *elem, int num_parent,
+			      int dense, struct rev_info *);
 
-extern const char *diff_tree_combined_merge(const unsigned char *sha1, const char *, int, struct diff_options *opt);
+extern void diff_tree_combined_merge(const unsigned char *sha1, int, struct rev_info *);
 
 extern void diff_addremove(struct diff_options *,
 			   int addremove,

git.c

@@ -282,75 +282,22 @@ static int cmd_log_wc(int argc, const char **argv, char **envp,
 		      struct rev_info *rev)
 {
 	struct commit *commit;
-	char *buf = xmalloc(LOGSIZE);
-	const char *commit_prefix = "commit ";
-	int shown = 0;
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
+	rev->verbose_header = 1;
 	argc = setup_revisions(argc, argv, rev, "HEAD");
 
 	if (argc > 1)
 		die("unrecognized argument: %s", argv[1]);
-	if (rev->commit_format == CMIT_FMT_ONELINE)
-		commit_prefix = "";
 
 	prepare_revision_walk(rev);
 	setup_pager();
 	while ((commit = get_revision(rev)) != NULL) {
-		unsigned long ofs = 0;
-
-		if (shown && rev->diff &&
-		    rev->commit_format != CMIT_FMT_ONELINE)
-			putchar('\n');
-
-		ofs = sprintf(buf, "%s", commit_prefix);
-		if (rev->abbrev_commit && rev->abbrev)
-			ofs += sprintf(buf + ofs, "%s",
-				       find_unique_abbrev(commit->object.sha1,
-							  rev->abbrev));
-		else
-			ofs += sprintf(buf + ofs, "%s",
-				       sha1_to_hex(commit->object.sha1));
-		if (rev->parents) {
-			struct commit_list *parents = commit->parents;
-			while (parents) {
-				struct object *o = &(parents->item->object);
-				parents = parents->next;
-				if (o->flags & TMP_MARK)
-					continue;
-				ofs += sprintf(buf + ofs, " %s",
-					       sha1_to_hex(o->sha1));
-				o->flags |= TMP_MARK;
-			}
-			/* TMP_MARK is a general purpose flag that can
-			 * be used locally, but the user should clean
-			 * things up after it is done with them.
-			 */
-			for (parents = commit->parents;
-			     parents;
-			     parents = parents->next)
-				parents->item->object.flags &= ~TMP_MARK;
-		}
-		buf[ofs++] = 
-			(rev->commit_format == CMIT_FMT_ONELINE) ? ' ' : '\n';
-		ofs += pretty_print_commit(rev->commit_format, commit, ~0,
-					   buf + ofs,
-					   LOGSIZE - ofs - 20,
-					   rev->abbrev);
-
-		if (rev->diff) {
-			rev->use_precomputed_header = buf;
-			strcpy(buf + ofs, "\n---\n");
-			log_tree_commit(rev, commit);
-		}
-		else
-			printf("%s\n", buf);
-		shown = 1;
+		log_tree_commit(rev, commit);
 		free(commit->buffer);
 		commit->buffer = NULL;
 	}
-	free(buf);
 	return 0;
 }