Am Mon, 16 Jul 2018 13:14:34 -0700
schrieb Junio C Hamano <gitster@xxxxxxxxx>:
> Henning Schild <henning.schild@xxxxxxxxxxx> writes:
>
> > Add "gpg.format" where the user can specify which type of signature
> > to use for commits. At the moment only "openpgp" is supported and
> > the value is not even used. This commit prepares for a new types of
> > signatures.
> >
> > Signed-off-by: Henning Schild <henning.schild@xxxxxxxxxxx>
> > ---
> > Documentation/config.txt | 4 ++++
> > gpg-interface.c | 7 +++++++
> > 2 files changed, 11 insertions(+)
> >
> > diff --git a/Documentation/config.txt b/Documentation/config.txt
> > index 1cc18a828..ac373e3f4 100644
> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt
> > @@ -1828,6 +1828,10 @@ gpg.program::
> > signed, and the program is expected to send the result to
> > its standard output.
> >
> > +gpg.format::
> > + Specifies which key format to use when signing with
> > `--gpg-sign`.
> > + Default is "openpgp", that is also the only supported
> > value. +
> > gui.commitMsgWidth::
> > Defines how wide the commit message window is in the
> > linkgit:git-gui[1]. "75" is the default.
> > diff --git a/gpg-interface.c b/gpg-interface.c
> > index 09ddfbc26..960c40086 100644
> > --- a/gpg-interface.c
> > +++ b/gpg-interface.c
> > @@ -7,6 +7,7 @@
> > #include "tempfile.h"
> >
> > static char *configured_signing_key;
> > +static const char *gpg_format = "openpgp";
> > static const char *gpg_program = "gpg";
> >
> > #define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"
> > @@ -138,6 +139,12 @@ int git_gpg_config(const char *var, const char
> > *value, void *cb) return 0;
> > }
> >
> > + if (!strcmp(var, "gpg.format")) {
> > + if (value && strcasecmp(value, "openpgp"))
> > + return error("malformed value for %s: %s",
> > var, value);
> > + return git_config_string(&gpg_format, var,
> > value);
>
> I may be mistaken (sorry, I read so many topics X-<) but I think the
> consensus was to accept only "openpgp" so that we can ensure that
>
> [gpg "openpgp"] program = foo
>
> etc. can be parsed more naturally without having to manually special
> case the subsection name to be case insensitive. In other words,
> strcasecmp() should just be strcmp(). Otherwise, people would get a
> wrong expectation that
>
> [gpg] format = OpenPGP
> [gpg "openpgp"] program = foo
>
> should refer to the same and single OpenPGP, but that would violate
> the usual configuration syntax rules.
Ok, also having read the other mails i think we are still at
gpg.format and gpg.<format>.program, so i switched the two patches from
strcasecmp back to strcmp.
> The structure of checking the error looks quite suboptimal even when
> we initially support the single "openpgp" at this step. I would
> have expected you to be writing the above like so:
>
> if (!value)
> return config_error_nonbool(var);
> if (strcmp(value, "openpgp"))
> return error("unsupported value for %s: %s", var,
> value); return git_config_string(...);
>
> That would make it more clear that the variable is "nonbool", which
> does not change across additional support for new formats in later
> steps.
Right, at one point (not mailed) i had that but expected it would not
pass the review, since git_config_string contains that check as well.
Changed.
Henning
> > + }
> > +
> > if (!strcmp(var, "gpg.program")) {
> > if (!value)
> > return config_error_nonbool(var);
