Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs

Karthik Nayak [off-list ref] writes:

quoted

Here is what I see...

ok 98 - verifying rfc1991 signature

expecting success:
        echo "rfc1991" >gpghome/gpg.conf &&
        echo "rfc1991-signed-tag RFC1991 signed tag" >expect &&
        git tag -l -n1 rfc1991-signed-tag >actual &&
        test_cmp expect actual &&
        git tag -l -n2 rfc1991-signed-tag >actual &&
        test_cmp expect actual &&
        git tag -l -n999 rfc1991-signed-tag >actual &&
        test_cmp expect actual

--- expect      2015-08-24 22:54:44.607272653 +0000
+++ actual      2015-08-24 22:54:44.611272643 +0000

@@ -1 +1 @@
-rfc1991-signed-tag RFC1991 signed tag
+rfc1991-signed-tagRFC1991 signed tag

not ok 99 - list tag with rfc1991 signature

Thats weird, I just ran all tests, and nothing failed.

You may be missing GPG or RFC1991 prerequisites and not running this
particular test, which could be why you missed it.

Your builtin/tag.c calls show_ref_array_item() with 

	"%(align:16,left)%(refname:short)%(end)"

as the format, and "rfc1991-signed-tag" is longer than 16.

And immediately after showing that, there is this hack at the end of
show_ref_array_item() function, which I _think_ should not be there
in ref-filter.c:show_ref_array_item() in the first place.

	if (lines > 0) {
		struct object_id oid;
		hashcpy(oid.hash, info->objectname);
		show_tag_lines(&oid, lines);
	}

This belongs to the caller who knows that it is dealing with a tag.

But that broken design aside, the reason why this breaks is because
you do not have a separating SP after the aligned short refs.

I didn't check how wide the original is supposed to be, but perhaps
changing builtin/tag.c this way

		if (filter->lines)
-			format = "%(align:16,left)%(refname:short)%(end)";
+			format = "%(align:15,left)%(refname:short)%(end) ";
		else
			format = "%(refname:short)";
	}

may be one way to fix it.  Check the original "tag -l" output for
tags whose names are 14, 15, 16 and 17 display-columns wide and try
to match it.

And then move the tag-specific code at the end of
show_ref_array_item() to here:

	verify_ref_format(format);
	filter_refs(&array, filter, FILTER_REFS_TAGS);
	ref_array_sort(sorting, &array);

-	for (i = 0; i < array.nr; i++)
+	for (i = 0; i < array.nr; i++) {
		show_ref_array_item(array.items[i], format, QUOTE_NONE, filter->lines);
+		if (lines) {
+			struct object_id oid;
+			hashcpy(oid.hash, info->objectname);
+			show_tag_lines(&oid, lines);
+		}
+	}

perhaps.