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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Karthik Nayak <karthik.188@xxxxxxxxx> writes:

> On Mon, Oct 5, 2015 at 2:19 PM, Matthieu Moy
> <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
>> Karthik Nayak <karthik.188@xxxxxxxxx> 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 */

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]