Re: [PATCH v5] format-patch --signature-file <file>

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

Jeff King <peff@peff.net> writes:

> But one could easily specify a longer, multi-line signature,
> like:
>
>   git format-patch --signature='
>   this is my long signature
>
>   it has multiple lines
>   ' ...
>
> We should notice that it already has its own trailing
> newline, and suppress one of ours.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> In the example above, there's also a newline before the signature
> starts. Should we suppress the first one, too?
>
> Also, I'm not clear on the purpose of the extra trailing line in the
> first place. Emails now end with (">" added to show blanks):
>
>   > -- 
>   > 2.0.0-rc3...
>   >
>
> Is there are a reason for that final blank line?

I actually think these "supress extra LFs" trying to be overly smart
and inviting unnecessary surprises.  Unlike log messages people type
(in which we do squash runs of blank lines and other prettifying),
mail-signature string is not something people keep typing, and it
would be better to keep it simple and consistent, i.e. we can
declare that the users who use non-default mail-signature can and
should learn to:

    --signature='this is the first line of my long sig

    with a blank line and then it ends here'

and be done with it, I think.

The trailing blank after the mail-signature is a different issue.  I
think it is safe to remove it and I also think the result may look
better, but at the same time, it is very close to the "if we were
writing format-patch today, then we would..." category, I would say.

>  builtin/log.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 39e8836..5acc048 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -844,8 +844,13 @@ static void gen_message_id(struct rev_info *info, char *base)
>  
>  static void print_signature(void)
>  {
> -	if (signature && *signature)
> -		printf("-- \n%s\n\n", signature);
> +	if (!signature || !*signature)
> +		return;
> +
> +	printf("-- \n%s", signature);
> +	if (signature[strlen(signature)-1] != '\n')
> +		putchar('\n');
> +	putchar('\n');
>  }
>  
>  static void add_branch_description(struct strbuf *buf, const char *branch_name)