On Wed, Oct 7, 2015 at 1:08 AM, Matthieu Moy
<matthieu....@grenoble-inp.fr> wrote:
> Karthik Nayak <karthik....@gmail.com> writes:
>
>> On Mon, Oct 5, 2015 at 2:19 PM, Matthieu Moy
>> <matthieu....@grenoble-inp.fr> wrote:
>>> Karthik Nayak <karthik....@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:
>
Yes! this :)
> -- 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< --
>
Looks Good.
> 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 */
>
Will do this, thanks a bunch
--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
