On Tue, Dec 27, 2022 at 12:55:23AM +0000, nsengaw4c via GitGitGadget wrote:
> This only works for commits. Add "signature" atom with `grade`,
> `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`
> as arguments. This code and it's documentation are inspired by
> how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were
> implemented.
I don't have a real review for you, but rather two small requests since
I was working in this area recently.
> @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
> return 0;
> }
>
> +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
> + const char *arg, struct strbuf *err)
Can you squash in an annotation for the unused parameter, like this:
diff --git a/ref-filter.c b/ref-filter.c
index a4c3f89f64..3b3592acb2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -405,8 +405,9 @@ static int subject_atom_parser(struct ref_format *format UNUSED,
return 0;
}
-static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
- const char *arg, struct strbuf *err)
+static int signature_atom_parser(struct ref_format *format UNUSED,
+ struct used_atom *atom,
+ const char *arg, struct strbuf *err)
{
if (arg) {
if (!strcmp(arg, "signer"))
This will eventually be necessary once we turn on -Wunused-parameter.
I'm preparing a patch to convert all of the other parsers that need it,
and I don't want to create a dependency between the two patches (it's OK
for you to add the UNUSED now, it's just not enforced yet).
I can also circle back after your patch is merged and add it, but it's a
bit easier to do it up front.
> +{
> + if (arg) {
> + if (!strcmp(arg, "signer"))
> + atom->u.signature.option = S_SIGNER;
> + else if (!strcmp(arg, "grade"))
> + atom->u.signature.option = S_GRADE;
> + else if (!strcmp(arg, "key"))
> + atom->u.signature.option = S_KEY;
> + else if (!strcmp(arg, "fingerprint"))
> + atom->u.signature.option = S_FINGERPRINT;
> + else if (!strcmp(arg, "primarykeyfingerprint"))
> + atom->u.signature.option = S_PRI_KEY_FP;
> + else if (!strcmp(arg, "trustlevel"))
> + atom->u.signature.option = S_TRUST_LEVEL;
> + else
> + return strbuf_addf_ret(err, -1, _("unknown %%(signature) argument: %s"), arg);
> + }
The ref-filter code recently got a helper function to report this kind
of argument error consistently, via dda4fc1a84 (ref-filter: factor out
"unrecognized %(foo) arg" errors, 2022-12-14). If you rebase the patch
on the current master, you can just do:
return err_bad_arg(err, "signature", arg);
which will make the error message match the others (which in turn saves
work for translators).
-Peff
