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

[Jeff King <peff@peff.net>]

On Wed, May 21, 2014 at 11:26:18AM -0700, Junio C Hamano wrote:

> Yeah, I agree with the last sentence.  My mention of "cleansing"
> took into account your "do we want to omit the leading blank as
> well?" as well.  In any case, wouldn't reusing stripspace() make the
> fix-up shorter?

Not really. Doing stripspace() on the file we read would be a one-liner,
but it doesn't address the extra line. We _already_ have
stripspace-compatible output in the file, with a trailing newline. It's
the one we add for the cmdline arg or default (which lack a newline
generally) that causes the doubling.

So I think it would be something like:

diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..0312402 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) {
+		struct strbuf buf = STRBUF_INIT;
+		strbuf_addstr(&buf, signature);
+		stripspace(&buf, 0);
+		printf("-- \n%s\n", buf.buf);
+		strbuf_release(&buf);
+	}
 }
 
 static void add_branch_description(struct strbuf *buf, const char *branch_name)

I.e., make sure we have consistent stripspace output, and then stop
adding the extra newline in the printf (notice we still include the
"extra" blank at the end, though we can obviously easily drop it here).
Compare to the previous patch I sent, or to the shortest possible:

diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..16b8771 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -845,7 +845,8 @@ 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);
+		printf("-- \n%s%s\n", signature,
+		       ends_with(signature, "\n") ? "" : "\n");
 }
 
 static void add_branch_description(struct strbuf *buf, const char *branch_name)

But I'm not sure "length of code" is the right metric. They're all
pretty easy to do, if we can decide on which.

I think the ratio of usefulness to number of words in this sub-thread is
getting off. I'd be OK with any of:

  1. Leave it as-is. Files get two blank lines.

  2. Conditional newline

  3. Stripspace.

-Peff