Re: [PATCH 6/9] ref-filter: introduce format_ref_array_item()

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> On Mon, Oct 5, 2015 at 2:19 PM, Matthieu Moy
>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>>> Karthik Nayak <karthik.188@gmail.com> writes:
>>>> which does not play well with the implementation of --column as done
>>>> in tag.c. Where, If I'm not wrong the --column option captures all
>>>> output, formats it and then prints it to stdout. Hence when using
>>>> colors, we're told that the printing isn't done to a tty which
>>>> supports colors, hence we lose out on colors.
>>>
>>> What I don't understand is how --column is different from --no-column
>>> wrt colors.
>>>
>>> In any case, this should be explained better in comments.
>>
>> Well, If we use column the way we do in tag.c then it'll replace the tty
>> and color will not print color because it will assume the output tty doesn't
>> support colors.
>>
>> I hope that's what you're asking
>
> Looking a bit more closely at the code, I think I understand what's
> going on. branch.c used print_columns which does all the clever things
> wrt columns display. tag.c used run_column_filter which pipes the output
> to "git column" (hence, indeed, color detection does not work since
> we're writting to a pipe).
>
> The branch.c way seems good to me (why fork another process when we can
> format the list ourselves ?). Probably tag.c could (should?) be modified
> to use it too. It should just be justified, like, replacing this commit
> message with:
>

> -- 8< --
> To allow column display, we will need to first render the output in a
> string list to allow print_columns() to compute the proper size of each
> column before starting the actual output. Introduce the function
> format_ref_array_item() that does the formatting of a ref_array_item to
> an strbuf.
>
> show_ref_array_item() is kept as a convenience wrapper around it which
> obtains the strbuf and prints it the standard output.
> -- 8< --
>

> and adding a comment next to if (column_active(colopts)) { in patch
> 8/9:
>
>         /* format to a string_list to let print_columns() do its job */
>