Re: [PATCH] Clean up use of ANSI color sequences
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 12, 2009, 23:03 UTC
- Message-ID
- <7vy6wbi7gm.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1234471059-53625-1-git-send-email-arjen@yaph.org>
Arjen Laarhoven <arjen@yaph.org> writes:
Show 17 quoted lines
> diff --git a/color.h b/color.h > index 5019df8..c4d2e53 100644 > --- a/color.h > +++ b/color.h > @@ -4,6 +4,16 @@ > /* "\033[1;38;5;2xx;48;5;2xxm\0" is 23 bytes */ > #define COLOR_MAXLEN 24 > > +#define COLOR_NORMAL "" > +#define COLOR_RESET "\033[m" > +#define COLOR_BOLD "\033[1m" > +#define COLOR_RED "\033[31m" > +#define COLOR_GREEN "\033[32m" > +#define COLOR_YELLOW "\033[33m" > +#define COLOR_BLUE "\033[34m" > +#define COLOR_CYAN "\033[36m" > +#define COLOR_BG_RED "\033[41m"
Sounds like a very sane thing to do in principle, but the choice of constant names are problematic.
(1) There are COLOR_BRANCH_$category constants, that look very similar
(they probably should be renamed to BRANCH_COLOR_$category). (2) These are ANSI constants so it might be better to call them
ANSI_COLOR_$physical_attributes, or GIT_COLOR_$physical_attributes. I actually prefer the latter because then later we can potentially
redefine these macros with something like:#define GIT_COLOR_RED ti_setf(COLOR_RED) #define GIT_COLOR_BG_RED ti_setb(COLOR_RED)
and write a set of small wrappers to terminfo to support non ANSI
terminals without changing the rest of the code. It is nicer to use
GIT_COLOR_RED instead of COLOR_RED, because the latter are defined in
ncurses.h like this: /* colors */
#define COLOR_BLACK 0
#define COLOR_RED 1
#define COLOR_GREEN 2
#define COLOR_YELLOW 3
#define COLOR_BLUE 4
#define COLOR_MAGENTA 5
#define COLOR_CYAN 6
#define COLOR_WHITE 7