Re: [PATCH] Clean up use of ANSI color sequences

[Junio C Hamano <gitster@pobox.com>]

Arjen Laarhoven <arjen@yaph.org> writes:

> 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