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

[Junio C Hamano <gitster@pobox.com>]

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.

Of course you can (and probably would want to) do both, giving a
bit lower level "emit to a strbuf" function to the callers _and_
losing hardcoded call to show_tag_lines().