Re: [RFC PATCH 1/2] crypto sign: add crypto-sign alias flag

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

Fabian Stelzer <fs@gigacodes.de> writes:

> +--crypto-sign[=<keyid>]::
> +--no-crypto-sign::
>  --gpg-sign[=<keyid>]::
>  --no-gpg-sign::
> -	GPG-sign commits. The `keyid` argument is optional and
> -	defaults to the committer identity; if specified, it must be
> -	stuck to the option without a space. `--no-gpg-sign` is useful to
> -	countermand both `commit.gpgSign` configuration variable, and
> -	earlier `--gpg-sign`.
> +	Cryptographically sign commits. The `keyid` argument is optional and
> +	its default depends on the configured `cryptoSign.format`; if specified,
> +	it must be stuck to the option without a space. `--no-crypto-sign` is
> +	useful to countermand both `commit.gpgSign` configuration variable, and
> +	earlier `--crypto-sign`.
> +	`--(no-)gpg-sign` is a compatibility alias and has no effect on which
> +	cryptographic format will be used. This is determined by the
> +	configuration variable cryptoSign.format (see linkgit:git-config[1]).

I'd make the last three lines into a separate paragraph and nudge
users toward the new spelling if I were doing this change, e.g.

	...
	earlier `--crypto-sign`.
+
The option was originally called `--[no-]gpg-sign` and is still
supported as a synonym, but it is encouraged to migrate to use
the `--crypto-sign` option.

Not the problem with this patch, but can the format be inferred from
<keyid>?

If so, `--crypt-sign=<keyid of format X>` can choose the format X
and specify what exact key to use at the same time without the
cryptosign.format configuration variable.  But if not, the interface
leaves us in an awkward place by letting different keys easily
specified from the command line while making it impossible to
switch between GPG and SSH from the command line.

If --gpg-sign is not a mere synonym, but also implies GPG is
preferred when cryptoSign.format is not specified, that is a
different story, of course.  That makes it unnecessary to deprecate
`--gpg-sign` and in addition we need to add `--ssh-sign` option that
works similarly, which may not scale well but I do not expect we'd
add many more next to GPG and SSH, hopefully?  I dunno.

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 883c16256c..2c789ff6f9 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1639,8 +1639,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
>  		OPT_CLEANUP(&cleanup_arg),
>  		OPT_BOOL(0, "status", &include_status, N_("include status in commit message template")),
> -		{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
> +		{ OPTION_STRING, 'S', "crypto-sign", &sign_commit, N_("key-id"),
> +		  N_("cryptographically sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> +		{ OPTION_STRING, 0, "gpg-sign", &sign_commit, N_("key-id"),
>  		  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },

Leaving this explained as "GPG sign commit" contradicts "it is a
mere alias that does not even imply GPG is preferred when no
preference is given by the cryptoSign.format variable".

> +

Unwanted blank line before the "this is the last line of these
options" marker?

>  		/* end commit message options */
>  
>  		OPT_GROUP(N_("Commit contents options")),