threads / patch / 29687

v2cherry-pick: No advice to commit if --no-commit

Subject: [PATCH v2] cherry-pick: No advice to commit if --no-commit

## tl;dr

2 messages between Feb 22, 2012 and Feb 22, 2012. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Phil Hord· Feb 22, 2012, 00:44 UTC · lore

When cherry-pick fails it offers a helpful hint about how to proceed. The hint tells the user how to do the cleanup needed by the conflicted cherry-pick and finish the job after conflict resolution. In case of cherry-pick --no-commit, the hint goes too far. It tells the user to finish up with 'git commit'. That is not what this git-cherry-pick was trying to do in the first place.

Restrict the hint in case of --no-commit to avoid giving this extra advice.

Also, add a test verifying the reduced hint for the --no-commit version of cherry-pick.

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 sequencer.c                     |   17 +++++++++++------
 t/t3507-cherry-pick-conflict.sh |   14 ++++++++++++++
 2 files changed, 25 insertions(+), 6 deletions(-)
Show changes to 2 files +25 −6

sequencer.c, t/t3507-cherry-pick-conflict.sh

diff --git a/sequencer.c b/sequencer.c
index 5fcbcb8..9716c63 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -123,7 +123,7 @@ static void write_cherry_pick_head(struct commit *commit, const char *pseudoref)
 	strbuf_release(&buf);
 }
 
-static void print_advice(int show_hint)
+static void print_advice(int show_hint, struct replay_opts *opts)
 {
 	char *msg = getenv("GIT_CHERRY_PICK_HELP");
 
@@ -138,10 +138,15 @@ static void print_advice(int show_hint)
 		return;
 	}
 
-	if (show_hint)
-		advise(_("after resolving the conflicts, mark the corrected paths\n"
-			 "with 'git add <paths>' or 'git rm <paths>'\n"
-			 "and commit the result with 'git commit'"));
+	if (show_hint) {
+		if (opts->no_commit)
+			advise(_("after resolving the conflicts, mark the corrected paths\n"
+				 "with 'git add <paths>' or 'git rm <paths>'"));
+		else
+			advise(_("after resolving the conflicts, mark the corrected paths\n"
+				 "with 'git add <paths>' or 'git rm <paths>'\n"
+				 "and commit the result with 'git commit'"));
+    }
 }
 
 static void write_message(struct strbuf *msgbuf, const char *filename)
@@ -423,7 +428,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 		      : _("could not apply %s... %s"),
 		      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
 		      msg.subject);
-		print_advice(res == 1);
+		print_advice(res == 1, opts);
 		rerere(opts->allow_rerere_auto);
 	} else {
 		if (!opts->no_commit)
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index ee1659c..0c81b3c 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -59,6 +59,20 @@ test_expect_success 'advice from failed cherry-pick' "
 	test_i18ncmp expected actual
 "
 
+test_expect_success 'advice from failed cherry-pick --no-commit' "
+	pristine_detach initial &&
+
+	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>'
+	EOF
+	test_must_fail git cherry-pick --no-commit picked 2>actual &&
+
+	test_i18ncmp expected actual
+"
+
 test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' '
 	pristine_detach initial &&
 	test_must_fail git cherry-pick picked &&
-- 
1.7.9.267.gda172.dirty
Jonathan Nieder· Feb 22, 2012, 00:51 UTC · re: Phil Hord · lore

Re: [PATCH v2] cherry-pick: No advice to commit if --no-commit

Phil Hord wrote:
>                       In case of cherry-pick --no-commit, the
> hint goes too far. It tells the user to finish up with
> 'git commit'.  That is not what this git-cherry-pick was
> trying to do in the first place.

Especially since if I do try to commit as it says, it will not reuse the old commit message and timestamp like the advice made me suspect it would. :)

[...]
Show 10 quoted lines
> --- a/t/t3507-cherry-pick-conflict.sh
> +++ b/t/t3507-cherry-pick-conflict.sh
> @@ -59,6 +59,20 @@ test_expect_success 'advice from failed cherry-pick' "
>  	test_i18ncmp expected actual
>  "
>  
> +test_expect_success 'advice from failed cherry-pick --no-commit' "
> +	pristine_detach initial &&
> +
> +	picked=\$(git rev-parse --short picked) &&
The escaping here is obnoxiously tricky.  Not your fault, though.
For what it's worth,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.

← back to recent threads