[PATCH 1/4] ply-text-display: support bright colors

[Ray Strode]

Hi,

> @@ -44,8 +44,16 @@ typedef enum
>    PLY_TERMINAL_COLOR_BLUE,
>    PLY_TERMINAL_COLOR_MAGENTA,
>    PLY_TERMINAL_COLOR_CYAN,
> +  PLY_TERMINAL_COLOR_GRAY,
> +  PLY_TERMINAL_COLOR_DARKGRAY,
> +  PLY_TERMINAL_COLOR_BRIGHTRED,
> +  PLY_TERMINAL_COLOR_BRIGHTGREEN,
> +  PLY_TERMINAL_COLOR_YELLOW,
> +  PLY_TERMINAL_COLOR_BRIGHTBLUE,
> +  PLY_TERMINAL_COLOR_BRIGHTMAGENTA,
> +  PLY_TERMINAL_COLOR_BRIGHTCYAN,
>    PLY_TERMINAL_COLOR_WHITE,
> -  PLY_TERMINAL_COLOR_DEFAULT = PLY_TERMINAL_COLOR_WHITE + 2
> +  PLY_TERMINAL_COLOR_DEFAULT,
>  } ply_terminal_color_t;
So this highlights a problem in the current API: we have this special color
"DEFAULT" that's supposed to mean the terminal default color (and indeed does
mean that in an ANSI escape sequence sort of way), but that same value means
"bright red" when setting up terminal palettes.

I think rather than trying to keep this square peg, we could just drop
PLY_TERMINAL_COLOR_DEFAULT, and introduce new apis:

ply_text_display_reset_background_color (display);
ply_text_display_reset_foreground_color (display);

What do you think?

minor review nits below:

> +  if (color == PLY_TERMINAL_COLOR_DEFAULT)
> +        color = 9;
We unfortunately use GNU coding style and, also, we don't use tabs.

> @@ -190,9 +197,19 @@ void
>  ply_text_display_set_foreground_color (ply_text_display_t       *display,
>                                         ply_terminal_color_t  color)
>  {
> +  bool hi = false;
> +
> +  if (color >= PLY_TERMINAL_COLOR_DARKGRAY &&
> +      color <= PLY_TERMINAL_COLOR_WHITE) {
> +        color -= PLY_TERMINAL_COLOR_DARKGRAY;
> +        hi = true;
> +  } else if (color == PLY_TERMINAL_COLOR_DEFAULT) {
> +        color = 9;
> +  }
>    ply_terminal_write (display->terminal,
> -                      COLOR_SEQUENCE_FORMAT,
> -                      FOREGROUND_COLOR_BASE + color);
> +                      "\x1b\x5b""%d;%u;%um", hi,
> +                      FOREGROUND_COLOR_BASE + color,
> +                      BACKGROUND_COLOR_BASE + display->background_color);
Should update COLOR_SEQUENCE_FORMAT to use the new format rather than
stop using it, or remove the COLOR_SEQUENCE_FORMAT definition (though
I'd prefer if we kept it).

Out of curiosity, why are you setting the background color in the
set_foreground_color function?

--Ray