Re: [PATCH] color: allow multiple attributes

Jeff King [off-list ref] writes:

quoted

+		if (COLOR_MAXLEN <=
+		    /* Number of bytes to denote colors and attributes */
+		    num_attrs
+		    + (fg < 0 ? 0 : (fg < 8) ? 2 : 8) /* "3x" or "38;5;xxx" */
+		    + (bg < 0 ? 0 : (bg < 8) ? 2 : 8) /* "4x" or "48;5;xxx" */
+		    /* Number of semicolons between the above elements */
+		    + (num_attrs + (0 <= fg) + (0 <= bg) - 1)
+		    /* ESC '[', terminating 'm' and NUL */
+		    + 4)
+			goto bad;

We don't need this, because the length of what can be specified is
bounded, and we simply need to set COLOR_MAXLEN high enough to handle
the longest case?

Yes, I think we are now bounded and don't need this; I just thought it
would have an educational value to show how to comment a complex
expression in a readable way ;-)

quoted

+test_expect_success 'fg bg attr...' '
+	color "blue bold dim ul blink reverse" "[1;2;4;5;7;34m"
+'

Hmm. Just a thought on the bit-setting approach, but does the order of
attributes ever matter? We are going to lose the ordering information
the user specifies, obviously.

True, I don't know if it matters.  I don't know if "blue bold bold" would
result in bolder blue than "blue bold" on some terminal emulators, either.

I'd suggest that we ignore the issue for now, and when somebody complains
with an actual non-working case, we would assess the damage that comes
from this reordering to decide what to do next.  Parhaps a "non-working
case" could be "'blink ul' blinks letter without blinking underline, but
'ul blink' makes both letter and underline blink".  At that point we can
say "Ok, you found a case the order changes the results.  But does that
difference matter in practice?" and move forward, either by fixing it, or
declaring it doesn't matter in practice.

We were already losing the order by emitting attr then fg then bg even
though attr can come before any colors (an undocumented side effect of a
sloppy parsing logic, but some of the existing tests insist on kepping it
working), by the way.