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

From: Junio C Hamano <[email-masked]>
Date: Tue, 25 Aug 2015 12:16:08 -0700

Karthik Nayak <[email-masked]> writes:

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