Re: [PATCH v4] format-patch: --rfc honors what --subject-prefix sets

[Junio C Hamano <gitster@pobox.com>]

Drew DeVault <sir@cmpwn.com> writes:

> Minor correction to the documentation is also included, and a second
> test just for good measure which demonstrates that the order of
> arguments no longer important.

Perfect.

>  Documentation/git-format-patch.txt | 18 +++++++++++------
>  builtin/log.c                      | 31 +++++++++++++++---------------
>  t/t4014-format-patch.sh            | 22 ++++++++++++++++++++-
>  3 files changed, 48 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 373b46fc0d..b96e142a8d 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -217,9 +217,15 @@ populated with placeholder text.
>  
>  --subject-prefix=<subject prefix>::
>  	Instead of the standard '[PATCH]' prefix in the subject
> -	line, instead use '[<subject prefix>]'. This
> -	allows for useful naming of a patch series, and can be
> -	combined with the `--numbered` option.
> +	line, instead use '[<subject prefix>]'. This can be used
> +	to name a patch series, and can be combined with the
> +	`--numbered` option.
> ++
> +The configuration variable `format.subjectPrefix` may also be used
> +to to configure a subject prefix to apply to a given repository for
> +all patches. This is often useful on mailing lists which receive
> +patches for several repositories and can be used to disambiguate
> +the patches (with a value of e.g. "PATCH my-project").

Nice.

I'll locally fix "to to" -> "to" while queuing; no need to reroll
only to fix this.

> diff --git a/builtin/log.c b/builtin/log.c
> index db3a88bfe9..29c86dc798 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1468,19 +1468,16 @@ static int subject_prefix = 0;
>  static int subject_prefix_callback(const struct option *opt, const char *arg,
>  			    int unset)
>  {
> +	struct strbuf *sprefix;
> +
>  	BUG_ON_OPT_NEG(unset);
> +	sprefix = (struct strbuf *)opt->value;
>  	subject_prefix = 1;
> -	((struct rev_info *)opt->value)->subject_prefix = arg;
> +	strbuf_reset(sprefix);
> +	strbuf_addstr(sprefix, arg);
>  	return 0;
>  }

OK.

> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 3cf2b7a7fb..9fa1f3bc7a 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1373,7 +1373,27 @@ test_expect_success '--rfc' '
>  	Subject: [RFC PATCH 1/1] header with . in it
>  	EOF
>  	git format-patch -n -1 --stdout --rfc >patch &&
> -	grep ^Subject: patch >actual &&
> +	grep "^Subject:" patch >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '--rfc does not overwrite prefix' '
> +	cat >expect <<-\EOF &&
> +	Subject: [RFC PATCH foobar 1/1] header with . in it
> +	EOF
> +	git -c format.subjectPrefix="PATCH foobar" \
> +		format-patch -n -1 --stdout --rfc >patch &&
> +	grep "^Subject:" patch >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '--rfc is argument order independent' '
> +	cat >expect <<-\EOF &&
> +	Subject: [RFC PATCH foobar 1/1] header with . in it
> +	EOF
> +	git format-patch -n -1 --stdout --rfc \
> +		--subject-prefix="PATCH foobar" >patch &&
> +	grep "^Subject:" patch >actual &&
>  	test_cmp expect actual
>  '

Nice.

Will queue.  Let's wait to see if others find something fishy for a
day or two and then merge it down to 'next'.

Thanks.