Using really empty start prefixes for git-format-patch numbered patches?

Using really empty start prefixes for git-format-patch numbered patches?

[Frédéric Delanoy]

Hi,

I'm trying to generate numbered patches using git-format-patch such
that I get e.g. [1/2] instead of [PATCH 1/2]

However, if I use an empty string as prefix, for instance in

git-format-patch --subject-prefix="" -2

an extraneous space is inserted, and I get [ 1/2] instead of desired
[1/2] in the Subject line

(I also tried using a single backspace char as prefix, but that didn't
change anything, as might have been expected)

Is there a way to get rid of this space? IMO the space should only be
added if the prefix is non-empty

Frédéric Delanoy

Re: Using really empty start prefixes for git-format-patch numbered patches?

[Frédéric Delanoy]

Is this a bug or a feature?

2011/5/28 Frédéric Delanoy <frederic.delanoy@gmail.com>:
> Hi,
>
> I'm trying to generate numbered patches using git-format-patch such
> that I get e.g. [1/2] instead of [PATCH 1/2]
>
> However, if I use an empty string as prefix, for instance in
>
> git-format-patch --subject-prefix="" -2
>
> an extraneous space is inserted, and I get [ 1/2] instead of desired
> [1/2] in the Subject line
>
> (I also tried using a single backspace char as prefix, but that didn't
> change anything, as might have been expected)
>
> Is there a way to get rid of this space? IMO the space should only be
> added if the prefix is non-empty
>
> Frédéric Delanoy
>

Re: Using really empty start prefixes for git-format-patch numbered patches?

[Jeff King]

On Sat, May 28, 2011 at 11:57:01PM +0200, Frédéric Delanoy wrote:

> I'm trying to generate numbered patches using git-format-patch such
> that I get e.g. [1/2] instead of [PATCH 1/2]
> 
> However, if I use an empty string as prefix, for instance in
> 
> git-format-patch --subject-prefix="" -2
> 
> an extraneous space is inserted, and I get [ 1/2] instead of desired
> [1/2] in the Subject line
> 
> (I also tried using a single backspace char as prefix, but that didn't
> change anything, as might have been expected)
> 
> Is there a way to get rid of this space? IMO the space should only be
> added if the prefix is non-empty

I don't think there is currently a way to do what you want short of
post-processing the output of format-patch. You can use "-k" to preserve
the subject, but then you don't get the "1/2" that you want.

So I think we should do this:

-- >8 --
Subject: [PATCH] format-patch: make zero-length subject prefixes prettier

If you give a zero-length subject prefix to format-patch
(e.g., "format-patch --subject-prefix="), we will print the
ugly:

  Subject: [ 1/2] your subject here

because we always insert a space between the prefix and
numbering. Requiring the user to provide the space in their
prefix would be more flexible, but would break existing
usage. This patch provides a DWIM and suppresses the space
for zero-length prefixes, under the assumption that nobody
actually wants "[ 1/2]".

Signed-off-by: Jeff King <peff@peff.net>
---
 log-tree.c              |    3 ++-
 t/t4014-format-patch.sh |   18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 2a1e3a9..296f417 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -294,8 +294,9 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 	if (opt->total > 0) {
 		static char buffer[64];
 		snprintf(buffer, sizeof(buffer),
-			 "Subject: [%s %0*d/%d] ",
+			 "Subject: [%s%s%0*d/%d] ",
 			 opt->subject_prefix,
+			 *opt->subject_prefix ? " " : "",
 			 digits_in_number(opt->total),
 			 opt->nr, opt->total);
 		subject = buffer;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 045cee3..92248d2 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -851,4 +851,22 @@ test_expect_success 'subject lines do not have 822 atom-quoting' '
 	test_cmp expect actual
 '
 
+cat >expect <<'EOF'
+Subject: [PREFIX 1/1] header with . in it
+EOF
+test_expect_success 'subject prefixes have space prepended' '
+	git format-patch -n -1 --stdout --subject-prefix=PREFIX >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [1/1] header with . in it
+EOF
+test_expect_success 'empty subject prefix does not have extra space' '
+	git format-patch -n -1 --stdout --subject-prefix= >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.5.3.12.g99e25

Re: Using really empty start prefixes for git-format-patch numbered patches?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I don't think there is currently a way to do what you want short of
> post-processing the output of format-patch. You can use "-k" to preserve
> the subject, but then you don't get the "1/2" that you want.

You would unconditionally get "[" and "]" that you may or may not want and
there is no way to change it to "(1/2)" or "1 of 2: "; unless we shoot for
a completely flexible solution, we need to draw a line somewhere.

It was not optimal in the hindsight to draw the line where the prefix is
customizable, while always assuming that people would want some prefix,
perhaps.

> So I think we should do this:

Yes, ...

> ... under the assumption that nobody
> actually wants "[ 1/2]".

which I think is a reasonable thing to do.

Will queue; thanks.

Re: Using really empty start prefixes for git-format-patch numbered patches?

[Jeff King]

On Mon, May 30, 2011 at 12:19:22PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I don't think there is currently a way to do what you want short of
> > post-processing the output of format-patch. You can use "-k" to preserve
> > the subject, but then you don't get the "1/2" that you want.
> 
> You would unconditionally get "[" and "]" that you may or may not want and
> there is no way to change it to "(1/2)" or "1 of 2: "; unless we shoot for
> a completely flexible solution, we need to draw a line somewhere.

Hmm, yeah, I guess we could go farther pretty simply with:

  git format-patch --subject='[PATCH %n/%m] %s'

but I admit I don't really care enough to work on it. :)

> Will queue; thanks.

Thanks.

-Peff