On Sat, Feb 27, 2010 at 06:56:38PM -0800, Junio C Hamano wrote:
> >>> but wouldn't it be more user friendly for us
> >>> to support "red blink bold ul italic"?
> >>
> >> Yes, I think this should be done whether or not the patch in question
> >> is accepted.
>
> This time with a bit of test updates as well for real inclusion.
Looks OK to me, but...
> Also I realized that we can stuff them in an unsigned flag word as
> bitfields ("red bold" and "red bold bold bold" would give the same
> boldness anyway) to lift the artificial limit of number of attribute
> words.
I also had this thought, but shouldn't that mean:
> + int i;
> + int num_attrs = count_bits(attr);
> +
> + 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? Though I suppose it doesn't hurt to be paranoid.
> +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.
-Peff
--
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