Re: [PATCH 4/6] tag: add format specifier to gpg_verify_tag
- From
Santiago Torres <santiago@nyu.edu>
- Date
- Sep 23, 2016, 14:36 UTC
- Message-ID
- <20160923143625.ynnzgkikj625f6ij@LykOS.localdomain>
- In-Reply-To
- <xmqqponv3b4x.fsf@gitster.mtv.corp.google.com>
On Thu, Sep 22, 2016 at 01:58:06PM -0700, Junio C Hamano wrote:
Show 58 quoted lines
> santiago@nyu.edu writes:
>
> > Calling functions for gpg_verify_tag() may desire to print relevant
> > information about the header for further verification. Add an optional
> > format argument to print any desired information after GPG verification.
>
> > diff --git a/builtin/tag.c b/builtin/tag.c
> > index dbf271f..94ed8a2 100644
> > --- a/builtin/tag.c
> > +++ b/builtin/tag.c
> > @@ -106,7 +106,7 @@ static int delete_tag(const char *name, const char *ref,
> > static int verify_tag(const char *name, const char *ref,
> > const unsigned char *sha1)
> > {
> > - return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
> > + return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
> > }
> >
> > static int do_sign(struct strbuf *buffer)
> > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> > index 99f8148..7a1121b 100644
> > --- a/builtin/verify-tag.c
> > +++ b/builtin/verify-tag.c
> > @@ -51,8 +51,10 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
> > const char *name = argv[i++];
> > if (get_sha1(name, sha1))
> > had_error = !!error("tag '%s' not found.", name);
> > - else if (gpg_verify_tag(sha1, name, flags))
> > - had_error = 1;
> > + else {
> > + if (verify_and_format_tag(sha1, name, NULL, flags))
> > + had_error = 1;
> > + }
>
> Revert the unnecessary reformatting here.
>
> > @@ -56,6 +57,15 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
> > ret = run_gpg_verify(buf, size, flags);
> >
> > free(buf);
> > +
> > + if (fmt_pretty) {
> > + struct ref_array_item *ref_item;
> > + ref_item = new_ref_item(name_to_report, sha1, 0);
> > + ref_item->kind = FILTER_REFS_TAGS;
> > + show_ref_item(ref_item, fmt_pretty, 0);
> > + free_ref_item(ref_item);
> > + }
>
> I haven't seen 5/6 and 6/6, but if this is the only user of the 3/6,
> it would be much better to have a single function to format a ref
> exported from ref-filter.[ch] so that this one can say
>
> if (fmt_pretty)
> format_ref(name_to_report, sha1, FILTER_REFS_TAGS);
>
> or something like that, instead of doing three that will always be
> used together in quick succession in the above pattern.Oh, this sounds like a better alternative. This would be instead of 0003 right?
Thanks, -Santiago.