Henning Schild <henning.schild@xxxxxxxxxxx> writes:
> -enum gpgformats { PGP_FMT };
> +enum gpgformats { PGP_FMT, X509_FMT };
> struct gpg_format_data gpg_formats[] = {
> { .format = "PGP", .program = "gpg",
> .extra_args_verify = { "--keyid-format=long", },
> .sigs = { PGP_SIGNATURE, PGP_MESSAGE, },
> },
> + { .format = "X509", .program = "gpgsm",
> + .extra_args_verify = { NULL },
> + .sigs = {X509_SIGNATURE, NULL, }
Missing SP between "{X" is a bit irritating.
Also the trailing comma (the issue is shared with the PGP side) when
the initializer is smashed on a single line feels pretty much
pointless. If it were multi-line, then such a trailing comma would
help future developers to add a new entry, i.e.
.sigs = {
PGP_SIGNATURE,
PGP_MESSAGE,
+ PGP_SOMETHING_NEW,
}
without touching the last existing entry. But on a single line?
-.sigs = { PGP_SIGNATURE, PGP_MESSAGE }
+.sigs = { PGP_SIGNATURE, PGP_MESSAGE, PGP_SOMETHING_NEW }
is probably prettier without such a trailing comma.
> @@ -190,6 +195,9 @@ int git_gpg_config(const char *var, const char *value, void *cb)
> if (!strcmp(var, "gpg.program"))
> return git_config_string(&gpg_formats[PGP_FMT].program, var,
> value);
> + if (!strcmp(var, "gpg.programX509"))
> + return git_config_string(&gpg_formats[X509_FMT].program, var,
> + value);
This is a git_config() callback, isn't it? A two-level variable
name is given to a callback after downcasing, so nothing will match
"gpg.programX509", I suspect. I see Brian already commented on the
name and the better organization being
- gpg.format defines 'openpgp' or whatever other values;
- gpg.<format>.program defines the actual program
where <format> is the value gpg.format would take
(e.g. "gpg.openpgp.program = gnupg"). And I agree with these
suggestions.
