Re: [PATCH v7 01/9] Add column layout skeleton and git-column

2012/3/14 Junio C Hamano [off-list ref]:

Nguyễn Thái Ngọc Duy  [off-list ref] writes:

quoted

+#define COL_ENABLE(c) ((c) & COL_ENABLE_MASK)

That is a misleading name for a boolean macro.  It looked as if this

quoted

+     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