Re: [PATCH] format-patch: add arbitrary email headers

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 25, 2009, 19:11 UTC
Message-ID: <7v3ad11kqh.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1237996712-61859-1-git-send-email-michael@ndrix.org>

Michael Hendricks <michael@ndrix.org> writes:

Show 5 quoted lines

> format-patch supports the format.headers configuration for adding
> arbitrary email headers to the patches it outputs.  This patch adds
> support for a --header argument which makes the same feature available
> from the command line.  This is useful when the content of custom
> email headers must change from branch to branch.

How should this interact with the configuration variable?

Typically we allow command line options to override the matching config variable, so that people can say "here are the settings I ordinarily use" in the config file, and say "but I do not want the usual values to take effect for this particular invocation; please use these _instead_" with command line options.

Note that the above question is "how should this interact"; not "how does this interact". I can see you chose to make this cumulative in your patch and the documentaiton.

I am asking if that is what the users want, overriding is preferred, or perhaps another option to clear extra headers (say, "--no-extra-headers") is necessary to allow both.

Show 10 quoted lines

> diff --git a/builtin-log.c b/builtin-log.c
> index c7a5772..35701a7 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -918,6 +918,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  			cover_letter = 1;
>  		else if (!strcmp(argv[i], "--no-binary"))
>  			no_binary_diff = 1;
> +		else if (!prefixcmp(argv[i], "--header="))
> +			add_header( argv[i] + 9 );

No extra SP immediately after ( and before ), please.