I noticed that the gdb.Color.escape_sequence() method would produce an
escape sequence even when styling is disabled.
I think this is the wrong choice. Ideally, when styling is
disabled (e.g. with 'set style enabled off'), GDB should not be
producing styled output.
If a GDB extension is using gdb.Color to apply styling to the output,
then currently, the extension should be checking 'show style enabled'
any time Color.escape_sequence() is used. This means lots of code
duplication, and the possibility that some locations will be missed,
which means disabling styling no longer does what it says.
I propose that Color.escape_sequence() should return the empty string
if styling is disabled. A Python extension can then do:
python
c_none = gdb.Color('none')
c_red = gdb.Color('red')
print(c_red.escape_sequence(True)
+ "Text in red."
+ c_none.escape_sequence(True))
end
If styling is enable this will print some red text. And if styling is
disabled, then it will print text in the terminal's default color.
I have applied the same fix to the guile API.
I have extended the tests to cover this case.
Approved-By: Tom Tromey <tom@tromey.com>
GDB's Python documentation does make it clear that keywords arguments
are supported for functions that take 2 or more arguments. The
documentation makes no promise for keyword argument support on
functions that only take a single argument.
That said, I'm a fan of keyword arguments, I think they help document
the code, and make intentions clearer, even for single argument
functions.
As I'm changing gdb.Color anyway (see previous commit), I'd like to
add keyword argument support to gdb.Color.escape_sequence, even though
this is a single argument method. This should be harmless for anyone
who doesn't want to use keywords, but adds the option for those of us
that do.
I've also removed a redundant check that the 'self' argument was a
gdb.Color object; Python already ensures this is the case.
And I have folded the check that the single argument is a bool into
the gdb_PyArg_ParseTupleAndKeywords call, this means that the error
message will include the incorrect type name now, which should make
debugging issues easier.
Tests have been extended to cover both cases -- it appears the
incorrect argument type error was not previously tested, so it is
now.
Approved-By: Tom Tromey <tom@tromey.com>
GDB's Python API documentation is clear:
Functions and methods which have two or more optional arguments allow
them to be specified using keyword syntax.
The gdb.Color.__init__ method matches this description, but doesn't
support keyword arguments.
This commit fixes this by adding keyword argument support.
There's a new test to cover this functionality.
Approved-By: Tom Tromey <tom@tromey.com>
The gdbpy_is_color function uses PyObject_IsInstance, and converts the
return from PyObject_IsInstance to a bool.
Unfortunately, PyObject_IsInstance can return -1, 0, or 1, for error,
failure, or success respectively. When converting to a bool both -1
and 1 will convert to true.
Additionally, when PyObject_IsInstance returns -1 an error will be
set.
What this means is that, if gdbpy_is_color is called with a non
gdb.Color object, and the PyObject_IsInstance check raises an error,
then (a) GDB will continue as if the object is a gdb.Color object,
which is likely going to invoke undefined behaviour, see
gdbpy_get_color for example, and (b) when GDB eventually returns to
the Python interpreter, due to an error being set, we'll see:
Python Exception <class 'SystemError'>: PyEval_EvalFrameEx returned a result with an error set
Error occurred in Python: PyEval_EvalFrameEx returned a result with an error set
However, after the previous commit, gdb.Color can no longer be
sub-classed, this means that fixing the above problems is easy, we can
replace the PyObject_IsInstance check with a PyObject_TypeCheck, the
PyObject_TypeCheck function only returns 0 or 1, there's no -1 error
case.
It's also worth noting that PyObject_TypeCheck is the function that is
more commonly used within GDB's Python API implementation, include the
py-color.c use there were only 4 PyObject_IsInstance uses. Of the
remaining 3, 2 are fine, and one other (in py-disasm.c) is also
wrong. I'll address that in a separate patch.
There's also a new test included which exposes the above issue.
Approved-By: Tom Tromey <tom@tromey.com>
Remove the Py_TPFLAGS_BASETYPE flag from the gdb.Color type. This
effectively makes gdb.Color final; users can no longer create classes
that inherit from gdb.Color.
Right now I cannot think of any cases where inheritance would be
needed over composition for a simple type like gdb.Color. If I'm
wrong, then it's easy to add Py_TPFLAGS_BASETYPE back in later, this
would be an extension of the API. But it's much harder to remove the
flag later as that might break existing user code (note: there has
been no release of GDB yet that includes the gdb.Color type).
Introducing this restriction makes the next commit easier.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
A few minor GNU/GDB coding style issues in py-color.c:
- Space after '&' reference operator in one place.
- Some excessive indentation on a couple of lines.
- Spaces after '!' logical negation operator.
- Using a pointer as a bool in a couple of places.
There should be no functional changes after this commit.
I noticed that this commit:
commit 6447969d0a
Date: Sat Oct 5 22:27:44 2024 +0300
Add an option with a color type.
has an unnecessary `Py_INCREF (self);` in gdb.Color.__init__. This
means that the reference count on all gdb.Color objects (that pass
through __init__) will be +1 from where they should normally be, and
this will stop the gdb.Color objects from being deallocated.
Fix by removing the Py_INCREF call.
Add a test which exposes the memory leak.
Approved-By: Tom Tromey <tom@tromey.com>
This updates the copyright headers to include 2025. I did this by
running gdb/copyright.py and then manually modifying a few files as
noted by the script.
Approved-By: Eli Zaretskii <eliz@gnu.org>
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.
PATCH v5 changes: documentation fixed.
PATCH v6 changes: documentation fixed.
PATCH v7 changes: rebase onto master and fixes after review.
PATCH v8 changes: fixes after review.
