Re: [PATCH 4/6] tag: add format specifier to gpg_verify_tag

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.

Revert the unnecessary reformatting here.

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.

On Thu, Sep 22, 2016 at 01:58:06PM -0700, Junio C Hamano wrote:

santiago@nyu.edu writes:

quoted

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.

quoted

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.

quoted

@@ -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.