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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 26, 2015, 14:37 UTC
Message-ID: <xmqq6142w1xf.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAOLa=ZSu7XyZhWUL=25dnkiGm2mNDyFhXwS_JqudyfKPE5sisQ@mail.gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

Show 22 quoted lines

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