Re: [PATCH v6] Add an option with a color type.

[Tom Tromey <tom@tromey.com>]

>>>>> Andrei Pikas <gdb@mail.api.win> writes:

Hello.  Thanks for the patch and for getting a copyright assignment.
I'm sorry it's taken so long to get back to this.

It's a large & ambitious patch.  I read through it once and sent some
notes.  I assume I'll have more next time through as well.

> Colors can be specified as "none" for terminal's default color, as a name of
> one of the eight standard colors of ISO/IEC 6429 "black", "red", "green", etc.,
> as an RGB hexadecimal tripplet #RRGGBB for 24-bit TrueColor, or as an
> integer from 0 to 255.  Integers 0 to 7 are the synonyms for the standard
> colors.  Integers 8-15 are used for the so-called bright colors from the
> aixterm extended 16-color palette.  Integers 16-255 are the indexes into xterm
> extended 256-color palette (usually 6x6x6 cube plus gray ramp).  In
> general, 256-color palette is terminal dependent and sometimes can be
> changed with OSC 4 sequences, e.g. "\033]4;1;rgb:00/FF/00\033\\".

> It is the responsibility of the user to verify that the terminal supports
> the specified colors.

I often fantasize of getting rid of curses and just assuming an ANSI
terminal.

> +++ b/gdb/guile/scm-color.c
> @@ -0,0 +1,445 @@
...
> +#include "defs.h"

Since you wrote this, this area changed and defs.h doesn't have to be
manually included any more.

> diff --git a/gdb/python/py-color.c b/gdb/python/py-color.c
> new file mode 100644
> index 00000000000..fb816a06328
> --- /dev/null
> +++ b/gdb/python/py-color.c

> +extern PyTypeObject colorpy_object_type
> +    CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("parmpy_object");

You can just remove the CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF thing.  AFAIK
nobody has tried the checker in years and IIRC it doesn't work on C++
anyway.

> +  if (color_obj == NULL)
> +    return NULL;

Prefer nullptr in new code.

> +  if (! PyUnicode_CompareWithASCIIString (attr_name, "is_none"))
> +    return color.is_none () ? Py_True : Py_False;

In the Python C API, the True and False singletons still need to be
reference counted.

A simple way to do this is:

    return PyBool_FromLong (color.is_none ());

This affects a few spots.

> +  if (color.is_direct ()
> +      && ! PyUnicode_CompareWithASCIIString (attr_name, "components"))

TIL.

> +      PyObject *comp = PyTuple_New (3);
> +      if (!comp)

We started preferring 'comp == nullptr' for this kind of thing.

> +      int colors = tgetnum ((char *)"Co");

I wonder if that cast is really needed any longer.

Tom