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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:01:16

Jeff King [off-list ref] 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 <redacted>
---
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.
quoted hunk
 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)

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

From: Jeff King <hidden>
Date: 2016-06-15 23:01:16

On Tue, May 20, 2014 at 10:53:11AM -0700, Junio C Hamano wrote:
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.
If it were just "--signature", I'd agree. After all, nobody is even
complaining. But this is also in preparation for --signature-file.
Should the user create a file without a trailing newline?

We can special-case --signature-file to strip the final newline from the
read file, but it seems friendlier to handle it at the printing stage
(and then we handle the unlikely but possible --signature as above for
free). I don't think it would adversely impact any real-world case,
because somebody would have to:

  1. already be including an extra trailing newline

  2. really _want_ three newlines at the end
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.
Yeah. I think it is probably extraneous and would not hurt to remove.
But it may not be worth worrying about (it's really the _two_ lines
caused by the unconditional newline above that bugs me).

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help