Re: [RFC] Making pathspec limited log play nicer with --first-parent

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Jan 19, 2012, 20:15 UTC
Message-ID: <CA+55aFxucaeX7it_Kj7WV3ZbwCukN+wvbuxqJzh3V5Rxz4ib1g@mail.gmail.com>
In-Reply-To: <7v1uqvjwga.fsf@alter.siamese.dyndns.org>

On Thu, Jan 19, 2012 at 11:58 AM, Junio C Hamano <gitster@pobox.com> wrote:

>
> Comments?

Looks conceptually right, but I do have to admit to hating that new variable.

I don't see a better way to do it, though. Sure, you could do it with just

   if (revs->first_parent_only && pp != &commit->parents)
             break;

and avoid the new variable that way, but that replaces the annoying variable with a pretty subtle thing.

Or we could re-write that while() loop and move the 'parent' variable into it. Like the appended untested thing.

But maybe your patch is better, and my dislike for that parent counter is just irrational.

                  Linus

 revision.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/revision.c b/revision.c
index 064e35108478..5e8eb379c369 100644
--- a/revision.c
+++ b/revision.c
@@ -415,7 +415,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
 
 static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
 {
-	struct commit_list **pp, *parent;
+	struct commit_list **pp;
 	int tree_changed = 0, tree_same = 0;
 
 	/*
@@ -441,8 +441,14 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
 		return;
 
 	pp = &commit->parents;
-	while ((parent = *pp) != NULL) {
-		struct commit *p = parent->item;
+	do {
+		struct commit_list *parent = *pp;
+		struct commit *p;
+
+		if (!parent)
+			break;
+		pp = &parent->next;
+		p = parent->item;
 
 		if (parse_commit(p) < 0)
 			die("cannot simplify commit %s (because of %s)",
@@ -458,7 +464,6 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
 				 * to lose the other branches of this
 				 * merge, so we just keep going.
 				 */
-				pp = &parent->next;
 				continue;
 			}
 			parent->next = NULL;
@@ -487,11 +492,10 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
 		case REV_TREE_OLD:
 		case REV_TREE_DIFFERENT:
 			tree_changed = 1;
-			pp = &parent->next;
 			continue;
 		}
 		die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1));
-	}
+	} while (!revs->first_parent_only);
 	if (tree_changed && !tree_same)
 		return;
 	commit->object.flags |= TREESAME;