On Mon, Nov 12, 2018 at 07:20:28PM +0900, Junio C Hamano wrote:
> nbelakovski@xxxxxxxxx writes:
>
> > diff --git a/color.h b/color.h
> > index 98894d6a17..857653df73 100644
> > --- a/color.h
> > +++ b/color.h
> > @@ -42,6 +42,24 @@ struct strbuf;
> > #define GIT_COLOR_FAINT_BLUE "\033[2;34m"
> > #define GIT_COLOR_FAINT_MAGENTA "\033[2;35m"
> > #define GIT_COLOR_FAINT_CYAN "\033[2;36m"
> > +#define GIT_COLOR_LIGHT_RED "\033[91m"
> > +#define GIT_COLOR_LIGHT_GREEN "\033[92m"
> > +#define GIT_COLOR_LIGHT_YELLOW "\033[93m"
> > +#define GIT_COLOR_LIGHT_BLUE "\033[94m"
> > +#define GIT_COLOR_LIGHT_MAGENTA "\033[95m"
> > +#define GIT_COLOR_LIGHT_CYAN "\033[96m"
> > +#define GIT_COLOR_BOLD_LIGHT_RED "\033[1;91m"
> > +#define GIT_COLOR_BOLD_LIGHT_GREEN "\033[1;92m"
> > +#define GIT_COLOR_BOLD_LIGHT_YELLOW "\033[1;93m"
> > +#define GIT_COLOR_BOLD_LIGHT_BLUE "\033[1;94m"
> > +#define GIT_COLOR_BOLD_LIGHT_MAGENTA "\033[1;95m"
> > +#define GIT_COLOR_BOLD_LIGHT_CYAN "\033[1;96m"
> > +#define GIT_COLOR_FAINT_LIGHT_RED "\033[2;91m"
> > +#define GIT_COLOR_FAINT_LIGHT_GREEN "\033[2;92m"
> > +#define GIT_COLOR_FAINT_LIGHT_YELLOW "\033[2;93m"
> > +#define GIT_COLOR_FAINT_LIGHT_BLUE "\033[2;94m"
> > +#define GIT_COLOR_FAINT_LIGHT_MAGENTA "\033[2;95m"
> > +#define GIT_COLOR_FAINT_LIGHT_CYAN "\033[2;96m"
>
> Hopefully you made sure that there is no other topic in-flight that
> touch this area before doing this change? Otherwise you'd be
> creating pointless merge conflict by futzing with spaces.
This hunk confused me for a minute, too. It's not changing spaces, but
just adding a bunch of color variants. It would be nice if we could just
do this with a run-time parse_color("bold red") or whatever, but we use
these as static initializers.
We don't strictly need anything more than FAINT_LIGHT_GREEN here. I
don't have a strong opinion on adding just what we need versus
being more complete.
> Ditto for an earlier hunk of this patch.
Yeah, I think this does apply to the earlier hunk that defines
branch_colors[].
-Peff
