From 7b53b92fdb22b90d2be558db84c725641c4ad170 Mon Sep 17 00:00:00 2001
From: Christian Couder <chriscool@tuxfamily.org>
Date: Wed, 14 Jul 2010 01:28:12 +0200
Subject: revert: report success when using option --strategy

"git cherry-pick foo" has always reported success with
"Finished one cherry-pick" but "cherry-pick --strategy"
does not print anything. So move the code to write that
message from do_recursive_merge() to do_cherry_pick()
so other strategies can share it.

This patch also refactors the code that prints a message
like "Automatic cherry-pick failed. <help message>". This
code was duplicated in both do_recursive_merge() and
do_pick_commit().

To do that, now do_recursive_merge() returns an int to signal
success or failure. And in case of failure we just return 1
from do_pick_commit() instead of doing "exit(1)" from either
do_recursive_merge() or do_pick_commit().

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index 8b9d829..b082bb4 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -311,10 +311,9 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from)
 	return write_ref_sha1(ref_lock, to, "cherry-pick");
 }
 
-static void do_recursive_merge(struct commit *base, struct commit *next,
-			       const char *base_label, const char *next_label,
-			       unsigned char *head, struct strbuf *msgbuf,
-			       char *defmsg)
+static int do_recursive_merge(struct commit *base, struct commit *next,
+			      const char *base_label, const char *next_label,
+			      unsigned char *head, struct strbuf *msgbuf)
 {
 	struct merge_options o;
 	struct tree *result, *next_tree, *base_tree, *head_tree;
@@ -357,14 +356,9 @@ static void do_recursive_merge(struct commit *base, struct commit *next,
 					i++;
 			}
 		}
-		write_message(msgbuf, defmsg);
-		fprintf(stderr, "Automatic %s failed.%s\n",
-			me, help_msg());
-		rerere(allow_rerere_auto);
-		exit(1);
 	}
-	write_message(msgbuf, defmsg);
-	fprintf(stderr, "Finished one %s.\n", me);
+
+	return !clean;
 }
 
 static int do_pick_commit(void)
@@ -375,6 +369,8 @@ static int do_pick_commit(void)
 	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
 	char *defmsg = NULL;
 	struct strbuf msgbuf = STRBUF_INIT;
+	struct strbuf mebuf = STRBUF_INIT;
+	int res;
 
 	if (no_commit) {
 		/*
@@ -470,30 +466,41 @@ static int do_pick_commit(void)
 		}
 	}
 
-	if (!strategy || !strcmp(strategy, "recursive") || action == REVERT)
-		do_recursive_merge(base, next, base_label, next_label,
-				   head, &msgbuf, defmsg);
-	else {
-		int res;
+	strbuf_addstr(&mebuf, me);
+
+	if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
+		res = do_recursive_merge(base, next, base_label, next_label,
+					 head, &msgbuf);
+		write_message(&msgbuf, defmsg);
+	} else {
 		struct commit_list *common = NULL;
 		struct commit_list *remotes = NULL;
+
+		strbuf_addf(&mebuf, " with strategy %s", strategy);
 		write_message(&msgbuf, defmsg);
+
 		commit_list_insert(base, &common);
 		commit_list_insert(next, &remotes);
 		res = try_merge_command(strategy, common,
 					sha1_to_hex(head), remotes);
 		free_commit_list(common);
 		free_commit_list(remotes);
-		if (res) {
-			fprintf(stderr, "Automatic %s with strategy %s failed.%s\n",
-				me, strategy, help_msg());
-			rerere(allow_rerere_auto);
-			exit(1);
-		}
 	}
 
+	if (res) {
+		fprintf(stderr, "Automatic %s failed.%s\n",
+			mebuf.buf, help_msg());
+		rerere(allow_rerere_auto);
+	} else {
+		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
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index f90ed3d..d90b365 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -23,12 +23,36 @@ test_expect_success setup '
 '
 
 test_expect_success 'cherry-pick first..fourth works' '
+	cat <<-\EOF >expected &&
+	Finished one cherry-pick.
+	Finished one cherry-pick.
+	Finished one cherry-pick.
+	EOF
+
+	git checkout -f master &&
+	git reset --hard first &&
+	test_tick &&
+	git cherry-pick first..fourth 2>actual &&
+	git diff --quiet other &&
+	git diff --quiet HEAD other &&
+	test_cmp expected actual &&
+	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
+'
+
+test_expect_success 'cherry-pick --strategy resolve first..fourth works' '
+	cat <<-\EOF >expected &&
+	Finished one cherry-pick with strategy resolve.
+	Finished one cherry-pick with strategy resolve.
+	Finished one cherry-pick with strategy resolve.
+	EOF
+
 	git checkout -f master &&
 	git reset --hard first &&
 	test_tick &&
-	git cherry-pick first..fourth &&
+	git cherry-pick --strategy resolve first..fourth 2>actual &&
 	git diff --quiet other &&
 	git diff --quiet HEAD other &&
+	test_cmp expected actual &&
 	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
 '
 
-- 
cgit v0.10.2-6-g49f6


From 5df16453d4da41e5a36de536acf9acc8aab98041 Mon Sep 17 00:00:00 2001
From: Christian Couder <chriscool@tuxfamily.org>
Date: Wed, 14 Jul 2010 01:28:13 +0200
Subject: revert: refactor commit code into a new run_git_commit() function

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index b082bb4..b84b5b8 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];
@@ -501,33 +527,9 @@ static int do_pick_commit(void)
 	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);
+		res = run_git_commit(defmsg);
 		free(defmsg);
-
 		return res;
 	}
 
-- 
cgit v0.10.2-6-g49f6


From 3b2c5b6df4be14cac6b36cf0db0468efa1f42916 Mon Sep 17 00:00:00 2001
From: Christian Couder <chriscool@tuxfamily.org>
Date: Wed, 14 Jul 2010 01:28:14 +0200
Subject: revert: don't print "Finished one cherry-pick." if commit failed

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index b84b5b8..ec931bd 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -518,24 +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 (!no_commit) {
-		res = run_git_commit(defmsg);
-		free(defmsg);
-		return res;
-	}
-
 	free(defmsg);
 
-	return 0;
+	return res;
 }
 
 static void prepare_revs(struct rev_info *revs)
-- 
cgit v0.10.2-6-g49f6


From f29b5e06b3bdbe3c66a01db8ee67e3c87ae55705 Mon Sep 17 00:00:00 2001
From: Christian Couder <chriscool@tuxfamily.org>
Date: Wed, 14 Jul 2010 01:28:15 +0200
Subject: revert: improve success message by adding abbreviated commit sha1

Instead of saying "Finished one cherry-pick." or "Finished one revert.",
we now say "Finished cherry-pick of commit <abbreviated sha1>." or
"Finished revert of commit <abbreviated sha1>." which is more informative,
especially when cherry-picking or reverting many commits.

In case of failure the message is now "Automatic cherry-pick of commit
<abbreviated sha1> failed." instead of "Automatic cherry-pick failed."

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index ec931bd..e261fb2 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -492,7 +492,8 @@ static int do_pick_commit(void)
 		}
 	}
 
-	strbuf_addstr(&mebuf, me);
+	strbuf_addf(&mebuf, "%s of commit %s", me,
+		    find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
 
 	if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
 		res = do_recursive_merge(base, next, base_label, next_label,
@@ -521,7 +522,7 @@ static int do_pick_commit(void)
 		if (!no_commit)
 			res = run_git_commit(defmsg);
 		if (!res)
-			fprintf(stderr, "Finished one %s.\n", mebuf.buf);
+			fprintf(stderr, "Finished %s.\n", mebuf.buf);
 	}
 
 	strbuf_release(&mebuf);
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index d90b365..6044173 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -23,10 +23,10 @@ test_expect_success setup '
 '
 
 test_expect_success 'cherry-pick first..fourth works' '
-	cat <<-\EOF >expected &&
-	Finished one cherry-pick.
-	Finished one cherry-pick.
-	Finished one cherry-pick.
+	cat <<-EOF >expected &&
+	Finished cherry-pick of commit $(git rev-parse --short second).
+	Finished cherry-pick of commit $(git rev-parse --short third).
+	Finished cherry-pick of commit $(git rev-parse --short fourth).
 	EOF
 
 	git checkout -f master &&
@@ -40,10 +40,10 @@ test_expect_success 'cherry-pick first..fourth works' '
 '
 
 test_expect_success 'cherry-pick --strategy resolve first..fourth works' '
-	cat <<-\EOF >expected &&
-	Finished one cherry-pick with strategy resolve.
-	Finished one cherry-pick with strategy resolve.
-	Finished one cherry-pick with strategy resolve.
+	cat <<-EOF >expected &&
+	Finished cherry-pick of commit $(git rev-parse --short second) with strategy resolve.
+	Finished cherry-pick of commit $(git rev-parse --short third) with strategy resolve.
+	Finished cherry-pick of commit $(git rev-parse --short fourth) with strategy resolve.
 	EOF
 
 	git checkout -f master &&
-- 
cgit v0.10.2-6-g49f6


From 6bc83cdd0b9c7eab365e10d46d1e2acdb769728a Mon Sep 17 00:00:00 2001
From: Christian Couder <chriscool@tuxfamily.org>
Date: Wed, 21 Jul 2010 06:25:02 +0200
Subject: t3508: add check_head_differs_from() helper function and use it

In a test like:

test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"

the --verify does not accomplish much, since the exit status of
git rev-parse is not propagated to test. So it is more robust to
define and use the helper functions check_head_differs_from() and
check_head_equals() as done by this patch.

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 6044173..0f61495 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -4,6 +4,18 @@ test_description='test cherry-picking many commits'
 
 . ./test-lib.sh
 
+check_head_differs_from() {
+	head=$(git rev-parse --verify HEAD) &&
+	arg=$(git rev-parse --verify "$1") &&
+	test "$head" != "$arg"
+}
+
+check_head_equals() {
+	head=$(git rev-parse --verify HEAD) &&
+	arg=$(git rev-parse --verify "$1") &&
+	test "$head" = "$arg"
+}
+
 test_expect_success setup '
 	echo first > file1 &&
 	git add file1 &&
@@ -36,7 +48,7 @@ test_expect_success 'cherry-pick first..fourth works' '
 	git diff --quiet other &&
 	git diff --quiet HEAD other &&
 	test_cmp expected actual &&
-	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
+	check_head_differs_from fourth
 '
 
 test_expect_success 'cherry-pick --strategy resolve first..fourth works' '
@@ -53,7 +65,7 @@ test_expect_success 'cherry-pick --strategy resolve first..fourth works' '
 	git diff --quiet other &&
 	git diff --quiet HEAD other &&
 	test_cmp expected actual &&
-	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
+	check_head_differs_from fourth
 '
 
 test_expect_success 'cherry-pick --ff first..fourth works' '
@@ -63,7 +75,7 @@ test_expect_success 'cherry-pick --ff first..fourth works' '
 	git cherry-pick --ff first..fourth &&
 	git diff --quiet other &&
 	git diff --quiet HEAD other &&
-	test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify fourth)"
+	check_head_equals fourth
 '
 
 test_expect_success 'cherry-pick -n first..fourth works' '
@@ -113,7 +125,7 @@ test_expect_success 'cherry-pick -3 fourth works' '
 	git cherry-pick -3 fourth &&
 	git diff --quiet other &&
 	git diff --quiet HEAD other &&
-	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
+	check_head_differs_from fourth
 '
 
 test_expect_success 'cherry-pick --stdin works' '
@@ -123,7 +135,7 @@ test_expect_success 'cherry-pick --stdin works' '
 	git rev-list --reverse first..fourth | git cherry-pick --stdin &&
 	git diff --quiet other &&
 	git diff --quiet HEAD other &&
-	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
+	check_head_differs_from fourth
 '
 
 test_done
-- 
cgit v0.10.2-6-g49f6


From 130ab8ab9c64b59b367c08a041200b6b75758b91 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 11 Aug 2010 03:36:07 -0500
Subject: =?UTF-8?q?Eliminate=20=E2=80=9CFinished=20cherry-pick/revert?=
 =?UTF-8?q?=E2=80=9D=20message?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When cherry-pick was written (v0.99.6~63, 2005-08-27), “git commit”
was quiet, and the output from cherry-pick provided useful information
about the progress of a rebase.

Now next to the output from “git commit”, the cherry-pick notification
is so much noise (except for the name of the picked commit).

 $ git cherry-pick ..topic
 Finished cherry-pick of 499088b.
 [detached HEAD 17e1ff2] Move glob module to libdpkg
  Author: Guillem Jover <guillem@debian.org>
  8 files changed, 12 insertions(+), 9 deletions(-)
  rename {src => lib/dpkg}/glob.c (98%)
  rename {src => lib/dpkg}/glob.h (93%)
 Finished cherry-pick of ae947e1.
 [detached HEAD 058caa3] libdpkg: Add missing symbols to Versions script
  Author: Guillem Jover <guillem@debian.org>
  1 files changed, 2 insertions(+), 0 deletions(-)
 $

The noise is especially troublesome when sifting through the output of
a rebase or multiple cherry-pick that eventually failed.

With the commit subject, it is already not hard to figure out where
the commit came from.  So drop the “Finished” message.

Cc: Christian Couder <chriscool@tuxfamily.org>
Cc: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/howto/revert-branch-rebase.txt b/Documentation/howto/revert-branch-rebase.txt
index 8c32da6..093c656 100644
--- a/Documentation/howto/revert-branch-rebase.txt
+++ b/Documentation/howto/revert-branch-rebase.txt
@@ -112,25 +112,19 @@ $ git tag pu-anchor pu
 $ git rebase master
 * Applying: Redo "revert" using three-way merge machinery.
 First trying simple merge strategy to cherry-pick.
-Finished one cherry-pick.
 * Applying: Remove git-apply-patch-script.
 First trying simple merge strategy to cherry-pick.
 Simple cherry-pick fails; trying Automatic cherry-pick.
 Removing Documentation/git-apply-patch-script.txt
 Removing git-apply-patch-script
-Finished one cherry-pick.
 * Applying: Document "git cherry-pick" and "git revert"
 First trying simple merge strategy to cherry-pick.
-Finished one cherry-pick.
 * Applying: mailinfo and applymbox updates
 First trying simple merge strategy to cherry-pick.
-Finished one cherry-pick.
 * Applying: Show commits in topo order and name all commits.
 First trying simple merge strategy to cherry-pick.
-Finished one cherry-pick.
 * Applying: More documentation updates.
 First trying simple merge strategy to cherry-pick.
-Finished one cherry-pick.
 ------------------------------------------------
 
 The temporary tag 'pu-anchor' is me just being careful, in case 'git
diff --git a/builtin/revert.c b/builtin/revert.c
index e261fb2..c3d64af 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -521,8 +521,6 @@ static int do_pick_commit(void)
 	} else {
 		if (!no_commit)
 			res = run_git_commit(defmsg);
-		if (!res)
-			fprintf(stderr, "Finished %s.\n", mebuf.buf);
 	}
 
 	strbuf_release(&mebuf);
diff --git a/contrib/examples/git-revert.sh b/contrib/examples/git-revert.sh
index 49f0032..60a05a8 100755
--- a/contrib/examples/git-revert.sh
+++ b/contrib/examples/git-revert.sh
@@ -181,7 +181,6 @@ Conflicts:
 	esac
 	exit 1
 }
-echo >&2 "Finished one $me."
 
 # If we are cherry-pick, and if the merge did not result in
 # hand-editing, we will hit this commit and inherit the original
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 0f61495..8e09fd0 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -35,36 +35,54 @@ test_expect_success setup '
 '
 
 test_expect_success 'cherry-pick first..fourth works' '
-	cat <<-EOF >expected &&
-	Finished cherry-pick of commit $(git rev-parse --short second).
-	Finished cherry-pick of commit $(git rev-parse --short third).
-	Finished cherry-pick of commit $(git rev-parse --short fourth).
+	cat <<-\EOF >expected &&
+	[master OBJID] second
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
+	[master OBJID] third
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
+	[master OBJID] fourth
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
 	EOF
 
 	git checkout -f master &&
 	git reset --hard first &&
 	test_tick &&
-	git cherry-pick first..fourth 2>actual &&
+	git cherry-pick first..fourth >actual &&
 	git diff --quiet other &&
 	git diff --quiet HEAD other &&
-	test_cmp expected actual &&
+
+	sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" <actual >actual.fuzzy &&
+	test_cmp expected actual.fuzzy &&
 	check_head_differs_from fourth
 '
 
 test_expect_success 'cherry-pick --strategy resolve first..fourth works' '
-	cat <<-EOF >expected &&
-	Finished cherry-pick of commit $(git rev-parse --short second) with strategy resolve.
-	Finished cherry-pick of commit $(git rev-parse --short third) with strategy resolve.
-	Finished cherry-pick of commit $(git rev-parse --short fourth) with strategy resolve.
+	cat <<-\EOF >expected &&
+	Trying simple merge.
+	[master OBJID] second
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
+	Trying simple merge.
+	[master OBJID] third
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
+	Trying simple merge.
+	[master OBJID] fourth
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
 	EOF
 
 	git checkout -f master &&
 	git reset --hard first &&
 	test_tick &&
-	git cherry-pick --strategy resolve first..fourth 2>actual &&
+	git cherry-pick --strategy resolve first..fourth >actual &&
 	git diff --quiet other &&
 	git diff --quiet HEAD other &&
-	test_cmp expected actual &&
+	sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" <actual >actual.fuzzy &&
+	test_cmp expected actual.fuzzy &&
 	check_head_differs_from fourth
 '
 
-- 
cgit v0.10.2-6-g49f6


From 2a41dfb03b93c3e5b7d1deca537276aed063a044 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 11 Aug 2010 03:36:41 -0500
Subject: Introduce advise() to print hints

Like error(), warn(), and die(), advise() prints a short message
with a formulaic prefix to stderr.

It is local to revert.c for now because I am not sure this is
the right API (we may want to take an array of advice lines or a
boolean argument for easy suppression of unwanted advice).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index c3d64af..74c1581 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -241,6 +241,15 @@ static void set_author_ident_env(const char *message)
 			sha1_to_hex(commit->object.sha1));
 }
 
+static void advise(const char *advice, ...)
+{
+	va_list params;
+
+	va_start(params, advice);
+	vreportf("hint: ", advice, params);
+	va_end(params);
+}
+
 static char *help_msg(void)
 {
 	struct strbuf helpbuf = STRBUF_INIT;
-- 
cgit v0.10.2-6-g49f6


From 981ff5c37ae20687c98d98c8689d5e89016026d2 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 11 Aug 2010 03:37:24 -0500
Subject: cherry-pick/revert: Use error() for failure message
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When cherry-pick fails after picking a large series of commits, it can
be hard to pick out the error message and advice.  Clarify the error
and prefix it with “error: ” to help.

Before:

	Automatic cherry-pick failed.  [...advice...]

After:

	error: could not apply 7ab78c9... Do something neat.
	[...advice...]

Noticed-by: Thomas Rast <trast@student.ethz.ch>
Encouraged-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index 74c1581..9a7483b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -524,8 +524,11 @@ static int do_pick_commit(void)
 	}
 
 	if (res) {
-		fprintf(stderr, "Automatic %s failed.%s\n",
-			mebuf.buf, help_msg());
+		error("could not %s %s... %s",
+		      action == REVERT ? "revert" : "apply",
+		      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
+		      msg.subject);
+		fprintf(stderr, help_msg());
 		rerere(allow_rerere_auto);
 	} else {
 		if (!no_commit)
-- 
cgit v0.10.2-6-g49f6


From 314eeb6e483350cc7ef0bee0498ff24a12346495 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 11 Aug 2010 03:37:51 -0500
Subject: cherry-pick/revert: Use advise() for hints
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When cherry-pick fails after picking a large series of commits, it can
be hard to pick out the error message and advice.  Prefix the advice
with “hint: ” to help.

Before:

    error: could not apply 7ab78c9... foo
      After resolving the conflicts,
    mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
    and commit the result with:

            git commit -c 7ab78c9a7898b87127365478431289cb98f8d98f

After:

    error: could not apply 7ab78c9... foo
    hint: after resolving the conflicts, mark the corrected paths
    hint: with 'git add <paths>' or 'git rm <paths>'
    hint: and commit the result with 'git commit -c 7ab78c9'

Noticed-by: Thomas Rast <trast@student.ethz.ch>
Encouraged-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index 9a7483b..7f35cc6 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -250,27 +250,21 @@ static void advise(const char *advice, ...)
 	va_end(params);
 }
 
-static char *help_msg(void)
+static void print_advice(void)
 {
-	struct strbuf helpbuf = STRBUF_INIT;
 	char *msg = getenv("GIT_CHERRY_PICK_HELP");
 
-	if (msg)
-		return msg;
+	if (msg) {
+		fprintf(stderr, "%s\n", msg);
+		return;
+	}
 
-	strbuf_addstr(&helpbuf, "  After resolving the conflicts,\n"
-		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\n"
-		"and commit the result");
+	advise("after resolving the conflicts, mark the corrected paths");
+	advise("with 'git add <paths>' or 'git rm <paths>'");
 
-	if (action == CHERRY_PICK) {
-		strbuf_addf(&helpbuf, " with: \n"
-			"\n"
-			"        git commit -c %s\n",
-			    sha1_to_hex(commit->object.sha1));
-	}
-	else
-		strbuf_addch(&helpbuf, '.');
-	return strbuf_detach(&helpbuf, NULL);
+	if (action == CHERRY_PICK)
+		advise("and commit the result with 'git commit -c %s'",
+		       find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
 }
 
 static void write_message(struct strbuf *msgbuf, const char *filename)
@@ -404,7 +398,6 @@ static int do_pick_commit(void)
 	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
 	char *defmsg = NULL;
 	struct strbuf msgbuf = STRBUF_INIT;
-	struct strbuf mebuf = STRBUF_INIT;
 	int res;
 
 	if (no_commit) {
@@ -501,9 +494,6 @@ static int do_pick_commit(void)
 		}
 	}
 
-	strbuf_addf(&mebuf, "%s of commit %s", me,
-		    find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
-
 	if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
 		res = do_recursive_merge(base, next, base_label, next_label,
 					 head, &msgbuf);
@@ -512,7 +502,6 @@ static int do_pick_commit(void)
 		struct commit_list *common = NULL;
 		struct commit_list *remotes = NULL;
 
-		strbuf_addf(&mebuf, " with strategy %s", strategy);
 		write_message(&msgbuf, defmsg);
 
 		commit_list_insert(base, &common);
@@ -528,14 +517,13 @@ static int do_pick_commit(void)
 		      action == REVERT ? "revert" : "apply",
 		      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
 		      msg.subject);
-		fprintf(stderr, help_msg());
+		print_advice();
 		rerere(allow_rerere_auto);
 	} else {
 		if (!no_commit)
 			res = run_git_commit(defmsg);
 	}
 
-	strbuf_release(&mebuf);
 	free_message(&msg);
 	free(defmsg);
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 31e6860..8f6876d 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -113,9 +113,9 @@ REBASE_ROOT=
 AUTOSQUASH=
 NEVER_FF=
 
-GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
-mark the corrected paths with 'git add <paths>', and
-run 'git rebase --continue'"
+GIT_CHERRY_PICK_HELP="\
+hint: after resolving the conflicts, mark the corrected paths
+hint: with 'git add <paths>' and run 'git rebase --continue'"
 export GIT_CHERRY_PICK_HELP
 
 warn () {
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index e25cf80..3f29594 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -38,6 +38,26 @@ test_expect_success 'failed cherry-pick does not advance HEAD' '
 	test "$head" = "$newhead"
 '
 
+test_expect_success 'advice from failed cherry-pick' '
+	git checkout -f initial^0 &&
+	git read-tree -u --reset HEAD &&
+	git clean -d -f -f -q -x &&
+
+	git update-index --refresh &&
+	git diff-index --exit-code HEAD &&
+
+	picked=$(git rev-parse --short picked) &&
+	cat <<-EOF >expected &&
+	error: could not apply $picked... picked
+	hint: after resolving the conflicts, mark the corrected paths
+	hint: with 'git add <paths>' or 'git rm <paths>'
+	hint: and commit the result with 'git commit -c $picked'
+	EOF
+	test_must_fail git cherry-pick picked 2>actual &&
+
+	test_cmp expected actual
+'
+
 test_expect_success 'failed cherry-pick produces dirty index' '
 
 	git checkout -f initial^0 &&
-- 
cgit v0.10.2-6-g49f6


From 997b688769f8d3ea539990ab53d6a44a46a2b2b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
 <avarab@gmail.com>
Date: Wed, 18 Aug 2010 14:36:44 +0000
Subject: tests: fix syntax error in "Use advise() for hints" test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Change the test introduced in the "Use advise() for hints" patch by
Jonathan Nieder not to use '' for quotes inside '' delimited code. It
ended up introducing a file called <paths> to the main git repository.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 3f29594..607bf25 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -38,7 +38,7 @@ test_expect_success 'failed cherry-pick does not advance HEAD' '
 	test "$head" = "$newhead"
 '
 
-test_expect_success 'advice from failed cherry-pick' '
+test_expect_success 'advice from failed cherry-pick' "
 	git checkout -f initial^0 &&
 	git read-tree -u --reset HEAD &&
 	git clean -d -f -f -q -x &&
@@ -46,17 +46,17 @@ test_expect_success 'advice from failed cherry-pick' '
 	git update-index --refresh &&
 	git diff-index --exit-code HEAD &&
 
-	picked=$(git rev-parse --short picked) &&
+	picked=\$(git rev-parse --short picked) &&
 	cat <<-EOF >expected &&
-	error: could not apply $picked... picked
+	error: could not apply \$picked... picked
 	hint: after resolving the conflicts, mark the corrected paths
 	hint: with 'git add <paths>' or 'git rm <paths>'
-	hint: and commit the result with 'git commit -c $picked'
+	hint: and commit the result with 'git commit -c \$picked'
 	EOF
 	test_must_fail git cherry-pick picked 2>actual &&
 
 	test_cmp expected actual
-'
+"
 
 test_expect_success 'failed cherry-pick produces dirty index' '
 
-- 
cgit v0.10.2-6-g49f6