Re: [PATCH] commit-tree: add missing --gpg-sign flag

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

Brandon Richardson <brandon1024.br@gmail.com> writes:

> -		if (skip_prefix(arg, "-S", &sign_commit))
> +		if(!strcmp(arg, "--gpg-sign")) {

Style.  "if (!strcmp(arg, "--gpg-sign")) {"

> +		    skip_prefix(arg, "--gpg-sign", &sign_commit);
> +		    continue;

Technically, skipping the prefix S of string S will make us point at
an empty substring at the end.  So from that point of view,
skip_prefix(arg, "--gpg-sign", &sign_commit) is not incorrect
per-se, but it is highly misleading.  We have already determined
that the user gave us "--gpg-sign" option without anything after it,
so we want to summon the "use the default key" behaviour by giving
an empty string to sign_commit.

An explicit assignment

	sign_commit = "";

would be a lot more readable and make the intent a lot more clear.

> +		}
> +
> +		if (skip_prefix(arg, "-S", &sign_commit) ||
> +			skip_prefix(arg, "--gpg-sign=", &sign_commit))

This side is OK.  "-S" gives us an empty string, but "-Skeyid" gives
us "keyid" in sign_commit.

>  			continue;
>  
>  		if (!strcmp(arg, "--no-gpg-sign")) {
> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
> index 86d3f93fa..efc136eaf 100755
> --- a/t/t7510-signed-commit.sh
> +++ b/t/t7510-signed-commit.sh
> @@ -51,7 +51,9 @@ test_expect_success GPG 'create signed commits' '
>  	# commit.gpgsign is still on but this must not be signed
>  	git tag ninth-unsigned $(echo 9 | git commit-tree HEAD^{tree}) &&
>  	# explicit -S of course must sign.
> -	git tag tenth-signed $(echo 9 | git commit-tree -S HEAD^{tree})
> +	git tag tenth-signed $(echo 10 | git commit-tree -S HEAD^{tree})
> +	# --gpg-sign must sign.
> +	git tag eleventh-signed $(echo 11 | git commit-tree --gpg-sign HEAD^{tree})
>  '
>  
>  test_expect_success GPG 'verify and show signatures' '