On Tue, Jan 17, 2017 at 11:57:25AM -0500, Santiago Torres wrote:

quoted

Having read through the rest of the series, it looks like you'd
sometimes have to do:

IIRC, we did this to make the diff "simpler". It also feeds odd that the
call chain is the following:

    verify_and_format_tag()
    gpg_verify_tag()
    run_gpg_verification()

I'm afraid that adding yet another wrapper would further convolute the
call chain. If you think this is not an issue, I could easily do it. Do
you have any suggested name for the wrapper?

Actually, looking at the callsites, I think they are fine to just call
pretty_print_ref() themselves, and I don't think it actually matters if
it happens before or after the verification.

So I think you could just throw out patch 3 entirely and squash these
hunks into patches 4 and 5:

diff --git a/builtin/tag.c b/builtin/tag.c
index 9da11e0c2..fab9fa8f9 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c

@@ -111,10 +111,12 @@ static int verify_tag(const char *name, const char *ref,
 	char *fmt_pretty = cb_data;
 	flags = GPG_VERIFY_VERBOSE;
 
-	if (fmt_pretty)
+	if (fmt_pretty) {
 		flags = GPG_VERIFY_QUIET;
+		pretty_print_ref(name, sha1, fmt_pretty);
+	}
 
-	return verify_and_format_tag(sha1, ref, fmt_pretty, flags);
+	return gpg_verify_tag(sha1, ref, flags);
 }
 
 static int do_sign(struct strbuf *buffer)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 212449f47..114df1c52 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c

@@ -58,9 +58,15 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 	while (i < argc) {
 		unsigned char sha1[20];
 		const char *name = argv[i++];
-		if (get_sha1(name, sha1))
+
+		if (get_sha1(name, sha1)) {
 			had_error = !!error("tag '%s' not found.", name);
-		else if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
+			continue;
+		}
+
+		if (fmt_pretty)
+			pretty_print_ref(name, sha1, fmt_pretty);
+		if (gpg_verify_tag(sha1, name, flags))
 			had_error = 1;
 	}
 	return had_error;

You could make the diff in the second one simpler by skipping the
"continue" and just doing the whole thing in an "else" block. But IMHO
the continue-on-error makes the logic more clear.

-Peff