Subject: [PATCH 8/8] diff --submodule: split into bite-sized pieces

From: Jonathan Nieder <[email-masked]>
Date: Wed, 16 Mar 2011 02:14:11 -0500

Introduce two functions:

 - prepare_submodule_summary prepares the revision walker
   to list changes in a submodule.  That is, it:

   * finds merge bases between the commits pointed to this
     path from before ("left") and after ("right") the change;
   * checks whether this is a fast-forward or fast-backward;
   * prepares a revision walk to list commits in the symmetric
     difference between the commits at each endpoint.

   It returns nonzero on error.

 - print_submodule_summary runs the revision walk and saves
   the result to a strbuf in --left-right format.

The goal is just readability.  No functional change intended.

Signed-off-by: Jonathan Nieder <[email-masked]>
---
 submodule.c |  103 +++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 61 insertions(+), 42 deletions(-)

diff --git a/submodule.c b/submodule.c
index 6f1c107..e9f2b19 100644
--- a/submodule.c
+++ b/submodule.c
@@ -152,17 +152,69 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,
 		die("bad --ignore-submodules argument: %s", arg);
 }
 
+static int prepare_submodule_summary(struct rev_info *rev, const char *path,
+		struct commit *left, struct commit *right,
+		int *fast_forward, int *fast_backward)
+{
+	struct commit_list *merge_bases, *list;
+
+	init_revisions(rev, NULL);
+	setup_revisions(0, NULL, rev, NULL);
+	rev->left_right = 1;
+	rev->first_parent_only = 1;
+	left->object.flags |= SYMMETRIC_LEFT;
+	add_pending_object(rev, &left->object, path);
+	add_pending_object(rev, &right->object, path);
+	merge_bases = get_merge_bases(left, right, 1);
+	if (merge_bases) {
+		if (merge_bases->item == left)
+			*fast_forward = 1;
+		else if (merge_bases->item == right)
+			*fast_backward = 1;
+	}
+	for (list = merge_bases; list; list = list->next) {
+		list->item->object.flags |= UNINTERESTING;
+		add_pending_object(rev, &list->item->object,
+			sha1_to_hex(list->item->object.sha1));
+	}
+	return prepare_revision_walk(rev);
+}
+
+static void print_submodule_summary(struct rev_info *rev, FILE *f,
+		const char *del, const char *add, const char *reset)
+{
+	static const char format[] = "  %m %s";
+	struct strbuf sb = STRBUF_INIT;
+	struct commit *commit;
+
+	while ((commit = get_revision(rev))) {
+		struct pretty_print_context ctx = {0};
+		ctx.date_mode = rev->date_mode;
+		strbuf_setlen(&sb, 0);
+		if (commit->object.flags & SYMMETRIC_LEFT) {
+			if (del)
+				strbuf_addstr(&sb, del);
+		}
+		else if (add)
+			strbuf_addstr(&sb, add);
+		format_commit_message(commit, format, &sb, &ctx);
+		if (reset)
+			strbuf_addstr(&sb, reset);
+		strbuf_addch(&sb, '\n');
+		fprintf(f, "%s", sb.buf);
+	}
+	strbuf_release(&sb);
+}
+
 void show_submodule_summary(FILE *f, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule,
 		const char *del, const char *add, const char *reset)
 {
 	struct rev_info rev;
-	struct commit *commit, *left = left, *right = right;
-	struct commit_list *merge_bases, *list;
+	struct commit *left = left, *right = right;
 	const char *message = NULL;
 	struct strbuf sb = STRBUF_INIT;
-	static const char *format = "  %m %s";
 	int fast_forward = 0, fast_backward = 0;
 
 	if (is_null_sha1(two))
@@ -175,29 +227,10 @@ void show_submodule_summary(FILE *f, const char *path,
 		 !(right = lookup_commit_reference(two)))
 		message = "(commits not present)";
 
-	if (!message) {
-		init_revisions(&rev, NULL);
-		setup_revisions(0, NULL, &rev, NULL);
-		rev.left_right = 1;
-		rev.first_parent_only = 1;
-		left->object.flags |= SYMMETRIC_LEFT;
-		add_pending_object(&rev, &left->object, path);
-		add_pending_object(&rev, &right->object, path);
-		merge_bases = get_merge_bases(left, right, 1);
-		if (merge_bases) {
-			if (merge_bases->item == left)
-				fast_forward = 1;
-			else if (merge_bases->item == right)
-				fast_backward = 1;
-		}
-		for (list = merge_bases; list; list = list->next) {
-			list->item->object.flags |= UNINTERESTING;
-			add_pending_object(&rev, &list->item->object,
-				sha1_to_hex(list->item->object.sha1));
-		}
-		if (prepare_revision_walk(&rev))
-			message = "(revision walker failed)";
-	}
+	if (!message &&
+	    prepare_submodule_summary(&rev, path, left, right,
+					&fast_forward, &fast_backward))
+		message = "(revision walker failed)";
 
 	if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
 		fprintf(f, "Submodule %s contains untracked content\n", path);
@@ -221,25 +254,11 @@ void show_submodule_summary(FILE *f, const char *path,
 	fwrite(sb.buf, sb.len, 1, f);
 
 	if (!message) {
-		while ((commit = get_revision(&rev))) {
-			struct pretty_print_context ctx = {0};
-			ctx.date_mode = rev.date_mode;
-			strbuf_setlen(&sb, 0);
-			if (commit->object.flags & SYMMETRIC_LEFT) {
-				if (del)
-					strbuf_addstr(&sb, del);
-			}
-			else if (add)
-				strbuf_addstr(&sb, add);
-			format_commit_message(commit, format, &sb, &ctx);
-			if (reset)
-				strbuf_addstr(&sb, reset);
-			strbuf_addch(&sb, '\n');
-			fprintf(f, "%s", sb.buf);
-		}
+		print_submodule_summary(&rev, f, del, add, reset);
 		clear_commit_marks(left, ~0);
 		clear_commit_marks(right, ~0);
 	}
+
 	strbuf_release(&sb);
 }
 
-- 
1.7.4.1