Re: [PATCH] git-am: fix "Applying" message when applypatch-hook was run

[Simon Ruderich <simon@ruderich.org>]

applypatch-hook can modify the commit message. Display the updated
commit message instead of the original one.

Signed-off-by: Simon Ruderich <simon@ruderich.org>
---

On Thu, Mar 21, 2013 at 12:36:00AM +0100, Matthieu Moy wrote:
> Please, read SubmittingPatches in the Documentation directory of Git's
> source tree. Your text above should be a commit message (hence, no
> hello), and should not be below the --- line.
>
> Also, read about signed-off-by in the same document.

Hello Matthieu,

Thank you for the suggestions. I've adapted the patch message and
added the signed-off.

> This copy/paste a piece of code that is already a few lines above. Is
> there any reason not to _move_ the assignment to FIRSTLINE after the "if
> test -x "$GIT_DIR"/hooks/applypatch-msg", to avoid duplicating?

No, there wasn't a reason not to move the code. I just wasn't
sure if it had any side effects. But I rechecked and it should
work fine. Updating version attached.

On Wed, Mar 20, 2013 at 04:52:43PM -0700, Junio C Hamano wrote:
> More importantly, is this change even desirable?
>
> The original motivation behind the "Applying:" message was to help
> the user identify which one of the 100+ patches being fed to the
> command, and it was not about showing what we ended up committing.
> When you are running the command interactively, we do grab the
> edited result since f23272f3fd84 (git-am -i: report rewritten title,
> 2007-12-04), but I tend to feel that the automated munging done by
> applypatch-msg falls into a different category.

When I first used the applypatch-msg hook I was confused because
the messages were different and I thought the hook wasn't
working, hence the patch.

I'm not sure how extensive most applypatch-msg hooks modify the
commit message (in my case just a number prepended), but I think
it's more natural and less confusing to see the message which is
being applied.

If the original behaviour is preferred, a short comment in
githooks(5) should prevent any confusion.

Regards
Simon

 git-am.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 202130f..c092855 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -778,13 +778,6 @@ To restore the original branch and stop patching run \"\$cmdline --abort\"."
 	    action=yes
 	fi
 
-	if test -f "$dotest/final-commit"
-	then
-		FIRSTLINE=$(sed 1q "$dotest/final-commit")
-	else
-		FIRSTLINE=""
-	fi
-
 	if test $action = skip
 	then
 		go_next
@@ -797,6 +790,13 @@ To restore the original branch and stop patching run \"\$cmdline --abort\"."
 		stop_here $this
 	fi
 
+	if test -f "$dotest/final-commit"
+	then
+		FIRSTLINE=$(sed 1q "$dotest/final-commit")
+	else
+		FIRSTLINE=""
+	fi
+
 	say "$(eval_gettext "Applying: \$FIRSTLINE")"
 
 	case "$resolved" in
-- 
1.8.2

-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9