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

From: Junio C Hamano <gitster@pobox.com>
Date: May 20, 2014, 17:53 UTC
Message-ID: <xmqq61l01gmw.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140520082740.GB27590@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 25 quoted lines

> 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.

Show 23 quoted lines

>  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)