Had I gotten any of those constants wrong, unit tests would fail.
It's probably on purpose that those values were chosen to be the same
but for us it's just a happy coincidence.
I also have just one value for COLOR_BACKGROUND_OFFSET, because for
both the ANSI colors and the AIXTERM, the difference is 10. Just
another happy coincidence. I could have split those into two
constants like with RGB vs 256.
None of those values are likely to ever change. I think that the most
important feature of the constants is that they are descriptive.
Thanks for your help.
Eyal
On Tue, Feb 11, 2020 at 2:46 PM Jeff King <peff@xxxxxxxx> wrote:
>
> On Tue, Feb 11, 2020 at 11:36:23AM -0800, Junio C Hamano wrote:
>
> > +enum {
> > + COLOR_BACKGROUND_OFFSET = 10,
> > + COLOR_FOREGROUND_ANSI = 30,
> > + COLOR_FOREGROUND_RGB = 38,
> > + COLOR_FOREGROUND_256 = 38,
> > +};
>
> I had to double-check to make sure the duplication in the last two
> wasn't a bug. It's correct, because "38" here is really "set the
> foreground color", and they're followed by more magic for "256" or
> "RGB".
>
> So really this could be a single COLOR_FOREGROUND_EXTENDED or similar
> that gets used in both places. But I don't know that it really matters
> that much.
>
> Other than that nitpick, the patches all looked OK to me. Thanks for
> tying up this loose end.
>
> -Peff
