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

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> On Wed, Aug 26, 2015 at 12:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Karthik Nayak <karthik.188@gmail.com> writes:
>>>
>>> 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.
>>
>> That should be the fix, since it's a space problem.
>> ....
>> The problem with doing this is, the lines need to be displayed
>> immediately after  the refname,
>> followed by a newline, what you're suggesting breaks that flow.
>
> That is only because show_ref_array_item() does too much.  The
> function is given a placeholder string and a set of data to fill the
> placeholder with for an item, and the reason why the caller
> repeatedly calls it, once per item it has, is because it wants to
> produce a one-line-per-item output.  An alternative valid way to
> structure the API is to have it format into a string and leave the
> printing to the caller.  You can give a new format_ref_array_item()
> that does not print but fills a strbuf to this caller, make
> show_ref_array_item() a thin wrapper that calls it and prints it
> with the final LF for other callers.
>
> Another alternate approach, if you want to give "tag -l" annotation
> available to for-each-ref, would be to do this:
>
>        if (filter->lines)
>                format = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
>                                 "%%(contents:lines=%s)", filter->lines);
>        else
>                format = "%(refname:short)";
>
> and teach a new %(contents:lines=1) atom.  That way, you can lose
> the ugly special case call to show_tag_lines() that can only be at
> the end of the output.  I somehow think this is a better approach.
>