[PATCH 2/2] revert: don't print "Finished one cherry-pick." if commit failed

[PATCH 2/2] revert: don't print "Finished one cherry-pick." if commit failed

[Christian Couder]

And, while at it, refactor commit code into a new run_git_commit()
function.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/revert.c |   67 +++++++++++++++++++++++++-----------------------------
 1 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index b082bb4..ec931bd 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -361,6 +361,32 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
 	return !clean;
 }
 
+/*
+ * If we are cherry-pick, and if the merge did not result in
+ * hand-editing, we will hit this commit and inherit the original
+ * author date and name.
+ * If we are revert, or if our cherry-pick results in a hand merge,
+ * we had better say that the current user is responsible for that.
+ */
+static int run_git_commit(const char *defmsg)
+{
+	/* 6 is max possible length of our args array including NULL */
+	const char *args[6];
+	int i = 0;
+
+	args[i++] = "commit";
+	args[i++] = "-n";
+	if (signoff)
+		args[i++] = "-s";
+	if (!edit) {
+		args[i++] = "-F";
+		args[i++] = defmsg;
+	}
+	args[i] = NULL;
+
+	return run_command_v_opt(args, RUN_GIT_CMD);
+}
+
 static int do_pick_commit(void)
 {
 	unsigned char head[20];
@@ -492,48 +518,17 @@ static int do_pick_commit(void)
 			mebuf.buf, help_msg());
 		rerere(allow_rerere_auto);
 	} else {
-		fprintf(stderr, "Finished one %s.\n", mebuf.buf);
+		if (!no_commit)
+			res = run_git_commit(defmsg);
+		if (!res)
+			fprintf(stderr, "Finished one %s.\n", mebuf.buf);
 	}
 
 	strbuf_release(&mebuf);
 	free_message(&msg);
-
-	if (res)
-		return 1;
-
-	/*
-	 *
-	 * If we are cherry-pick, and if the merge did not result in
-	 * hand-editing, we will hit this commit and inherit the original
-	 * author date and name.
-	 * If we are revert, or if our cherry-pick results in a hand merge,
-	 * we had better say that the current user is responsible for that.
-	 */
-
-	if (!no_commit) {
-		/* 6 is max possible length of our args array including NULL */
-		const char *args[6];
-		int res;
-		int i = 0;
-
-		args[i++] = "commit";
-		args[i++] = "-n";
-		if (signoff)
-			args[i++] = "-s";
-		if (!edit) {
-			args[i++] = "-F";
-			args[i++] = defmsg;
-		}
-		args[i] = NULL;
-		res = run_command_v_opt(args, RUN_GIT_CMD);
-		free(defmsg);
-
-		return res;
-	}
-
 	free(defmsg);
 
-	return 0;
+	return res;
 }
 
 static void prepare_revs(struct rev_info *revs)
-- 
1.7.2.rc1.213.gf3fb81

Re: [PATCH 2/2] revert: don't print "Finished one cherry-pick." if commit failed

[Jonathan Nieder]

Christian Couder wrote:

> And, while at it, refactor commit code into a new run_git_commit()
> function.

Probably should be two patches.

> +++ b/builtin/revert.c
> @@ -492,48 +518,17 @@ static int do_pick_commit(void)
>  			mebuf.buf, help_msg());
>  		rerere(allow_rerere_auto);
>  	} else {
> -		fprintf(stderr, "Finished one %s.\n", mebuf.buf);
> +		if (!no_commit)
> +			res = run_git_commit(defmsg);
> +		if (!res)
> +			fprintf(stderr, "Finished one %s.\n", mebuf.buf);
>  	}

Ack for this part.  Thanks.

Re: [PATCH 2/2] revert: don't print "Finished one cherry-pick." if commit failed

[Christian Couder]

On Monday 12 July 2010 18:39:35 Jonathan Nieder wrote:
> Christian Couder wrote:
> > And, while at it, refactor commit code into a new run_git_commit()
> > function.
> 
> Probably should be two patches.

Ok, I will split it in two patches.

> > +++ b/builtin/revert.c
> > @@ -492,48 +518,17 @@ static int do_pick_commit(void)
> > 
> >  			mebuf.buf, help_msg());
> >  		
> >  		rerere(allow_rerere_auto);
> >  	
> >  	} else {
> > 
> > -		fprintf(stderr, "Finished one %s.\n", mebuf.buf);
> > +		if (!no_commit)
> > +			res = run_git_commit(defmsg);
> > +		if (!res)
> > +			fprintf(stderr, "Finished one %s.\n", mebuf.buf);
> > 
> >  	}
> 
> Ack for this part.  Thanks.

Thanks,
Christian.