Re: [PATCH] commit-tree: add missing --gpg-sign flag
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 18, 2019, 21:02 UTC
- Message-ID
- <xmqqy37hiss5.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190118010918.43705-1-brandon1024.br@gmail.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.
Show 18 quoted lines
> 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' '