[PATCH] hooks: add signature to the top of the commit message

[PATCH] hooks: add signature to the top of the commit message

[Kaartic Sivaraam]

The sample hook to prepare the commit message before
a commit allows users to opt-in to add the signature
to the commit message. The signature is added at a place
that isn't consistent with the "-s" option of "git commit".
Further, it could go out of view in certain cases.

Add the signature to the top of the commit message as it's
more appropriate and consistent with the "-s" option of git
commit.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 The change might seem to be bit of an hack, but it seems
 worth it (at least to me). I hope I haven't used any
 commands that aren't portable.

 templates/hooks--prepare-commit-msg.sample | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 86b8f227e..3c8f5a53d 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -33,4 +33,7 @@ case "$2,$3" in
 esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
-# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
+# SOB_TO_ADD='
+#
+# '$SOB
+# grep -qs "^$SOB" "$1" || (echo "$SOB_TO_ADD" | cat - "$1" >temp-template && mv temp-template "$1")
-- 
2.11.0

Re: [PATCH] hooks: add signature to the top of the commit message

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> The sample hook to prepare the commit message before
> a commit allows users to opt-in to add the signature
> to the commit message. The signature is added at a place
> that isn't consistent with the "-s" option of "git commit".
> Further, it could go out of view in certain cases.
>
> Add the signature to the top of the commit message as it's
> more appropriate and consistent with the "-s" option of git
> commit.
>
> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
> ---
>  The change might seem to be bit of an hack, but it seems
>  worth it (at least to me). I hope I haven't used any
>  commands that aren't portable.

It does look like a hack.  I was wondering if "interpret-trailers"
is mature enough and can be used for this by now.  Also the big
comment before these examples say that this one you are updating is
"rarely a good idea", though.

By the way, the one that is still actually enabled is no longer
needed.  The commit template generated internally was corrected some
time ago not to add the "Conflicts:" section without commenting it
out.

Have you tried "merge", "cherry-pick" and "commit --amend" with this
patch on (meaning, with the "add sob at the top" logic in your actual
hook that is enabled in your repository)?

>  templates/hooks--prepare-commit-msg.sample | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
> index 86b8f227e..3c8f5a53d 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -33,4 +33,7 @@ case "$2,$3" in
>  esac
>  
>  # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
> -# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
> +# SOB_TO_ADD='
> +#
> +# '$SOB
> +# grep -qs "^$SOB" "$1" || (echo "$SOB_TO_ADD" | cat - "$1" >temp-template && mv temp-template "$1")

Re: [PATCH] hooks: add signature to the top of the commit message

[Kaartic Sivaraam]

On Fri, 2017-06-30 at 09:44 -0700, Junio C Hamano wrote:
> It does look like a hack.  I was wondering if "interpret-trailers"
> is mature enough and can be used for this by now.
It does look promising except for a few differences from the hook which
I'll explain in the following mail.

>  Also the big
> comment before these examples say that this one you are updating is
> "rarely a good idea", though.
I think the comment specifies the "editability" of the sign-off
drawback but AFAIK there's no way as of now to add such trailers to
commit messages. Until there's a solution for adding trailers that
aren't editable manually, I think it's not a bad idea to use a hook for
it and rely on the user to be true to their  conscience. Moreover the
script is commented out by default anyway.

The portion of the hook for adding sign-off could be removed in case
any configuration to enable "-s" option for "git commit" by default
existed. I'm not aware of any.

> By the way, the one that is still actually enabled is no longer
> needed.  The commit template generated internally was corrected some
> time ago not to add the "Conflicts:" section without commenting it
> out.
> 
I'll send in another patch that removes it but it seems removing it
would leave sample hook without anything turned on by default. That
doesn't sound fine, does it? 

Any other possible tweaks that could be done to the commit message
using the "prepare-commit-msg" hook, that's not done by git, and could
be left uncommented by default?

How about a script that removes the "Please enter your.." message from
the comments if it exists?

> Have you tried "merge", "cherry-pick" and "commit --amend" with this
> patch on (meaning, with the "add sob at the top" logic in your actual
> hook that is enabled in your repository)?
> 
Yes, they don't work with this patch. I have dropped it. More on it in
the following mail.

"git intepret-trailers" vs. "sed script" to add the signature

[Kaartic Sivaraam]

On Sat, 2017-07-01 at 19:45 +0530, Kaartic Sivaraam wrote:
> On Fri, 2017-06-30 at 09:44 -0700, Junio C Hamano wrote:
> > It does look like a hack.  I was wondering if "interpret-trailers"
> > is mature enough and can be used for this by now.
> 
> It does look promising except for a few differences from the hook
> which
> I'll explain in the following mail.

interpet-trailers
=================

After enabling the script I tried the following (shown here as a diff)
to add the signature with "interpret-trailers",

    diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
    index 6473bcacd..9f8cbe7fd 100755
    --- a/templates/hooks--prepare-commit-msg.sample
    +++ b/templates/hooks--prepare-commit-msg.sample
    @@ -33,4 +33,4 @@ case "$2,$3" in
     esac
     
     SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
    -grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
    +git interpret-trailers --in-place --trailer "$SOB" "$1"

It adds the signature if it's not present in the following cases,

* commit
* merge
* commit --amend
* commit -F
* cherry-pick

It's pretty good in adding the signature except that it's not in line
with "git commit -s" whose resulting "spacing" (new lines before and
after) as shown in the editor is given below,

> 
> 
> Signed-off-by: Test <hello@example.org>
> 
> # Please enter the commit message for your changes. Lines starting
> # with '#' will be ignored, and an empty message aborts the commit.
> ...

The spacing of "git interpret-trailers" in the editor for the relevant
cases are,

commit
------

> 
> Signed-off-by: Test <hello@example.org>
> # Please enter the commit message for your changes. Lines starting
> # with '#' will be ignored, and an empty message aborts the commit.
> ...


commit --amend
--------------

> Empty commit to test amending 
>  
> Signed-off-by: Test <hello@example.org> 
>  
> # Please enter the commit message for your changes. Lines starting 
> # with '#' will be ignored, and an empty message aborts the commit. 
> ...


merge
-----
> Merge branch 'hook-test' into hook-test-merge
> 
> Signed-off-by: Test <hello@example.org>
> 
> # Please enter a commit message to explain why this merge is necessary,
> # especially if it merges an updated upstream into a topic branch.
> #
> # Lines starting with '#' will be ignored, and an empty message aborts
> # the commit.

So, it seems that excepting for 'commit' it has quite a nice spacing. I
guess we could add something like the following to fix that,

    # Add new line after SOB in case of "git commit"
    NEW_LINE='\
    '
    if [ -z "$2" ]
    then
      sed -i "1i$NEW_LINE" "$1"
    fi


sed-script
==========
I also tried to add the signature that immitates the "-s" option
of "git commit" using "sed" but it works only in following cases,

* commit
* commit --amend
* merge

It doesn't seem to work in cases where user doesn't edit the message
using the editor. I'm not sure why.

I'm not including a patch of my manual way here as "git interpret-
trailers" (with the fix added) seems quite promising (at least to me).

I'll send a typical patch that uses "git interpret-headers" as a
follow-up.

-- 
Regards,
Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>

[PATCH/RFC] hooks: add signature using "interpret-trailers"

[Kaartic Sivaraam]

The sample hook to prepare the commit message before
a commit allows users to opt-in to add the signature
to the commit message. The signature is added at a place
that isn't consistent with the "-s" option of "git commit".
Further, it could go out of view in certain cases.

Add the signature in a way similar to "-s" option of
"git commit" using git's interpret-trailers command.

It works well in all cases except when the user invokes
"git commit" without any arguments. In that case manually
add a new line after the first line to ensure it's consistent
with the output of "-s" option.

While at it, name the input parameters to improve readability
of script.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 I've tried the various commands that I could think of and it 
 seems to work well. I guess it's safe to use.

 templates/hooks--prepare-commit-msg.sample | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 86b8f227e..c44a0a056 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -20,17 +20,27 @@
 # The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
-case "$2,$3" in
+COMMIT_MSG_FILE=$1
+COMMIT_SOURCE=$2
+SHA1=$3
+NEW_LINE='\
+'
+
+case "$COMMIT_SOURCE,$SHA1" in
   merge,)
-    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
+    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$COMMIT_MSG_FILE" ;;
 
 # ,|template,)
 #   @PERL_PATH@ -i.bak -pe '
 #      print "\n" . `git diff --cached --name-status -r`
-#	 if /^#/ && $first++ == 0' "$1" ;;
+#	 if /^#/ && $first++ == 0' "$COMMIT_MSG_FILE" ;;
 
   *) ;;
 esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
-# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
+# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
+# if [ -z "$COMMIT_SOURCE" ]
+# then
+#  sed -i "1i$NEW_LINE" "$COMMIT_MSG_FILE"
+# fi
-- 
2.11.0

Re: [PATCH] hooks: add signature to the top of the commit message

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

>> By the way, the one that is still actually enabled is no longer
>> needed.  The commit template generated internally was corrected some
>> time ago not to add the "Conflicts:" section without commenting it
>> out.
>> 
> I'll send in another patch that removes it but it seems removing it
> would leave sample hook without anything turned on by default. That
> doesn't sound fine, does it?

Actually I was wondering if it is a good idea to remove it, as it
seems to have outlived its usefulness.

Re: [PATCH] hooks: add signature to the top of the commit message

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:
>
>>> By the way, the one that is still actually enabled is no longer
>>> needed.  The commit template generated internally was corrected some
>>> time ago not to add the "Conflicts:" section without commenting it
>>> out.
>>>
>> I'll send in another patch that removes it but it seems removing it
>> would leave sample hook without anything turned on by default. That
>> doesn't sound fine, does it?
>
> Actually I was wondering if it is a good idea to remove it, as it
> seems to have outlived its usefulness.

Personally, I like the comfort of seeing the Conflicts: list, but if others 
have indicated otherwise...
--
Philip 

Re: [PATCH] hooks: add signature to the top of the commit message

[Kaartic Sivaraam]

On Sat, 2017-07-01 at 10:36 -0700, Junio C Hamano wrote:
> Actually I was wondering if it is a good idea to remove it, as it
> seems to have outlived its usefulness.
It does seem  to be a good idea but it would leave the 'prepare-commit-
msg' hook with no scripts that could be used by just activating it.
That's why I thought of adding a script that removes the "Please enter
your.." message from the comments if it exists. A typical patch will
follow.

Enabling the "prepare-commit-msg" hook with the patch that follows
would do have following result,

Before,

    # Please enter the commit message for your changes. Lines starting
    # with '#' will be ignored, and an empty message aborts the commit.
    # On branch hook-test-merge
    # Changes to be committed:
    #    	    new file:   commit-msg
    #

After,


    # On branch hook-test-merge
    # Changes to be committed:
    #    	    new file:   commit-msg
    #


A typical consequence for "git commit --amend" would be,

Before,

    Test commit

    # Please enter the commit message for your changes. Lines starting
    # with '#' will be ignored, and an empty message aborts the commit.
    #
    # Date:      Sun Jul 2 00:11:28 2017 +0530


After,

    Test commit

    #
    # Date:      Sun Jul 2 00:11:28 2017 +0530
    #

Re: [PATCH] hooks: add signature to the top of the commit message

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

> From: "Junio C Hamano" <gitster@pobox.com>
>> Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:
>>
>>>> By the way, the one that is still actually enabled is no longer
>>>> needed.  The commit template generated internally was corrected some
>>>> time ago not to add the "Conflicts:" section without commenting it
>>>> out.
>>>>
>>> I'll send in another patch that removes it but it seems removing it
>>> would leave sample hook without anything turned on by default. That
>>> doesn't sound fine, does it?
>>
>> Actually I was wondering if it is a good idea to remove it, as it
>> seems to have outlived its usefulness.
>
> Personally, I like the comfort of seeing the Conflicts: list, but if
> others have indicated otherwise...

Oh, I think you misread the discussion while arriving from the
sideways.  My "it" in the "remove it" refers to the sample
prepare-commit-msg hook; among the three examples in that hook, only
one of them is enabled but that one was to comment out the "Conflicts"
section in the log message editor.  These days, that section already
appears in a commented-out form without the help of that hook, so
there is nothing useful in there---hence a suggestion for removal of
the sample.

Re: [PATCH] hooks: add signature to the top of the commit message

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> On Sat, 2017-07-01 at 10:36 -0700, Junio C Hamano wrote:
>> Actually I was wondering if it is a good idea to remove it, as it
>> seems to have outlived its usefulness.
> It does seem  to be a good idea but it would leave the 'prepare-commit-
> msg' hook with no scripts that could be used by just activating it.
> That's why I thought of adding a script that removes the "Please enter
> your.." message from the comments if it exists.

That sounds like a sample that is there not because it would be
useful, but because we couldn't think of any useful example.

IOW, I view it just as useful as a sample that does

	#!/bin/sh
	echo "# useless cruft" >>"$1"

whose sole value is to demonstrate that you could affect what you
see in the editor by modifying "$1".

Re: [PATCH] hooks: add signature to the top of the commit message

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> From: "Junio C Hamano" <gitster@pobox.com>
>>> Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:
>>>
>>>>> By the way, the one that is still actually enabled is no longer
>>>>> needed.  The commit template generated internally was corrected some
>>>>> time ago not to add the "Conflicts:" section without commenting it
>>>>> out.
>>>>>
>>>> I'll send in another patch that removes it but it seems removing it
>>>> would leave sample hook without anything turned on by default. That
>>>> doesn't sound fine, does it?
>>>
>>> Actually I was wondering if it is a good idea to remove it, as it
>>> seems to have outlived its usefulness.
>>
>> Personally, I like the comfort of seeing the Conflicts: list, but if
>> others have indicated otherwise...
>
> Oh, I think you misread the discussion while arriving from the
> sideways.  My "it" in the "remove it" refers to the sample
> prepare-commit-msg hook; among the three examples in that hook, only
> one of them is enabled but that one was to comment out the "Conflicts"
> section in the log message editor.  These days, that section already
> appears in a commented-out form without the help of that hook, so
> there is nothing useful in there---hence a suggestion for removal of
> the sample.
>

Thanks, yes I had misread it. I hadn't managed the time to follow the 
details. Problem solved.

Philip 

Re: [PATCH] hooks: add signature to the top of the commit message

[Kaartic Sivaraam]

On Sat, 2017-07-01 at 13:31 -0700, Junio C Hamano wrote:
> That sounds like a sample that is there not because it would be
> useful, but because we couldn't think of any useful example.
> 
> IOW, I view it just as useful as a sample that does
> 
> 	#!/bin/sh
> 	echo "# useless cruft" >>"$1"
> 
> whose sole value is to demonstrate that you could affect what you
> see in the editor by modifying "$1".
I thought it would be useful as it could serve as a simple example
about what could be done using hooks and further would help save some
vertical spacing for users who are acquainted with that message and
wouldn't want to see it anymore.

Sending a typical patch as a follow-up.

Apart from that I would like to share a patch of my attempt to give
"notes" about a commit while writing the commit message itself. It was
an attempt to combine three hooks at once to achieve the outcome. I
just wanted to share it so that it might be of use to someone.

-- 
Kaartic

[PATCH/RFC] hooks: replace irrelevant hook sample

[Kaartic Sivaraam]

The pre-commit-msg hook sample has an example that comments
the "Conflicts:" part of the merge commmit. It isn't relevant
anymore as it's done by default since 261f315b ("merge & sequencer:
turn "Conflicts:" hint into a comment", 2014-08-28).

Add an alternative example that replaces it to ensure there's
atleast one example that could be used just by enabling the hook.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 This patch assumes that a previous patch that touched the commit
 template [1] has been appled. Else it could remove the information
 about the branch from the comments.

 [1]:http://public-inbox.org/git/%3C20170630121221.3327-2-kaarticsivaraam91196@gmail.com%3E/raw
 
 templates/hooks--prepare-commit-msg.sample | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 86b8f227e..03783b3de 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -9,8 +9,9 @@
 #
 # To enable this hook, rename this file to "prepare-commit-msg".
 
-# This hook includes three examples.  The first comments out the
-# "Conflicts:" part of a merge commit.
+# This hook includes three examples.  The first removes the
+# comment lines that start with "# Please enter the" and
+# "# with" from the commit message.
 #
 # The second includes the output of "git diff --name-status -r"
 # into the message, just before the "git status" output.  It is
@@ -20,17 +21,17 @@
 # The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
-case "$2,$3" in
-  merge,)
-    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
+sed -i '/# Please enter the .*/d' "$1"
+sed -i '/# with .*/d' "$1"
 
+# case "$2,$3" in
 # ,|template,)
 #   @PERL_PATH@ -i.bak -pe '
 #      print "\n" . `git diff --cached --name-status -r`
 #	 if /^#/ && $first++ == 0' "$1" ;;
-
-  *) ;;
-esac
+#
+#  *) ;;
+# esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
2.11.0

[PATCH] hooks: add script to HOOKS that allows adding notes from commit message

[Kaartic Sivaraam]

This script is a little different from others in that it uses THREE
hooks to acheive it's goal, which is to allow users to add notes for
a commit while writing the commit message. It's working isn't
guaranteed even if one of the hooks aren't executed.

It currently works in the following scenartios,

* commit
* commit --amend
* commit -t

From what I'm aware of it's not possible to make it work
with 'merge' as there isn't any hook that checks the message
of a merge commit. The notes are extracted from the commit message
using the hook that checks the commit message.

Use cases other than those specified here aren't tested. It might/
might not be possible to use this script in other cases (with/without
modifications).

The usage of various hooks are as follows,

prepare-commit-msg: Used to add the template in which the user
                    enters the notes for the commit. Leaving it
                    doesn't add any notes for the commit.

commit-msg: Used to extract the notes from the commit message if
            it exists and save it to a file. It removes the the
            content between the template to avoid it being added
            to the commit message.

post-commit: Used to check if the file with notes for the last commit
             exists and if it does, uses it to add notes for the commit.

             Note: It's often times an file with empty lines when notes
             aren't given for a commit. It would not be added as a note
             for the commit as 'git notes' doesn't accept files with
             empty lines.
---
 templates/hooks--commit-msg.sample         | 28 +++++++++++++++++++
 templates/hooks--post-commit.sample        | 18 +++++++++++++
 templates/hooks--prepare-commit-msg.sample | 43 +++++++++++++++++++++++-------
 3 files changed, 80 insertions(+), 9 deletions(-)
 create mode 100644 templates/hooks--post-commit.sample

diff --git a/templates/hooks--commit-msg.sample b/templates/hooks--commit-msg.sample
index b58d1184a..b15b72906 100755
--- a/templates/hooks--commit-msg.sample
+++ b/templates/hooks--commit-msg.sample
@@ -15,6 +15,34 @@
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
 
+# START : NOTES FOR A COMMIT FROM COMMIT MESSAGE
+# Script that allows you to enter notes for a commit while entering the
+# commit message in the editor.
+#
+# **NOTE** : It depends on the "prepare-commit-msg" and "post-commit"
+# hooks. They are to be enabled for it to function correctly.
+NOTES_HEADER='---------- NOTES : START ----------'
+NOTES_FOOTER='----------- NOTES : END -----------'
+GIT_DIR=`git rev-parse --git-dir`
+TEMP_NOTES_FILE="$GIT_DIR/.git/.NOTES_FOR_HEAD"
+
+save_notes_to_file() {
+  sed -n "/$NOTES_HEADER/,/$NOTES_FOOTER/ {
+    /$NOTES_HEADER/n
+    /$NOTES_FOOTER/ !{
+      p
+    }
+  }" "$1" >"$TEMP_NOTES_FILE"
+}
+
+delete_notes_from_message() {
+  sed -i "/$NOTES_HEADER/,/$NOTES_FOOTER/ d" "$1"
+}
+
+grep -q -e "^$NOTES_HEADER" "$1" && save_notes_to_file "$1" && delete_notes_from_message "$1"
+# END : NOTES FOR A COMMIT FROM COMMIT MESSAGE
+
+
 # This example catches duplicate Signed-off-by lines.
 
 test "" = "$(grep '^Signed-off-by: ' "$1" |
diff --git a/templates/hooks--post-commit.sample b/templates/hooks--post-commit.sample
new file mode 100644
index 000000000..ed99f114a
--- /dev/null
+++ b/templates/hooks--post-commit.sample
@@ -0,0 +1,18 @@
+#! /bin/sh
+# The following script allows you to enter notes for a commit
+# while entering the commit message in the editor.
+# 
+# **NOTE** : It depends on the "commit-msg" and "pre-commit-msg"
+# hooks. They are to be enabled for it to function correctly.
+
+
+# START : NOTES FOR A COMMIT FROM COMMIT MESSAGE
+GIT_DIR=`git rev-parse --git-dir`
+TEMP_NOTES_FILE="$GIT_DIR/.NOTES-FOR-HEAD"
+
+if [ -e "TEMP_NOTES_FILE" ]
+then
+  git notes add -F $TEMP_NOTES_FILE
+  rm $TEMP_NOTES_FILE
+fi
+# END : NOTES FOR A COMMIT FROM COMMIT MESSAGE
diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 86b8f227e..364f66677 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -9,8 +9,13 @@
 #
 # To enable this hook, rename this file to "prepare-commit-msg".
 
-# This hook includes three examples.  The first comments out the
-# "Conflicts:" part of a merge commit.
+# This hook includes three examples.  The first one is a script
+# that allows you to enter notes for a commit while entering the
+# commit message in the editor.
+#
+# **NOTE** : The script described above depends on the "commit-msg"
+# and "post-commit" hooks. They are to be enabled for it to function
+# correctly.
 #
 # The second includes the output of "git diff --name-status -r"
 # into the message, just before the "git status" output.  It is
@@ -20,17 +25,37 @@
 # The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
-case "$2,$3" in
-  merge,)
-    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
+# START : NOTES FOR A COMMIT FROM COMMIT MESSAGE
+REQUEST_NOTES_TEMPLATE='\
+# Add notes about this commit in the NOTES template below.\
+# DO NOT MODIFY the template to prevent surprises\
+# If you do not wish to add notes for this commit, leave it untouched\
+---------- NOTES : START ----------\
+\
+\
+----------- NOTES : END -----------\
+'
+COMMENT_IDENTIFIER='^# Please enter the .*'
+
+add_notes_template() {
+  sed -i "/$COMMENT_IDENTIFIER/ i\
+$REQUEST_NOTES_TEMPLATE" "$1"
+}
+
+case "$2," in
+  ,|commit,|template,) add_notes_template "$1"
+                       ;;
+esac
+# END : NOTES FOR A COMMIT FROM COMMIT MESSAGE
 
-# ,|template,)
+# case "$2,$3" in
+#  ,|template,)
 #   @PERL_PATH@ -i.bak -pe '
 #      print "\n" . `git diff --cached --name-status -r`
 #	 if /^#/ && $first++ == 0' "$1" ;;
-
-  *) ;;
-esac
+#
+#  *) ;;
+# esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
2.11.0

Re: "git intepret-trailers" vs. "sed script" to add the signature

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> So, it seems that excepting for 'commit' it has quite a nice spacing. I
> guess we could add something like the following to fix that,
>
>     # Add new line after SOB in case of "git commit"
>     NEW_LINE='\
>     '
>     if [ -z "$2" ]
>     then
>       sed -i "1i$NEW_LINE" "$1"
>     fi

Isn't "sed -i" GNUism that is not portable?

> I'll send a typical patch that uses "git interpret-headers" as a
> follow-up.

When you say a "typical" patch, what do you exactly mean?  Does
anybody else send typical patches (or atypical ones for that matter)
to the list?

Re: "git intepret-trailers" vs. "sed script" to add the signature

[Kaartic Sivaraam]

On Mon, 2017-07-03 at 09:58 -0700, Junio C Hamano wrote:
> Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:
> 
> > So, it seems that excepting for 'commit' it has quite a nice
> > spacing. I
> > guess we could add something like the following to fix that,
> > 
> >     # Add new line after SOB in case of "git commit"
> >     NEW_LINE='\
> >     '
> >     if [ -z "$2" ]
> >     then
> >       sed -i "1i$NEW_LINE" "$1"
> >     fi
> 
> Isn't "sed -i" GNUism that is not portable?
> 
It does seem to be the case. Then the alternative would be the
following,

    if [ -z "$2" ]
    then
      sed -e "1i$NEW_LINE" "$1" >"sed-output.temp"
      mv "sed-output.temp" "$1"
    fi

Actually the GNU's sed documentation tricked me into believing '-i'
wasn't a GNU extension. The '-i' option works with the '--posix' option
of GNU sed which made me believe it isn't an extension.

> > I'll send a typical patch that uses "git interpret-headers" as a
> > follow-up.
> 
> When you say a "typical" patch, what do you exactly mean?  Does
> anybody else send typical patches (or atypical ones for that matter)
> to the list?
> 
I apologise for the inconsistent wordings. I try to mean a patch which
I'm not sure is acceptable (or) not. I guess that's [PATCH/RFC] in the
language of this list. I'm not acquainted to the wordings as I'm an
"off-list" person trying to help and get help! I'll try to use
consistent wordings as far as I could. Once again, please excuse my
ignorance.

-- 
Kaartic

Re: "git intepret-trailers" vs. "sed script" to add the signature

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> On Mon, 2017-07-03 at 09:58 -0700, Junio C Hamano wrote:
>> Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:
>> 
>> > I'll send a typical patch that uses "git interpret-headers" as a
>> > follow-up.
>> 
>> When you say a "typical" patch, what do you exactly mean?  Does
>> anybody else send typical patches (or atypical ones for that matter)
>> to the list?
>> 
> I apologise for the inconsistent wordings. I try to mean a patch which
> I'm not sure is acceptable (or) not. I guess that's [PATCH/RFC] in the
> language of this list. I'm not acquainted to the wordings as I'm an
> "off-list" person trying to help and get help! I'll try to use
> consistent wordings as far as I could. Once again, please excuse my
> ignorance.

Oh no need to apologise for "ignorance"; that goes both ways. I
wasn't familiar with a phrase you used "a typical patch" (which I
suspected that is something you may be used to from your involvement
in other development community) and showing _my_ ignorance and
asking for help to clarify, so that both of us can understand each
other better.

My reading of your response is that it is a normal patch that
proposes a change, as opposed to the final version of a patch meant
for inclusion, after it has been discussed here and everybody
supports---let me know if I am still not reading you correctly.

Thanks.

[PATCH] hooks: replace irrelevant hook sample

[Kaartic Sivaraam]

The pre-commit-msg hook sample has an example that comments
the "Conflicts:" part of the merge commmit. It isn't relevant
anymore as it's done by default since 261f315b ("merge & sequencer:
turn "Conflicts:" hint into a comment", 2014-08-28).

Add an alternative example that replaces it. This ensures there's
at the least a simple example that illustrates what could be done
using the hook just by enabling it.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 This one's the final proposed patch for this simple hook. I have
 requested for suggestions for alternative scripts in the mailing
 list[1]. In case something interesting turns out, we could use that
 instead of this one.

 [1]: http://public-inbox.org/git/1499273152.16389.2.camel@gmail.com/T/#u

 templates/hooks--prepare-commit-msg.sample | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 86b8f227e..0e2ccef11 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -9,8 +9,9 @@
 #
 # To enable this hook, rename this file to "prepare-commit-msg".
 
-# This hook includes three examples.  The first comments out the
-# "Conflicts:" part of a merge commit.
+# This hook includes three examples.  The first one removes three
+# comment lines starting from the line that has the words
+# "# Please enter the" in it's beginning.
 #
 # The second includes the output of "git diff --name-status -r"
 # into the message, just before the "git status" output.  It is
@@ -20,17 +21,20 @@
 # The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
-case "$2,$3" in
-  merge,)
-    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
+sed -e '/^# Please enter the .*/ {
+  N
+  N
+  d
+}' "$1" >'.sed-output.temp' && mv '.sed-output.temp' "$1"
 
+# case "$2,$3" in
 # ,|template,)
 #   @PERL_PATH@ -i.bak -pe '
 #      print "\n" . `git diff --cached --name-status -r`
 #	 if /^#/ && $first++ == 0' "$1" ;;
-
-  *) ;;
-esac
+#
+#  *) ;;
+# esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
2.13.2.14.gb9bd03b20.dirty

[PATCH] hooks: add signature using "interpret-trailers"

[Kaartic Sivaraam]

The sample hook to prepare the commit message before
a commit allows users to opt-in to add the signature
to the commit message. The signature is added at a place
that isn't consistent with the "-s" option of "git commit".
Further, it could go out of view in certain cases.

Add the signature in a way similar to "-s" option of
"git commit" using git's interpret-trailers command.

It works well in all cases except when the user invokes
"git commit" without any arguments. In that case manually
add a new line after the first line to ensure it's consistent
with the output of "-s" option.

While at it, name the input parameters to improve readability
of script.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
  Removed the GNUism in sed command. In case no other changes are
  required this one's the final patch.

 templates/hooks--prepare-commit-msg.sample | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 86b8f22..f404f8f 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -20,17 +20,28 @@
 # The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
-case "$2,$3" in
+COMMIT_MSG_FILE=$1
+COMMIT_SOURCE=$2
+SHA1=$3
+NEW_LINE='\
+'
+SED_TEMP_FILE='.sed-output.temp'
+
+case "$COMMIT_SOURCE,$SHA1" in
   merge,)
-    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
+    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$COMMIT_MSG_FILE" ;;
 
 # ,|template,)
 #   @PERL_PATH@ -i.bak -pe '
 #      print "\n" . `git diff --cached --name-status -r`
-#	 if /^#/ && $first++ == 0' "$1" ;;
+#	 if /^#/ && $first++ == 0' "$COMMIT_MSG_FILE" ;;
 
   *) ;;
 esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
-# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
+# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
+# if [ -z "$COMMIT_SOURCE" ]
+# then
+#  sed -e "1i$NEW_LINE" "$COMMIT_MSG_FILE" >"$SED_TEMP_FILE" && mv "$SED_TEMP_FILE "$COMMIT_MSG_FILE"
+# fi
-- 
2.7.4

[PATCH] hooks: add signature using "interpret-trailers"

[Kaartic Sivaraam]

The sample hook to prepare the commit message before
a commit allows users to opt-in to add the signature
to the commit message. The signature is added at a place
that isn't consistent with the "-s" option of "git commit".
Further, it could go out of view in certain cases.

Add the signature in a way similar to "-s" option of
"git commit" using git's interpret-trailers command.

It works well in all cases except when the user invokes
"git commit" without any arguments. In that case manually
add a new line after the first line to ensure it's consistent
with the output of "-s" option.

While at it, name the input parameters to improve readability
of script.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 Added a close quote that got missed in the previous patch.

 templates/hooks--prepare-commit-msg.sample | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 86b8f227e..4ddab7896 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -20,17 +20,28 @@
 # The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
-case "$2,$3" in
+COMMIT_MSG_FILE=$1
+COMMIT_SOURCE=$2
+SHA1=$3
+NEW_LINE='\
+'
+SED_TEMP_FILE='.sed-output.temp'
+
+case "$COMMIT_SOURCE,$SHA1" in
   merge,)
-    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
+    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$COMMIT_MSG_FILE" ;;
 
 # ,|template,)
 #   @PERL_PATH@ -i.bak -pe '
 #      print "\n" . `git diff --cached --name-status -r`
-#	 if /^#/ && $first++ == 0' "$1" ;;
+#	 if /^#/ && $first++ == 0' "$COMMIT_MSG_FILE" ;;
 
   *) ;;
 esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
-# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
+# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
+# if [ -z "$COMMIT_SOURCE" ]
+# then
+#  sed -e "1i$NEW_LINE" "$COMMIT_MSG_FILE" >"$SED_TEMP_FILE" && mv "$SED_TEMP_FILE" "$COMMIT_MSG_FILE"
+# fi
-- 
2.13.1.78.gbaba0bc7d

Re: [PATCH] hooks: add signature using "interpret-trailers"

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> The sample hook to prepare the commit message before
> a commit allows users to opt-in to add the signature
> to the commit message. The signature is added at a place
> that isn't consistent with the "-s" option of "git commit".
> Further, it could go out of view in certain cases.
>
> Add the signature in a way similar to "-s" option of
> "git commit" using git's interpret-trailers command.

OK.

> It works well in all cases except when the user invokes
> "git commit" without any arguments. In that case manually
> add a new line after the first line to ensure it's consistent
> with the output of "-s" option.

OK (but its execution can be simplified).

> While at it, name the input parameters to improve readability
> of script.

Good.

>
> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
> ---
>  Added a close quote that got missed in the previous patch.
>
>  templates/hooks--prepare-commit-msg.sample | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
> index 86b8f227e..4ddab7896 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -20,17 +20,28 @@
>  # The third example adds a Signed-off-by line to the message, that can
>  # still be edited.  This is rarely a good idea.
>  
> -case "$2,$3" in
> +COMMIT_MSG_FILE=$1
> +COMMIT_SOURCE=$2
> +SHA1=$3
> +NEW_LINE='\
> +'
> +SED_TEMP_FILE='.sed-output.temp'

Move the latter two variable definitions to where they matter,
i.e. the commented-out part that computes and adds $SOB.

> +case "$COMMIT_SOURCE,$SHA1" in
>    merge,)
> -    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
> +    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$COMMIT_MSG_FILE" ;;
>  
>  # ,|template,)
>  #   @PERL_PATH@ -i.bak -pe '
>  #      print "\n" . `git diff --cached --name-status -r`
> -#	 if /^#/ && $first++ == 0' "$1" ;;
> +#	 if /^#/ && $first++ == 0' "$COMMIT_MSG_FILE" ;;
>  
>    *) ;;
>  esac
>  
>  # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
> -# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
> +# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
> +# if [ -z "$COMMIT_SOURCE" ]
> +# then

In our codebase (see Documentation/CodingGuidelines) we tend to
prefer "test" over "[".  I.e.

	if test -z "$COMMIT_SOURCE"
	then

> +#  sed -e "1i$NEW_LINE" "$COMMIT_MSG_FILE" >"$SED_TEMP_FILE" && mv "$SED_TEMP_FILE" "$COMMIT_MSG_FILE"

This looks like a more complex way to say

	{
		echo 
		cat "$COMMIT_MSG_FILE"
	} >"$SED_TEMP_FILE" &&
	mv "$SED_TEMP_FILE" "$COMMIT_MSG_FILE"

I wondered if it is safe to use a single hardcoded "$SED_TEMP_FILE",
but I think it is OK.

> +# fi

Thanks.

Re: [PATCH] hooks: replace irrelevant hook sample

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> +sed -e '/^# Please enter the .*/ {
> +  N
> +  N
> +  d
> +}' "$1" >'.sed-output.temp' && mv '.sed-output.temp' "$1"

Three things that caught my eyes:

 - Between "git commit --cleanup=strip" and "git commit --cleanup=verbatim",
   lines that make up this initial instruction section are different.

 - "git grep 'Please enter the '" finds that this string is subject
   to translation, so the pattern may not match (in which case it
   will be a no-op without doing any harm, which is OK).

 - core.commentChar can be set to something other than '#', so the
   pattern may not match (I do not offhand know if that may cause a
   wrong line to match, causing harm, or not).

As merely an example, it probably is OK to say "this won't work if
you are not using the C locale, and/or you are using custom
core.commentChar".  So if we disregard the latter two, I would think

    sed -e '/^# Please enter the commit message /,/^#$/d'

may be simpler to reason about to achieve the same goal.  

Thanks.

Re: [PATCH] hooks: add signature using "interpret-trailers"

[Ramsay Jones]

On 05/07/17 18:35, Kaartic Sivaraam wrote:
> The sample hook to prepare the commit message before
> a commit allows users to opt-in to add the signature
> to the commit message. The signature is added at a place
> that isn't consistent with the "-s" option of "git commit".
> Further, it could go out of view in certain cases.
> 
> Add the signature in a way similar to "-s" option of
> "git commit" using git's interpret-trailers command.
> 
> It works well in all cases except when the user invokes
> "git commit" without any arguments. In that case manually
> add a new line after the first line to ensure it's consistent
> with the output of "-s" option.
> 
> While at it, name the input parameters to improve readability
> of script.

I assume each occurrence of 'signature' in the commit message,
including the subject, should be 'sign-off' (or Signed-off-by)
instead. Yes?

(when I hear 'signature', I think GPG signature).

ATB,
Ramsay Jones

Re: [PATCH] hooks: add signature using "interpret-trailers"

[Kaartic Sivaraam]

On Wed, 2017-07-05 at 21:14 +0100, Ramsay Jones wrote:
> 
> On 05/07/17 18:35, Kaartic Sivaraam wrote:
> > The sample hook to prepare the commit message before
> > a commit allows users to opt-in to add the signature
> > to the commit message. The signature is added at a place
> > that isn't consistent with the "-s" option of "git commit".
> > Further, it could go out of view in certain cases.
> > 
> > Add the signature in a way similar to "-s" option of
> > "git commit" using git's interpret-trailers command.
> > 
> > It works well in all cases except when the user invokes
> > "git commit" without any arguments. In that case manually
> > add a new line after the first line to ensure it's consistent
> > with the output of "-s" option.
> > 
> > While at it, name the input parameters to improve readability
> > of script.
> 
> I assume each occurrence of 'signature' in the commit message,
> including the subject, should be 'sign-off' (or Signed-off-by)
> instead. Yes?
> 
Yes. Thanks for pointing out a possible way in which the message could
be misinterpreted.

-- 
Kaartic

Re: [PATCH] hooks: replace irrelevant hook sample

[Kaartic Sivaraam]

On Wed, 2017-07-05 at 12:50 -0700, Junio C Hamano wrote:
> Three things that caught my eyes:
> 
>  - Between "git commit --cleanup=strip" and "git commit --
> cleanup=verbatim",
>    lines that make up this initial instruction section are different.
> 
>  - "git grep 'Please enter the '" finds that this string is subject
>    to translation, so the pattern may not match (in which case it
>    will be a no-op without doing any harm, which is OK).
> 
>  - core.commentChar can be set to something other than '#', so the
>    pattern may not match (I do not offhand know if that may cause a
>    wrong line to match, causing harm, or not).
> 
> As merely an example, it probably is OK to say "this won't work if
> you are not using the C locale, and/or you are using custom
> core.commentChar".  So if we disregard the latter two, I would think
> 
>     sed -e '/^# Please enter the commit message /,/^#$/d'
> 
> may be simpler to reason about to achieve the same goal.  
> 
Thanks for enlightening me about this. I thought sed was greedy with
address spaces the same way it's greedy with regex.

    sed -e '/^# Please enter the commit message /,/^#$/d'


This command does seem to work regardless of the cleanup mode used.

That said, in case my interpretation that "'prepare-commit-msg' hook is
not to be shipped due to it's uselessness" is correct, the reply of
this mail as a whole seems to contradict it.

Should I work on this patch and another related one (he one that
modifies the signature part of the hook) or
should I drop it ?

IOW, would this patch likely make the hook useful again?

-- 
Kaartic

Re: [PATCH] hooks: replace irrelevant hook sample

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> That said, in case my interpretation that "'prepare-commit-msg' hook is
> not to be shipped due to it's uselessness" is correct, the reply of
> this mail as a whole seems to contradict it.

That is because I wear multiple hats, because I try to help in
different ways, and because open source is not a battle to see whose
idea is more right, but is a cooperative process to find a better
solution together.

As a fellow contributor, I do not think that removing the hint that
is commented out, which is meant to be helpful to users while in
their editor and which will be removed after the editor finishes
anyway, is a useful enough example to keep the now otherwise useless
sample hook.  But as the maintainer, I can see that you are still
making sincere efforts to come up with a useful example and improve
the end-user experience, and more importantly, I haven't heard from
other people what they think---the only thing I have are different
opinions from two people.  That is why I am not deciding and telling
you to go find another area to hack in.

At the same time, I found that your implementation of the idea, i.e.
removal of the commented-out hint, can be improved.  With an
improved implementation of the proposed solution, it may have a
better chance to be supported by others on the list, and equally
importantly, if it turns out that other people do support what this
patch tries to do, i.e. keep the sample hook alive by replacing the
now useless examples with this one, we would have a better
implementation of it.  And that is something I can help with, while
I, the maintainer, is waiting.

Oh, by the way, what the maintainer is waiting for is not just "me
too"s; this is not exactly a "having more people wins" democracy.

Re: [PATCH] hooks: replace irrelevant hook sample

[Kaartic Sivaraam]

On Fri, 2017-07-07 at 08:05 -0700, Junio C Hamano wrote:
> That is because I wear multiple hats, because I try to help in
> different ways, and because open source is not a battle to see whose
> idea is more right, but is a cooperative process to find a better
> solution together.
> 
Thanks for helping and being kind!

> As a fellow contributor, I do not think that removing the hint that
> is commented out, which is meant to be helpful to users while in
> their editor and which will be removed after the editor finishes
> anyway, is a useful enough example to keep the now otherwise useless
> sample hook.  But as the maintainer, I can see that you are still
> making sincere efforts to come up with a useful example and improve
> the end-user experience, and more importantly, I haven't heard from
> other people what they think---the only thing I have are different
> opinions from two people.  That is why I am not deciding and telling
> you to go find another area to hack in.
> 
> At the same time, I found that your implementation of the idea, i.e.
> removal of the commented-out hint, can be improved.  With an
> improved implementation of the proposed solution, it may have a
> better chance to be supported by others on the list, and equally
> importantly, if it turns out that other people do support what this
> patch tries to do, i.e. keep the sample hook alive by replacing the
> now useless examples with this one, we would have a better
> implementation of it.  And that is something I can help with, while
> I, the maintainer, is waiting.
> 
So, I'll improve it and of course wait for any replies. BTW, thanks for
clearing off the little confusion I had. I'm not used to these
multiple-hat replies from single person. Thanks for exposing me to it.

> Oh, by the way, what the maintainer is waiting for is not just "me
> too"s; this is not exactly a "having more people wins" democracy.
Got it.

-- 
Quote: “The most valuable person on any team is the person who makes
everyone else on the team more valuable”

Regards,
Kaartic

[PATCH 1/2] hooks: replace irrelevant hook sample

[Kaartic Sivaraam]

The pre-commit-msg hook sample has an example that comments
the "Conflicts:" part of the merge commmit. It isn't relevant
anymore as it's done by default since 261f315b ("merge & sequencer:
turn "Conflicts:" hint into a comment", 2014-08-28).

Add an alternative example that replaces it. This ensures there's
at the least a simple example that illustrates what could be done
using the hook just by enabling it.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 I have made the second patch depend on the first one to avoid
 conflicts that may occur. Further, it was meaningful to join
 the two patches as they would go together (or) not at all.

 templates/hooks--prepare-commit-msg.sample | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 86b8f227e..5a638ebda 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -9,8 +9,9 @@
 #
 # To enable this hook, rename this file to "prepare-commit-msg".
 
-# This hook includes three examples.  The first comments out the
-# "Conflicts:" part of a merge commit.
+# This hook includes three examples.  The first one removes three
+# comment lines starting from the line that has the words
+# "# Please enter the" in it's beginning.
 #
 # The second includes the output of "git diff --name-status -r"
 # into the message, just before the "git status" output.  It is
@@ -20,17 +21,16 @@
 # The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
-case "$2,$3" in
-  merge,)
-    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
+sed -e '/^. Please enter the commit message /,/^#$/d' "$1" >'.sed-output.temp' && mv '.sed-output.temp' "$1"
 
+# case "$2,$3" in
 # ,|template,)
 #   @PERL_PATH@ -i.bak -pe '
 #      print "\n" . `git diff --cached --name-status -r`
 #	 if /^#/ && $first++ == 0' "$1" ;;
-
-  *) ;;
-esac
+#
+#  *) ;;
+# esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
2.13.2.879.g2ab69f31a.dirty

[PATCH 2/2] hooks: add signature using "interpret-trailers"

[Kaartic Sivaraam]

The sample hook to prepare the commit message before
a commit allows users to opt-in to add the signature
to the commit message. The signature is added at a place
that isn't consistent with the "-s" option of "git commit".
Further, it could go out of view in certain cases.

Add the signature in a way similar to "-s" option of
"git commit" using git's interpret-trailers command.

It works well in all cases except when the user invokes
"git commit" without any arguments. In that case manually
add a new line after the first line to ensure it's consistent
with the output of "-s" option.

While at it, name the input parameters to improve readability
of script.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 templates/hooks--prepare-commit-msg.sample | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 5a638ebda..7495078cb 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -21,7 +21,12 @@
 # The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
-sed -e '/^. Please enter the commit message /,/^#$/d' "$1" >'.sed-output.temp' && mv '.sed-output.temp' "$1"
+COMMIT_MSG_FILE=$1
+COMMIT_SOURCE=$2
+SHA1=$3
+
+SED_OUTPUT_TEMP='.sed-output-temp'
+sed -e '/^. Please enter the commit message /,/^#$/d' "$COMMIT_MSG_FILE" >"$SED_OUTPUT_TEMP" && mv "$SED_OUTPUT_TEMP" "$1"
 
 # case "$2,$3" in
 # ,|template,)
@@ -32,5 +37,14 @@ sed -e '/^. Please enter the commit message /,/^#$/d' "$1" >'.sed-output.temp' &
 #  *) ;;
 # esac
 
+# TEMP_FILE='.template-temp'
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
-# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
+# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
+# if test -z "$COMMIT_SOURCE"
+# then
+#   {
+#     echo
+#     cat "$COMMIT_MSG_FILE"
+#   } >"$TEMP_FILE"
+#   mv "$TEMP_FILE" "$COMMIT_MSG_FILE"
+# fi
-- 
2.13.2.879.g2ab69f31a.dirty

Re: [PATCH 1/2] hooks: replace irrelevant hook sample

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> The pre-commit-msg hook sample has an example that comments
> the "Conflicts:" part of the merge commmit. It isn't relevant
> anymore as it's done by default since 261f315b ("merge & sequencer:
> turn "Conflicts:" hint into a comment", 2014-08-28).
>
> Add an alternative example that replaces it. This ensures there's
> at the least a simple example that illustrates what could be done
> using the hook just by enabling it.
>
> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
> ---
>  I have made the second patch depend on the first one to avoid
>  conflicts that may occur. Further, it was meaningful to join
>  the two patches as they would go together (or) not at all.

Whether the two samples these patches implement are useful ones or
not, the early part of 2/2 that starts using named variables instead
of positional ones is an improvement.  It makes it easier to see
what is going on.

Patch 2/2 as posted forgets to update some references to $1, $2 and
$3, both in the actual code and also commented out part, e.g.

    sed -e ... "$COMMIT_MSG_FILE" >"$SED_OUTPUT_TEMP" && mv "$SED_OUTPUT_TEMP" "$1"
    # case "$2,$3" in

In the first one, $COMMIT_MSG_FILE and $1 refer to the same thing;
they should be consistent.

If these updates were to be done as multiple patches, the change to
use named not positional variables, without changing anything else,
should come second, I think, after the one that simply removes the
now-useless "Conflicts:" sample from the script, as a preparatory
clean-up step.  You can build other things like hints-removal and
sign-off with interpret-trailers on top of these two steps.

Have you considered using "@PERL_PATH@ -i" instead of "sed" in this
step, by the way?  That would allow you not to worry about the
temporary file left behind (e.g. what happens when somebody else
runs this in a shared repository setting, creating and leaving the
temporary file that you may not be able to write into later because
her umask is tighter, and then you try to make a commit).

Thanks.

>  templates/hooks--prepare-commit-msg.sample | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
> index 86b8f227e..5a638ebda 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -9,8 +9,9 @@
>  #
>  # To enable this hook, rename this file to "prepare-commit-msg".
>  
> -# This hook includes three examples.  The first comments out the
> -# "Conflicts:" part of a merge commit.
> +# This hook includes three examples.  The first one removes three
> +# comment lines starting from the line that has the words
> +# "# Please enter the" in it's beginning.
>  #
>  # The second includes the output of "git diff --name-status -r"
>  # into the message, just before the "git status" output.  It is
> @@ -20,17 +21,16 @@
>  # The third example adds a Signed-off-by line to the message, that can
>  # still be edited.  This is rarely a good idea.
>  
> -case "$2,$3" in
> -  merge,)
> -    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
> +sed -e '/^. Please enter the commit message /,/^#$/d' "$1" >'.sed-output.temp' && mv '.sed-output.temp' "$1"
>  
> +# case "$2,$3" in
>  # ,|template,)
>  #   @PERL_PATH@ -i.bak -pe '
>  #      print "\n" . `git diff --cached --name-status -r`
>  #	 if /^#/ && $first++ == 0' "$1" ;;
> -
> -  *) ;;
> -esac
> +#
> +#  *) ;;
> +# esac
>  
>  # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
>  # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"

[PATCH 1/4] hook: cleanup script

[Kaartic Sivaraam]

Prepare the 'preare-commit-msg' sample script for
upcoming changes. Preparation includes removal of
an example that has outlived it's purpose. The example
is the one that comments the "Conflicts:" part of a
merge commit message. It isn't relevant anymore as
it's done by default since 261f315b ("merge & sequencer:
turn "Conflicts:" hint into a comment", 2014-08-28).

Further remove the relevant comments from the sample script
and update the documentation.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 Documentation/githooks.txt                 |  3 ---
 templates/hooks--prepare-commit-msg.sample | 20 ++++++++------------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 706091a56..fdc01aa25 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -121,9 +121,6 @@ it is not suppressed by the `--no-verify` option.  A non-zero exit
 means a failure of the hook and aborts the commit.  It should not
 be used as replacement for pre-commit hook.
 
-The sample `prepare-commit-msg` hook that comes with Git comments
-out the `Conflicts:` part of a merge's commit message.
-
 commit-msg
 ~~~~~~~~~~
 
diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 86b8f227e..b8ba335cf 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -9,8 +9,7 @@
 #
 # To enable this hook, rename this file to "prepare-commit-msg".
 
-# This hook includes three examples.  The first comments out the
-# "Conflicts:" part of a merge commit.
+# This hook includes three examples.
 #
 # The second includes the output of "git diff --name-status -r"
 # into the message, just before the "git status" output.  It is
@@ -20,17 +19,14 @@
 # The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
-case "$2,$3" in
-  merge,)
-    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
 
-# ,|template,)
-#   @PERL_PATH@ -i.bak -pe '
-#      print "\n" . `git diff --cached --name-status -r`
-#	 if /^#/ && $first++ == 0' "$1" ;;
-
-  *) ;;
-esac
+# case "$2,$3" in
+#  ,|template,)
+#    @PERL_PATH@ -i.bak -pe '
+#       print "\n" . `git diff --cached --name-status -r`
+# 	 if /^#/ && $first++ == 0' "$1" ;;
+#  *) ;;
+# esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
2.13.2.957.g457671ade

[PATCH 2/4] hook: name the positional variables

[Kaartic Sivaraam]

It's always nice to have named variables instead of
positional variables as they communicate their purpose
well.

Appropriately name the positional variables of the hook
to make it easier to see what's going on.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 templates/hooks--prepare-commit-msg.sample | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index b8ba335cf..708f0e92c 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -19,14 +19,17 @@
 # The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
+COMMIT_MSG_FILE=$1
+COMMIT_SOURCE=$2
+SHA1=$3
 
-# case "$2,$3" in
+# case "$COMMIT_SOURCE,$SHA1" in
 #  ,|template,)
 #    @PERL_PATH@ -i.bak -pe '
 #       print "\n" . `git diff --cached --name-status -r`
-# 	 if /^#/ && $first++ == 0' "$1" ;;
+# 	 if /^#/ && $first++ == 0' "$COMMIT_MSG_FILE" ;;
 #  *) ;;
 # esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
-# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
+# grep -qs "^$SOB" "$COMMIT_MSG_FILE" || echo "$SOB" >> "$COMMIT_MSG_FILE"
-- 
2.13.2.957.g457671ade

[PATCH 3/4] hook: add signature using "interpret-trailers"

[Kaartic Sivaraam]

The sample hook to prepare the commit message before
a commit allows users to opt-in to add the signature
to the commit message. The signature is added at a place
that isn't consistent with the "-s" option of "git commit".
Further, it could go out of view in certain cases.

Add the signature in a way similar to "-s" option of
"git commit" using git's interpret-trailers command.

It works well in all cases except when the user invokes
"git commit" without any arguments. In that case manually
add a new line after the first line to ensure it's consistent
with the output of "-s" option.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 templates/hooks--prepare-commit-msg.sample | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 708f0e92c..a15d6d634 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -32,4 +32,8 @@ SHA1=$3
 # esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
-# grep -qs "^$SOB" "$COMMIT_MSG_FILE" || echo "$SOB" >> "$COMMIT_MSG_FILE"
+# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
+# if test -z "$COMMIT_SOURCE"
+# then
+#   @PERL_PATH@ -i.bak -pe 'print "\n" if($first_line++ == 0)' "$COMMIT_MSG_FILE"
+# fi
-- 
2.13.2.957.g457671ade

[PATCH 4/4] hook: add a simple first example

[Kaartic Sivaraam]

Add a simple example that replaces an outdated example
that was removed. This ensures that there's at the least
a simple example that illustrates what could be done
using the hook just by enabling it.

Also, update the documentation.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 I made an attempt to make the second example work with amending 
 with the aim of making it suitable for usage out of the box. It
 seems that it's not easy to make it work as the status of a file
 cannot be determined correctly when the index while amending
 introduces changes to a file that has a change in the commit being
 amended.

 Is there any way in which the second example could be made to work with
 amending without much effort? I'm asking this assuming something might
 have happened, since the script was added, that could ease the task.

 Documentation/githooks.txt                 | 3 +++
 templates/hooks--prepare-commit-msg.sample | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index fdc01aa25..59f38efba 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -121,6 +121,9 @@ it is not suppressed by the `--no-verify` option.  A non-zero exit
 means a failure of the hook and aborts the commit.  It should not
 be used as replacement for pre-commit hook.
 
+The sample `prepare-commit-msg` hook that comes with Git removes the
+help message found in the commented portion of the commit template.
+
 commit-msg
 ~~~~~~~~~~
 
diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index a15d6d634..a84c3e5a8 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -9,7 +9,8 @@
 #
 # To enable this hook, rename this file to "prepare-commit-msg".
 
-# This hook includes three examples.
+# This hook includes three examples.  The first one removes the
+# "# Please enter the commit message..." help message.
 #
 # The second includes the output of "git diff --name-status -r"
 # into the message, just before the "git status" output.  It is
@@ -23,6 +24,8 @@ COMMIT_MSG_FILE=$1
 COMMIT_SOURCE=$2
 SHA1=$3
 
+@PERL_PATH@ -i.bak -ne 'print unless(m/^. Please enter the commit message/..m/^#$/)' "$COMMIT_MSG_FILE"
+
 # case "$COMMIT_SOURCE,$SHA1" in
 #  ,|template,)
 #    @PERL_PATH@ -i.bak -pe '
-- 
2.13.2.957.g457671ade

Re: [PATCH 3/4] hook: add signature using "interpret-trailers"

[Ramsay Jones]

On 10/07/17 15:17, Kaartic Sivaraam wrote:
> The sample hook to prepare the commit message before
> a commit allows users to opt-in to add the signature
> to the commit message. The signature is added at a place
> that isn't consistent with the "-s" option of "git commit".
> Further, it could go out of view in certain cases.
> 
> Add the signature in a way similar to "-s" option of
> "git commit" using git's interpret-trailers command.
> 
> It works well in all cases except when the user invokes
> "git commit" without any arguments. In that case manually
> add a new line after the first line to ensure it's consistent
> with the output of "-s" option.
> 

Again, s/signature/sign-off/g, or similar (including subject line).

ATB,
Ramsay Jones


> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
> ---
>  templates/hooks--prepare-commit-msg.sample | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
> index 708f0e92c..a15d6d634 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -32,4 +32,8 @@ SHA1=$3
>  # esac
>  
>  # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
> -# grep -qs "^$SOB" "$COMMIT_MSG_FILE" || echo "$SOB" >> "$COMMIT_MSG_FILE"
> +# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
> +# if test -z "$COMMIT_SOURCE"
> +# then
> +#   @PERL_PATH@ -i.bak -pe 'print "\n" if($first_line++ == 0)' "$COMMIT_MSG_FILE"
> +# fi
> 

Re: [PATCH 1/4] hook: cleanup script

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> Prepare the 'preare-commit-msg' sample script for
> upcoming changes. Preparation includes removal of
> an example that has outlived it's purpose. The example
> is the one that comments the "Conflicts:" part of a
> merge commit message. It isn't relevant anymore as
> it's done by default since 261f315b ("merge & sequencer:
> turn "Conflicts:" hint into a comment", 2014-08-28).
>
> Further remove the relevant comments from the sample script
> and update the documentation.
>
> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
> ---
>  Documentation/githooks.txt                 |  3 ---
>  templates/hooks--prepare-commit-msg.sample | 20 ++++++++------------
>  2 files changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 706091a56..fdc01aa25 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -121,9 +121,6 @@ it is not suppressed by the `--no-verify` option.  A non-zero exit
>  means a failure of the hook and aborts the commit.  It should not
>  be used as replacement for pre-commit hook.
>  
> -The sample `prepare-commit-msg` hook that comes with Git comments
> -out the `Conflicts:` part of a merge's commit message.
> -
>  commit-msg
>  ~~~~~~~~~~

Makes sense.

>  
> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
> index 86b8f227e..b8ba335cf 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -9,8 +9,7 @@
>  #
>  # To enable this hook, rename this file to "prepare-commit-msg".
>  
> -# This hook includes three examples.  The first comments out the
> -# "Conflicts:" part of a merge commit.
> +# This hook includes three examples.

Didn't we just remove one, reducing the total number to two?  If so,
the second and the third below would need to be promoted to the
first and the second, I think.

>  # The second includes the output of "git diff --name-status -r"
>  # into the message, just before the "git status" output.  It is
> @@ -20,17 +19,14 @@
>  # The third example adds a Signed-off-by line to the message, that can
>  # still be edited.  This is rarely a good idea.
>  
> -case "$2,$3" in
> -  merge,)
> -    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
>  
> -# ,|template,)
> -#   @PERL_PATH@ -i.bak -pe '
> -#      print "\n" . `git diff --cached --name-status -r`
> -#	 if /^#/ && $first++ == 0' "$1" ;;
> -
> -  *) ;;
> -esac
> +# case "$2,$3" in
> +#  ,|template,)
> +#    @PERL_PATH@ -i.bak -pe '
> +#       print "\n" . `git diff --cached --name-status -r`
> +# 	 if /^#/ && $first++ == 0' "$1" ;;
> +#  *) ;;
> +# esac
>  
>  # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
>  # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"

Re: [PATCH 2/4] hook: name the positional variables

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> It's always nice to have named variables instead of
> positional variables as they communicate their purpose
> well.
>
> Appropriately name the positional variables of the hook
> to make it easier to see what's going on.
>
> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
> ---

Makes sense.  Thanks.

Re: [PATCH 3/4] hook: add signature using "interpret-trailers"

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

>> It works well in all cases except when the user invokes
>> "git commit" without any arguments. In that case manually
>> add a new line after the first line to ensure it's consistent
>> with the output of "-s" option.
>> 
>
> Again, s/signature/sign-off/g, or similar (including subject line).

Thanks for being a careful reader.

>> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
>> ---
>>  templates/hooks--prepare-commit-msg.sample | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
>> index 708f0e92c..a15d6d634 100755
>> --- a/templates/hooks--prepare-commit-msg.sample
>> +++ b/templates/hooks--prepare-commit-msg.sample
>> @@ -32,4 +32,8 @@ SHA1=$3
>>  # esac
>>  
>>  # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
>> -# grep -qs "^$SOB" "$COMMIT_MSG_FILE" || echo "$SOB" >> "$COMMIT_MSG_FILE"
>> +# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
>> +# if test -z "$COMMIT_SOURCE"
>> +# then
>> +#   @PERL_PATH@ -i.bak -pe 'print "\n" if($first_line++ == 0)' "$COMMIT_MSG_FILE"
>> +# fi

I think we should do

	print "\n" if !$first_line++

for brevity (and also avoid "if(" that lacks SP) here.

Re: [PATCH 4/4] hook: add a simple first example

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

>  I made an attempt to make the second example work with amending 
>  with the aim of making it suitable for usage out of the box. It
>  seems that it's not easy to make it work as the status of a file
>  cannot be determined correctly when the index while amending
>  introduces changes to a file that has a change in the commit being
>  amended.
>
>  Is there any way in which the second example could be made to work with
>  amending without much effort? I'm asking this assuming something might
>  have happened, since the script was added, that could ease the task.

Sorry, but I do not understand what you are asking here.

Ahh, do you mean if we can avoid doing one half of the 1/4 (i.e. the
part that removes the commented out 'diff --name-status') and instead
make it a useful example (while still removing the thing that
comments out the "conflicts:")?

After going back and checking 1/4, I realize that I misread the patch.
you did keep the commented out 'diff --name-status' thing, so it still
has three---it just lost one half of the original "first" example.  So
please disregard my earlier "do we still have three, not two?"



>  Documentation/githooks.txt                 | 3 +++
>  templates/hooks--prepare-commit-msg.sample | 5 ++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index fdc01aa25..59f38efba 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -121,6 +121,9 @@ it is not suppressed by the `--no-verify` option.  A non-zero exit
>  means a failure of the hook and aborts the commit.  It should not
>  be used as replacement for pre-commit hook.
>  
> +The sample `prepare-commit-msg` hook that comes with Git removes the
> +help message found in the commented portion of the commit template.
> +
>  commit-msg
>  ~~~~~~~~~~
>  
> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
> index a15d6d634..a84c3e5a8 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -9,7 +9,8 @@
>  #
>  # To enable this hook, rename this file to "prepare-commit-msg".
>  
> -# This hook includes three examples.
> +# This hook includes three examples.  The first one removes the
> +# "# Please enter the commit message..." help message.
>  #
>  # The second includes the output of "git diff --name-status -r"
>  # into the message, just before the "git status" output.  It is
> @@ -23,6 +24,8 @@ COMMIT_MSG_FILE=$1
>  COMMIT_SOURCE=$2
>  SHA1=$3
>  
> +@PERL_PATH@ -i.bak -ne 'print unless(m/^. Please enter the commit message/..m/^#$/)' "$COMMIT_MSG_FILE"
> +
>  # case "$COMMIT_SOURCE,$SHA1" in
>  #  ,|template,)
>  #    @PERL_PATH@ -i.bak -pe '

Re: [PATCH 3/4] hook: add signature using "interpret-trailers"

[Kaartic Sivaraam]

On Mon, 2017-07-10 at 16:13 +0100, Ramsay Jones wrote:
> 
> Again, s/signature/sign-off/g, or similar (including subject line).
> 
Thanks! Forgot that in the course of splitting the patches and
modifying them.

Re: [PATCH 3/4] hook: add signature using "interpret-trailers"

[Kaartic Sivaraam]

Note: Re-attempting to send mail due to rejection
On Mon, 2017-07-10 at 16:13 +0100, Ramsay Jones wrote:
> 
> Again, s/signature/sign-off/g, or similar (including subject line).
> 
Thanks! Forgot that in the course of splitting the patches and
modifying them.

-- 
Kaartic

Re: [PATCH 4/4] hook: add a simple first example

[Kaartic Sivaraam]

On Mon, 2017-07-10 at 13:02 -0700, Junio C Hamano wrote:
> Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:
> 
> >  I made an attempt to make the second example work with amending 
> >  with the aim of making it suitable for usage out of the box. It
> >  seems that it's not easy to make it work as the status of a file
> >  cannot be determined correctly when the index while amending
> >  introduces changes to a file that has a change in the commit being
> >  amended.
> > 
> >  Is there any way in which the second example could be made to work
> > with
> >  amending without much effort? I'm asking this assuming something
> > might
> >  have happened, since the script was added, that could ease the
> > task.
> 
> Sorry, but I do not understand what you are asking here.
> 
I'm was trying to ask, "Is there any way to change the second example
(diff --name-status) to make it work with "commit --amend" so that it
could be uncommented by default ?" 

If there was a way then the patch 4/4 could be dropped as the name
status example would be enough make the script live (I think). 

> After going back and checking 1/4, I realize that I misread the
> patch.
> you did keep the commented out 'diff --name-status' thing, so it
> still
> has three---it just lost one half of the original "first"
> example.  So
> please disregard my earlier "do we still have three, not two?"
> 
Actually speaking, I did think of promoting the second to the first to
make the sub-patches independent of each other. I held myself as I
thought it would be overkill. Anyways, I'll just overkill it!

[PATCH 1/4] hook: cleanup script

[Kaartic Sivaraam]

Prepare the 'preare-commit-msg' sample script for
upcoming changes. Preparation includes removal of
an example that has outlived it's purpose. The example
is the one that comments the "Conflicts:" part of a
merge commit message. It isn't relevant anymore as
it's done by default since 261f315b ("merge & sequencer:
turn "Conflicts:" hint into a comment", 2014-08-28).

Further update the relevant comments from the sample script
and update the documentation.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 Documentation/githooks.txt                 |  3 ---
 templates/hooks--prepare-commit-msg.sample | 24 ++++++++++--------------
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 706091a56..fdc01aa25 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -121,9 +121,6 @@ it is not suppressed by the `--no-verify` option.  A non-zero exit
 means a failure of the hook and aborts the commit.  It should not
 be used as replacement for pre-commit hook.
 
-The sample `prepare-commit-msg` hook that comes with Git comments
-out the `Conflicts:` part of a merge's commit message.
-
 commit-msg
 ~~~~~~~~~~
 
diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 86b8f227e..279ddc1a7 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -9,28 +9,24 @@
 #
 # To enable this hook, rename this file to "prepare-commit-msg".
 
-# This hook includes three examples.  The first comments out the
-# "Conflicts:" part of a merge commit.
+# This hook includes two examples.
 #
-# The second includes the output of "git diff --name-status -r"
+# The first includes the output of "git diff --name-status -r"
 # into the message, just before the "git status" output.  It is
 # commented because it doesn't cope with --amend or with squashed
 # commits.
 #
-# The third example adds a Signed-off-by line to the message, that can
+# The second example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
-case "$2,$3" in
-  merge,)
-    @PERL_PATH@ -i.bak -ne 's/^/# /, s/^# #/#/ if /^Conflicts/ .. /#/; print' "$1" ;;
 
-# ,|template,)
-#   @PERL_PATH@ -i.bak -pe '
-#      print "\n" . `git diff --cached --name-status -r`
-#	 if /^#/ && $first++ == 0' "$1" ;;
-
-  *) ;;
-esac
+# case "$2,$3" in
+#  ,|template,)
+#    @PERL_PATH@ -i.bak -pe '
+#       print "\n" . `git diff --cached --name-status -r`
+# 	 if /^#/ && $first++ == 0' "$1" ;;
+#  *) ;;
+# esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
2.13.2.957.g457671ade

[PATCH 2/4] hook: name the positional variables

[Kaartic Sivaraam]

It's always nice to have named variables instead of
positional variables as they communicate their purpose
well.

Appropriately name the positional variables of the hook
to make it easier to see what's going on.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 templates/hooks--prepare-commit-msg.sample | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 279ddc1a7..eb5912163 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -19,14 +19,17 @@
 # The second example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
+COMMIT_MSG_FILE=$1
+COMMIT_SOURCE=$2
+SHA1=$3
 
-# case "$2,$3" in
+# case "$COMMIT_SOURCE,$SHA1" in
 #  ,|template,)
 #    @PERL_PATH@ -i.bak -pe '
 #       print "\n" . `git diff --cached --name-status -r`
-# 	 if /^#/ && $first++ == 0' "$1" ;;
+# 	 if /^#/ && $first++ == 0' "$COMMIT_MSG_FILE" ;;
 #  *) ;;
 # esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
-# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
+# grep -qs "^$SOB" "$COMMIT_MSG_FILE" || echo "$SOB" >> "$COMMIT_MSG_FILE"
-- 
2.13.2.957.g457671ade

[PATCH 3/4] hook: add sign-off using "interpret-trailers"

[Kaartic Sivaraam]

The sample hook to prepare the commit message before
a commit allows users to opt-in to add the sign-off
to the commit message. The sign-off is added at a place
that isn't consistent with the "-s" option of "git commit".
Further, it could go out of view in certain cases.

Add the sign-off in a way similar to "-s" option of
"git commit" using git's interpret-trailers command.

It works well in all cases except when the user invokes
"git commit" without any arguments. In that case manually
add a new line after the first line to ensure it's consistent
with the output of "-s" option.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 templates/hooks--prepare-commit-msg.sample | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index eb5912163..87d770592 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -32,4 +32,8 @@ SHA1=$3
 # esac
 
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
-# grep -qs "^$SOB" "$COMMIT_MSG_FILE" || echo "$SOB" >> "$COMMIT_MSG_FILE"
+# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
+# if test -z "$COMMIT_SOURCE"
+# then
+#   @PERL_PATH@ -i.bak -pe 'print "\n" if !$first_line++' "$COMMIT_MSG_FILE"
+# fi
-- 
2.13.2.957.g457671ade

[PATCH 4/4] hook: add a simple first example

[Kaartic Sivaraam]

Add a simple example that replaces an outdated example
that was removed. This ensures that there's at the least
a simple example that illustrates what could be done
using the hook just by enabling it.

Also, update the documentation.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 Documentation/githooks.txt                 | 3 +++
 templates/hooks--prepare-commit-msg.sample | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index fdc01aa25..59f38efba 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -121,6 +121,9 @@ it is not suppressed by the `--no-verify` option.  A non-zero exit
 means a failure of the hook and aborts the commit.  It should not
 be used as replacement for pre-commit hook.
 
+The sample `prepare-commit-msg` hook that comes with Git removes the
+help message found in the commented portion of the commit template.
+
 commit-msg
 ~~~~~~~~~~
 
diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 87d770592..f0228c1cb 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -9,7 +9,8 @@
 #
 # To enable this hook, rename this file to "prepare-commit-msg".
 
-# This hook includes two examples.
+# This hook includes three examples. The first one removes the
+# "# Please enter the commit message..." help message.
 #
 # The first includes the output of "git diff --name-status -r"
 # into the message, just before the "git status" output.  It is
@@ -23,6 +24,8 @@ COMMIT_MSG_FILE=$1
 COMMIT_SOURCE=$2
 SHA1=$3
 
+@PERL_PATH@ -i.bak -ne 'print unless(m/^. Please enter the commit message/..m/^#$/)' "$COMMIT_MSG_FILE"
+
 # case "$COMMIT_SOURCE,$SHA1" in
 #  ,|template,)
 #    @PERL_PATH@ -i.bak -pe '
-- 
2.13.2.957.g457671ade

[PATCH 4/4] hook: add a simple first example

[Kaartic Sivaraam]

Add a simple example that replaces an outdated example
that was removed. This ensures that there's at the least
a simple example that illustrates what could be done
using the hook just by enabling it.

Also, update the documentation.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 Documentation/githooks.txt                 | 3 +++
 templates/hooks--prepare-commit-msg.sample | 9 ++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index fdc01aa25..59f38efba 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -121,6 +121,9 @@ it is not suppressed by the `--no-verify` option.  A non-zero exit
 means a failure of the hook and aborts the commit.  It should not
 be used as replacement for pre-commit hook.
 
+The sample `prepare-commit-msg` hook that comes with Git removes the
+help message found in the commented portion of the commit template.
+
 commit-msg
 ~~~~~~~~~~
 
diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index 87d770592..dc707e46e 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -9,20 +9,23 @@
 #
 # To enable this hook, rename this file to "prepare-commit-msg".
 
-# This hook includes two examples.
+# This hook includes three examples. The first one removes the
+# "# Please enter the commit message..." help message.
 #
-# The first includes the output of "git diff --name-status -r"
+# The second includes the output of "git diff --name-status -r"
 # into the message, just before the "git status" output.  It is
 # commented because it doesn't cope with --amend or with squashed
 # commits.
 #
-# The second example adds a Signed-off-by line to the message, that can
+# The third example adds a Signed-off-by line to the message, that can
 # still be edited.  This is rarely a good idea.
 
 COMMIT_MSG_FILE=$1
 COMMIT_SOURCE=$2
 SHA1=$3
 
+@PERL_PATH@ -i.bak -ne 'print unless(m/^. Please enter the commit message/..m/^#$/)' "$COMMIT_MSG_FILE"
+
 # case "$COMMIT_SOURCE,$SHA1" in
 #  ,|template,)
 #    @PERL_PATH@ -i.bak -pe '
-- 
2.13.2.957.g457671ade

Re: [PATCH 4/4] hook: add a simple first example

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> I'm was trying to ask, "Is there any way to change the second example
> (diff --name-status) to make it work with "commit --amend" so that it
> could be uncommented by default ?"

But does it even be useful to enable it?  The commented out "git
status" output already lists the modified paths, so I do not think
anything is gained by adding 'diff --cached --name-status' there.

Re: [PATCH 4/4] hook: add a simple first example

[Kaartic Sivaraam]

On Tue, 2017-07-11 at 09:03 -0700, Junio C Hamano wrote:
> But does it even be useful to enable it?  The commented out "git
> status" output already lists the modified paths, so I do not think
> anything is gained by adding 'diff --cached --name-status' there.
The script *doesn't* add the output of "diff --cached --name-status"
near the status output. It adds it above the first comment and of
course in an uncommented form.

So, should we remove it (diff --cached --name-status) altogether?

Re: [PATCH 4/4] hook: add a simple first example

[Kaartic Sivaraam]

On Tue, 2017-07-11 at 09:03 -0700, Junio C Hamano wrote:
> But does it even be useful to enable it?
Just for the note, I thought it was considered useful (at least to
someone) due to it's presence in the sample script.

[PATCH] hook: use correct logical variable

[Kaartic Sivaraam]

Sign-off added should be that of the "committer" not that of the
"commit's author".

Use the correct logical variable that identifies the committer.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 This fixes a small issue when trying to do the following with the script enabled,

    $ git commit --amend -s

 If the commit being amended was signed off by the commit's author then the above command
 would *append* the sign-off of the committer followed by that of the commit's author.
 That' because the script is invoked only after the sign-off is added by the '-s' option AND
 the default of 'trailer.ifexists' for interpret-trailers currently defaults to the 'addIfDifferentNeighbor'
 thus interpret-trailer fails to identify the existing sign-off of the commit's author and adds it.

 Anyways, it doesn't make sense for a script to add the sign-off of the commit's author. So,
 fixing it seemed correct to me.

 templates/hooks--prepare-commit-msg.sample | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index a84c3e5a8..12dd8fd88 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -34,7 +34,7 @@ SHA1=$3
 #  *) ;;
 # esac
 
-# SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
+# SOB=$(git var GIT_COMMITTER_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
 # if test -z "$COMMIT_SOURCE"
 # then
-- 
2.14.1.534.g641031ecb

Re: [PATCH] hook: use correct logical variable

[Stefan Beller]

On Mon, Aug 14, 2017 at 1:46 AM, Kaartic Sivaraam
<kaarticsivaraam91196@gmail.com> wrote:
> Sign-off added should be that of the "committer" not that of the
> "commit's author".
>
> Use the correct logical variable that identifies the committer.
>
> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
> ---
>  This fixes a small issue when trying to do the following with the script enabled,
>
>     $ git commit --amend -s
>
>  If the commit being amended was signed off by the commit's author then the above command
>  would *append* the sign-off of the committer followed by that of the commit's author.
>  That' because the script is invoked only after the sign-off is added by the '-s' option AND
>  the default of 'trailer.ifexists' for interpret-trailers currently defaults to the 'addIfDifferentNeighbor'
>  thus interpret-trailer fails to identify the existing sign-off of the commit's author and adds it.

The background knowledge provided up to here seems like
a valuable information that we'd want to preserve in the commit
history, i.e. make it part of the commit message?

Code looks good.

Thanks,
Stefan

>
>  Anyways, it doesn't make sense for a script to add the sign-off of the commit's author. So,
>  fixing it seemed correct to me.
>
>  templates/hooks--prepare-commit-msg.sample | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
> index a84c3e5a8..12dd8fd88 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -34,7 +34,7 @@ SHA1=$3
>  #  *) ;;
>  # esac
>
> -# SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
> +# SOB=$(git var GIT_COMMITTER_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
>  # git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
>  # if test -z "$COMMIT_SOURCE"
>  # then
> --
> 2.14.1.534.g641031ecb
>

Re: [PATCH] hook: use correct logical variable

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> Sign-off added should be that of the "committer" not that of the
> "commit's author".
>
> Use the correct logical variable that identifies the committer.
>
> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
> ---
>  This fixes a small issue when trying to do the following with the script enabled,
>
>     $ git commit --amend -s
>
>  If the commit being amended was signed off by the commit's author
>  then the above command would *append* the sign-off of the
>  committer followed by that of the commit's author.
>  That' because the script is invoked only after the sign-off is
>  added by the '-s' option AND the default of 'trailer.ifexists'
>  for interpret-trailers currently defaults to the
>  'addIfDifferentNeighbor' thus interpret-trailer fails to identify
>  the existing sign-off of the commit's author and adds it.
>
>  Anyways, it doesn't make sense for a script to add the sign-off
>  of the commit's author. So, fixing it seemed correct to me.

I tend to agree with "Anyways" above ;-) simply because I found the
long paragraph more confusing than enlightening, leaving me wonder
"why is the user using different settings for author and committer
name" at the end, which _is_ an irrelevant point in the issue being
addressed.

The real issue is that this commented-out sample hook uses "author"
ident for the sign-off, while all the rest of Git (i.e. all callers
of append_signoff() in sequencer.c) use committer ident.

If anything to be changed to this patch, I would say its "should be"
in the log message can be clarified with "why", perhaps like:

    Sign-off added should be that of the "committer", not that of
    the "commit's author"; that is how the rest of Git adds sign-off
    using sequencer.c::append_signoff().

Thanks.

>  templates/hooks--prepare-commit-msg.sample | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
> index a84c3e5a8..12dd8fd88 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -34,7 +34,7 @@ SHA1=$3
>  #  *) ;;
>  # esac
>  
> -# SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
> +# SOB=$(git var GIT_COMMITTER_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
>  # git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
>  # if test -z "$COMMIT_SOURCE"
>  # then

Re: [PATCH] hook: use correct logical variable

[Kaartic Sivaraam]

On Monday 14 August 2017 11:24 PM, Stefan Beller wrote:

> On Mon, Aug 14, 2017 at 1:46 AM, Kaartic Sivaraam
> <kaarticsivaraam91196@gmail.com> wrote:
>> Sign-off added should be that of the "committer" not that of the
>> "commit's author".
>>
>> Use the correct logical variable that identifies the committer.
>>
>> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
>> ---
>>   This fixes a small issue when trying to do the following with the script enabled,
>>
>>      $ git commit --amend -s
>>
>>   If the commit being amended was signed off by the commit's author then the above command
>>   would *append* the sign-off of the committer followed by that of the commit's author.
>>   That' because the script is invoked only after the sign-off is added by the '-s' option AND
>>   the default of 'trailer.ifexists' for interpret-trailers currently defaults to the 'addIfDifferentNeighbor'
>>   thus interpret-trailer fails to identify the existing sign-off of the commit's author and adds it.
> The background knowledge provided up to here seems like
> a valuable information that we'd want to preserve in the commit
> history, i.e. make it part of the commit message?
I didn't do that previously expecting a few people would get confused by 
this (it did turn out
to be true). I could have made it more clearer but didn't attempt as I 
thought it wasn't worth
the effort. Yeah, it sometimes takes time to *simplify* things.

I guess Junio's suggestion found below seems concise enough although it 
doesn't
capture the reason I did the change.

         Sign-off added should be that of the "committer", not that of
         the "commit's author"; that is how the rest of Git adds sign-off
         using sequencer.c::append_signoff().


---
Kaartic

Re: [PATCH] hook: use correct logical variable

[Kaartic Sivaraam]

On Monday 14 August 2017 11:49 PM, Junio C Hamano wrote:
> I tend to agree with "Anyways" above ;-) simply because I found the
> long paragraph more confusing than enlightening, leaving me wonder
> "why is the user using different settings for author and committer
> name" at the end, which _is_ an irrelevant point in the issue being
> addressed.
This is typically the reason I didn't add the big paragraph to the log 
message.

---
Kaartic

Re: [PATCH] hook: use correct logical variable

[Junio C Hamano]

Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:

> I guess Junio's suggestion found below seems concise enough
> although it doesn't capture the reason I did the change.

I did shoot for conciseness, but what is a lot more important is to
record what is at the core of the issue.  "I found it by doing A"
can hint to careful readers why doing A leads to an undesirable
behaviour, but when there are other ways to trigger problems that
come from the same cause, "I found it by doing A" is less useful
unless we also record "Doing A reveals the underlying problem X"
that can be shared by other ways B, C, ... to trigger it.  The
careful readers need to guess what the X is.

And once you identify the underlying problem X and record _that_ in
the log message, I and A in "I found it by doing A" becomes much
less interesting and the readers do not have to guess.

Your "A" is 'git commit --amend -s' with the disabled part of hook
enabled.  But I think 'git commit' without "--amend" and "-s" would
also show an issue that come from the same root cause.  The hook
will add SoB that is based on the author, not the committer.  That
resulting commit would be different from 'git commit -s' without the
hook enabled, which would add SoB based on the commiter name (that
would be a "B", that causes a related but different problem that
comes from the same underlying issue "X" which is "we should
consistently use the committer info like other parts of the
system").

In any case, thanks for a fix-up.  Let's move this forward quickly,
as it is an update to a topic that is already in 'master'.

Thanks.

Re: [PATCH] hook: use correct logical variable

[Kaartic Sivaraam]

On Tue, 2017-08-15 at 10:28 -0700, Junio C Hamano wrote:
> I did shoot for conciseness, but what is a lot more important is to
> record what is at the core of the issue.  "I found it by doing A"
> can hint to careful readers why doing A leads to an undesirable
> behaviour, but when there are other ways to trigger problems that
> come from the same cause, "I found it by doing A" is less useful
> unless we also record "Doing A reveals the underlying problem X"
> that can be shared by other ways B, C, ... to trigger it.  The
> careful readers need to guess what the X is.
> 
> And once you identify the underlying problem X and record _that_ in
> the log message, I and A in "I found it by doing A" becomes much
> less interesting and the readers do not have to guess.
> 
> Your "A" is 'git commit --amend -s' with the disabled part of hook
> enabled.  But I think 'git commit' without "--amend" and "-s" would
> also show an issue that come from the same root cause.  The hook
> will add SoB that is based on the author, not the committer.  That
> resulting commit would be different from 'git commit -s' without the
> hook enabled, which would add SoB based on the commiter name (that
> would be a "B", that causes a related but different problem that
> comes from the same underlying issue "X" which is "we should
> consistently use the committer info like other parts of the
> system").
> 
> In any case, thanks for a fix-up.  Let's move this forward quickly,
> as it is an update to a topic that is already in 'master'.
> 
> Thanks.

Seeing the reply, I changed my opinion that "it isn't worth the effort
to write a better commit message for the change". I've given a try but
it got a little off-hand. Let me know if there's ways in which it could
be simplified and/or improved.

-- 
Kaartic

[PATCH v2/RFC] hook: use correct logical variable

[Kaartic Sivaraam]

In general, a 'Sign-off' added should be that of the *committer* and not
that of the *commit's author*. As the 3rd part of the 'prepare-commit-msg'
hook appended the sign-off of the *commit's author* it worked weirdly in
some cases. For example 'git commit --amend -s' when coupled with that part
of the script woked weirdly as illustrated by the following scenario in which
the last commit's log message has the following trailer,

    ...

    Signed-off-by: Random Developer <developer@example.com>

and the commit's author is "Random Developer <developer@example.com>". Assume
that the commit is trying to be amended by another developer who's identity is
"Another Developer <another-dev@example.com>". When he tries to do

    $ git commit --amend -s

with the 3rd part of the hook enabled then the trailer he would see in his editor
would be,

    ...

    Signed-off-by: Random Developer <developer@example.com>
    Signed-off-by: Another Developer <another-dev@example.com>
    Signed-off-by: Random Developer <developer@example.com>

This is because,

  * the hook is invoked only after the sign-off is appended by the '-s' option

  * the script tries to add the sign-off of the *commit's author* using interpret-trailers
    and 'interpret-trailers' in it's default configuration tries to adds the trailer
    when the *neighbouring* trailer isn't the same as the one trying to be added.

This is just an example and this kind of issue could repeat if similar conditions are
satisified for other cases.

Moreover the rest of Git adds the sign-off of the *committer* using sequencer.c::append_signoff().
So, use the correct logical variable that identifies the committer to append the sign-off
in the sample hook script.

Bottom line: Being consistent prevents all sorts of weird issues.

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
 Changes in v2:

    - updated the commit message
 
 Suggestions regarding ways to improve the message are most welcome.

 templates/hooks--prepare-commit-msg.sample | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index a84c3e5a8..12dd8fd88 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -34,7 +34,7 @@ SHA1=$3
 #  *) ;;
 # esac
 
-# SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
+# SOB=$(git var GIT_COMMITTER_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
 # if test -z "$COMMIT_SOURCE"
 # then
-- 
2.14.0.rc1.434.g6eded367a