On Thu, Jan 16, 2020 at 10:01:44AM -0800, Junio C Hamano wrote:
> Not that I agree with the (untold) reasoning why we chose to use
> 30-37 instead of 0-7, though. If this were up to me, I would have
> rather defined COLOR_BACKGROUND_ANSI = 40, kept .value to 0-7 and
> passed COLOR_{FORE,BACK}GROUPD_ANSI to callers of color_output().
>
> Since I haven't seen 2/3 and 3/3, perhaps there is a good reason why
> this step was done this way instead, though, I guess.
Yeah, it becomes more clear in patch 2, where the value can be either
"31" or "91", for the bright or non-bright variant, and adding "30" is
wrong. (But certainly I agree this needs to be explained here).
Another way to write it would be to store 0-7 in the value as before,
and then add a separate "bright" flag to "struct color". And then the
output becomes:
COLOR_FOREGROUND_OFFSET + c->value + (c->bright ? COLOR_BRIGHT_OFFSET : 0)
or similar. One minor confusion there is that COLOR_256 and COLOR_RGB
would ignore the "bright" field.
-Peff
