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

[Ray Strode]

Hi,

On Tue, Jul 31, 2012 at 4:06 AM, Jan Engelhardt <jengelh at inai.de> wrote:
>
> On Monday 2012-07-30 21:59, Ray Strode wrote:
>>
>>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?
>
> I am completely impartial to that :)
So I gave this some more thought and decided this is actually a bad
idea.  The problem is then there's no good value to return for
ply_text_display_get_{foreground,backgroud}_color then when the colors
are reset.  We need to have a symbolic value anyway
to represent the unset case.  So since we need to have a value tucked
away in the enum anyway for the getters, we might as
well use it for the setters.

>>> +  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).
>
> It should then be removed, because you need different sequences
> almost everytime. Format strings should preferably also be at the
> site of where they are used, rather than stashed away into a lonely
> #define somewhere at the top.
Well the idea is to try to somewhat separate logic from mechanics.  if
a drive by reader sees

terminal_write (COLOR_SEQUENCE, FG_COLOR_BASE + color)

then it might be pretty obvious to the user that the code is trying to
change the fg color of the terminal.

If a driveby user sees

terminal_write ("\x1b\x5b%d;%d%um", hi, FG_COLOR_BASE + color,
BG_COLOR_BASE + display->bg_color);

then they'll have to decode the line noise and figure out what it means.

That's also one of the reasons I don't like seeing BG color in the FG
color function.  It means people not familar with
the code have to work harder to follow along.

Using bg color also has the problem that display->bg_color needs to be
initialized before you can set fg_color.

> Because the "0" in \e[0;...m, which is emitted when going from bold to
> non-bold, resets everything, including bgcolor?
Instead of using \e[0m maybe use \e[22m ?  That should just toggle
high brightness off, but leave bg color alone, I think.

--Ray