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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]