2012/3/14 Junio C Hamano <gitster@xxxxxxxxx>:
> Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
>
>> +#define COL_ENABLE(c) ((c) & COL_ENABLE_MASK)
>
> That is a misleading name for a boolean macro. It looked as if this
>
>> + assert(COL_ENABLE(colopts) != COL_AUTO);
>
> was asking the helper to *enable* the column machinery with the given set
> of option in colopts, and expecting the helper to answer how it enabled
> ("I took the 'automatic' decision path"). But that is not what is
> happening.
>
> Unfortunately, COL_ENABLED?(c) is not an option, but this seriously needs
> a better name to avoid reader confusion.
I'm running out of names. Suggestions are welcome.
> I haven't formed an opinion on your "grouping" mode yet. The hardcoded
> slash hierarchy delimiter somewhat bothers me, but I haven't thought it
> deeply enough to judge if it is worth making it more generic. My gut
> feeling is that '/' probably is OK.
The column user should be able to decide how to display the group
line. I rely a lot on coloring, and I was already thinking about
giving group line a highlight color, which must be configurable
because different column user uses different base colors.
--
Duy
--
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