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.
Regarding the "denser" mode, I very much like the simplicity of the idea.
I was wondering if a solution that aims for the maximum density that does
not shuffle the original order of items would end up taking the output
from "fmt" and distributing the words on each line evenly to the width,
which would be totally unusable. Your "punt at an item that does not fit
and restart from there" is simple and seems to work well.
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.
Thanks.
--
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