Re: [PATCH 1/2] format-patch: Add a signature option (--signature)

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 15, 2010, 17:05 UTC
Message-ID: <7viq5k6xbj.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1276578039-25023-1-git-send-email-bebarino@gmail.com>

Stephen Boyd <bebarino@gmail.com> writes:

> This does modify the original behavior of format-patch a bit. First
> off the version string is now placed in the cover letter by default.

That part is fine; I think it was an oversight of the cover letter addition.

> Secondly, once the configuration variable format.signature is added
> to the .config file there is no way to revert back to the default
> git version signature. Instead, specifying the --no-signature option
> will remove the signature from the patches entirely.

Hmph.  That is somewhat sad, but it probably is Ok.

Show 8 quoted lines

> @@ -180,6 +180,12 @@ will want to ensure that threading is disabled for `git send-email`.
>  	containing the shortlog and the overall diffstat.  You can
>  	fill in a description in the file before sending it out.
>  
> +--signature=<signature>::
> +	Add a signature to each patch and, if --cover-letter is specified,
> +	the cover letter. Per RFC 3676 the signature is separated from the
> +	body by '-- '.

Wouldn't "Add a signature to each message produced" be easier to understand? Also perhaps s/body by '-- '/& a line with '-- ' on it/?

Don't we want either an extra header to this entry ("--no-signature::") and advertise it as a way to disable signature generation?

Show 13 quoted lines

> diff --git a/builtin/log.c b/builtin/log.c
> index 976e16f..b78027e 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -609,6 +610,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
>  		do_signoff = git_config_bool(var, value);
>  		return 0;
>  	}
> +	if (!strcmp(var, "format.signature"))
> +		return git_config_string(&signature, var, value);
>  
>  	return git_log_config(var, value, cb);
>  }

This, together with ...

Show 10 quoted lines

> @@ -703,6 +706,12 @@ static void gen_message_id(struct rev_info *info, char *base)
>  	info->message_id = strbuf_detach(&buf, NULL);
>  }
>  
> +static void print_signature(void)
> +{
> +	if (signature)
> +		printf("-- \n%s\n\n", signature);
> +}
> +

... this, and ...

Show 8 quoted lines

> @@ -1035,6 +1045,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  		{ OPTION_CALLBACK, 0, "thread", &thread, "style",
>  			    "enable message threading, styles: shallow, deep",
>  			    PARSE_OPT_OPTARG, thread_callback },
> +		OPT_STRING(0, "signature", &signature, "signature",
> +			    "add a signature"),
>  		OPT_END()
>  	};

... this would mean that

 (1) these behave differently:

	format-patch --no-signature -1 HEAD
        format-patch --signature= -1 HEAD

 (2) this behaves like the second one in the above:

	[format]
        	signature = ""

 (3) there is no way to say "no signature, ever" in the configuration.

Perhaps we would want to do this instead?

        static void print_signature(void)
        {
                if (signature && *signature)
                        printf("-- \n%s\n\n", signature);
        }