git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Sverre Rabbelier <srabbelier@gmail.com>
Cc: "Thomas Rast" <trast@student.ethz.ch>,
	"Ævar Arnfjörð" <avarab@gmail.com>,
	git@vger.kernel.org, "Jakub Narebski" <jnareb@gmail.com>,
	"Jeff King" <peff@peff.net>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: [PATCH 4/4] cherry-pick/revert: Use advise() for hints
Date: Wed, 11 Aug 2010 03:37:51 -0500	[thread overview]
Message-ID: <20100811083751.GE16495@burratino> (raw)
In-Reply-To: <20100811083100.GA16495@burratino>

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>
---
Thanks for reading.

 builtin/revert.c                |   36 ++++++++++++------------------------
 git-rebase--interactive.sh      |    6 +++---
 t/t3507-cherry-pick-conflict.sh |   20 ++++++++++++++++++++
 3 files changed, 35 insertions(+), 27 deletions(-)

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;
-
-	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");
-
-	if (action == CHERRY_PICK) {
-		strbuf_addf(&helpbuf, " with: \n"
-			"\n"
-			"        git commit -c %s\n",
-			    sha1_to_hex(commit->object.sha1));
+	if (msg) {
+		fprintf(stderr, "%s\n", msg);
+		return;
 	}
-	else
-		strbuf_addch(&helpbuf, '.');
-	return strbuf_detach(&helpbuf, NULL);
+
+	advise("after resolving the conflicts, mark the corrected paths");
+	advise("with 'git add <paths>' or 'git rm <paths>'");
+
+	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 &&
-- 
1.7.2.1.544.ga752d.dirty

  parent reply	other threads:[~2010-08-11  8:39 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-25  0:54 [RFC/PATCH 0/9] commit: more focused advice in the no-changes-staged case Jonathan Nieder
2010-07-25  0:56 ` [PATCH 1/9] wt-status: split wt_status_print into digestible pieces Jonathan Nieder
2010-07-25  0:57 ` [PATCH 2/9] wt-status: split off a function for printing submodule summary Jonathan Nieder
2010-07-25  0:58 ` [PATCH 3/9] commit: split off a function to fetch the default log message Jonathan Nieder
2010-07-25  0:58 ` [PATCH 4/9] commit: split commit -s handling into its own function Jonathan Nieder
2010-07-25  0:59 ` [PATCH 5/9] commit: split off the piece that writes status Jonathan Nieder
2010-07-25  0:59 ` [PATCH 6/9] t7508 (status): modernize style Jonathan Nieder
2010-07-25  8:38   ` Ævar Arnfjörð Bjarmason
2010-07-25  1:00 ` [PATCH 7/9] commit: give empty-commit avoidance code its own function Jonathan Nieder
2010-07-25  1:01 ` [PATCH 8/9] commit --dry-run: give advice on empty amend Jonathan Nieder
2010-07-25  1:02 ` [PATCH 9/9] commit: suppress status summary when no changes staged Jonathan Nieder
2010-08-11  7:11   ` Thomas Rast
2010-08-11  7:30     ` Jonathan Nieder
2010-08-11  7:49       ` [PATCH v2] t6040 (branch tracking): check “status” instead of “commit” Jonathan Nieder
2010-08-12  0:45         ` Ævar Arnfjörð Bjarmason
2010-08-11 12:15       ` [PATCH 9/9] commit: suppress status summary when no changes staged Ævar Arnfjörð Bjarmason
2010-08-11 23:57         ` Jonathan Nieder
2010-08-12  0:05           ` Ævar Arnfjörð Bjarmason
2010-08-12  0:10             ` Jonathan Nieder
2010-07-25  8:54 ` [RFC/PATCH 0/9] commit: more focused advice in the no-changes-staged case Ævar Arnfjörð Bjarmason
2010-07-25  9:22   ` Thomas Rast
2010-07-29 23:51     ` Making error messages stand out (Re: [RFC/PATCH 0/9] commit: more focused advice in the no-changes-staged case) Jonathan Nieder
2010-07-30 18:44       ` Sverre Rabbelier
2010-08-11  8:31         ` [WIP/PATCH 0/4] Re: Making error messages stand out Jonathan Nieder
2010-08-11  8:36           ` [PATCH 1/4] Eliminate “Finished cherry-pick/revert” message Jonathan Nieder
2010-08-11  8:36           ` [PATCH 2/4] Introduce advise() to print hints Jonathan Nieder
2010-08-11  8:37           ` [PATCH 3/4] cherry-pick/revert: Use error() for failure message Jonathan Nieder
2010-08-11  8:37           ` Jonathan Nieder [this message]
2010-08-11  9:21           ` [WIP/PATCH 0/4] Re: Making error messages stand out Nguyen Thai Ngoc Duy
2010-08-11  9:39             ` Matthieu Moy
2010-08-11  9:58               ` Nguyen Thai Ngoc Duy
2010-08-11 17:34           ` Sverre Rabbelier
2010-08-18 14:36           ` [PATCH] tests: fix syntax error in "Use advise() for hints" test Ævar Arnfjörð Bjarmason
2010-08-19  4:30             ` Jonathan Nieder
2010-08-19 12:22               ` Ævar Arnfjörð Bjarmason
2010-08-20 10:13                 ` Raja R Harinath
2010-08-20 14:22                   ` Ævar Arnfjörð Bjarmason
2010-08-20 17:51                     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100811083751.GE16495@burratino \
    --to=jrnieder@gmail.com \
    --cc=avarab@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.com \
    --cc=peff@peff.net \
    --cc=srabbelier@gmail.com \
    --cc=trast@student.ethz.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).