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