Re: [PATCH 3/3] templates: detect messages that contain a separator line

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Messages that contain "---" separator lines will be truncated by
> "git am". This often surprises users so add a check to the sample
> "commit-msg" hook to reject such messages. As it's conceivable that
> someone is using "---" as their comment string we delete any commented
> lines before checking for a separator. The trailing ".*" when matching
> commented lines ensures that if the comment string ends with a "$"
> it is not treated as an anchor.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  templates/hooks/commit-msg.sample | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

I have no qualms about the topic up to the previous step, but I know
one of the things that I sometimes do will be broken with the change
in this step, namely, when I know what I want to write below the
three-dash lines, I would commit with "---" and additional notes
below it, so that I do not forget during "format-patch".

When the commit is turned into a patch email, possibly with some
other material like "--notes=<ref>" that adds notes there, the
resulting message will have two three-dashes lines, but because "am"
cuts at the first one, and "apply" knows that the garbage lines at
front, including three-dash lines, do not matter until it sees "^diff",
this works out perfectly well.

Admittedly, I myself do not send out so many patches as I used to,
but I suspect that there are others who have discovered this trick
independently, and they would be unhappy to be interrupted by
commit-msg hook like this.

A saving grace is that when the user is stopped with this,
pre-commit hook that inspects the contents to be committed
have already run successfully, so rerunning with "--no-verify"
is not with too much risk.  But still, I am not sure if this is a
good thing to do overall.

> diff --git a/templates/hooks/commit-msg.sample b/templates/hooks/commit-msg.sample
> index 099cc58c303..c7a9db88cb9 100755
> --- a/templates/hooks/commit-msg.sample
> +++ b/templates/hooks/commit-msg.sample
> @@ -39,9 +39,16 @@ comment_re="$(
>  		}'
>  )"
>  line="$(sed -n -e "/^${comment_re} -\{8,\} >8 -\{8,\}\$/q
> +		   /^${comment_re}.*/d
> +		   /^---\$/{p;q;}
>  		   /^diff -/{p;q;}
>  		   /^Index: /{p;q;}" "$1")"
> -if test -n "$line"
> +if test "$line" = "---"
> +then
> +	echo >&2 "Message contains a '---' separator line that will confuse"
> +	echo >&2 "'git am'. To fix this indent the '---' line."
> +	ret=1
> +elif test -n "$line"
>  then
>  	echo >&2 "Message contains a diff that will confuse 'git am'."
>  	echo >&2 "To fix this indent the diff."