>
> 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.
Thanks, worked on it
> +{
> + 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).
Thanks for this, I have seen it too
On Tue, Dec 27, 2022 at 1:11 AM Jeff King <peff@xxxxxxxx> wrote:
>
> 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
