From: Andrew Burgess <aburgess@redhat.com>
Date: Wed, 23 Apr 2025 09:22:32 +0000 (+0100)
Subject: gdb/python: keyword arguments for gdb.Color.escape_sequence
X-Git-Tag: binutils-2_45~817
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=90fa7d2fd5a26bd0eb933c5cd9297c0e0801dacb;p=thirdparty%2Fbinutils-gdb.git

gdb/python: keyword arguments for gdb.Color.escape_sequence

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>
---

diff --git a/gdb/python/py-color.c b/gdb/python/py-color.c
index 97801f859f0..e208506c9d3 100644
--- a/gdb/python/py-color.c
+++ b/gdb/python/py-color.c
@@ -136,21 +136,21 @@ get_attr (PyObject *obj, PyObject *attr_name)
 /* Implementation of Color.escape_sequence (self, is_fg) -> str.  */
 
 static PyObject *
-colorpy_escape_sequence (PyObject *self, PyObject *is_fg_obj)
+colorpy_escape_sequence (PyObject *self, PyObject *args, PyObject *kwargs)
 {
-  if (!gdbpy_is_color (self))
-    {
-      PyErr_SetString (PyExc_RuntimeError,
-		       _("Object is not gdb.Color."));
-      return nullptr;
-    }
+  static const char *keywords[] = { "is_foreground", nullptr };
+  PyObject *is_fg_obj;
 
-  if (!PyBool_Check (is_fg_obj))
-    {
-      PyErr_SetString (PyExc_RuntimeError,
-		       _("A boolean argument is required."));
-      return nullptr;
-    }
+  /* Parse method arguments.  */
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "O!", keywords,
+					&PyBool_Type, &is_fg_obj))
+    return nullptr;
+
+  /* Python ensures the type of SELF.  */
+  gdb_assert (gdbpy_is_color (self));
+
+  /* The argument parsing ensures we have a bool.  */
+  gdb_assert (PyBool_Check (is_fg_obj));
 
   bool is_fg = is_fg_obj == Py_True;
   std::string s = gdbpy_get_color (self).to_ansi (is_fg);
@@ -288,7 +288,8 @@ gdbpy_initialize_color (void)
 
 static PyMethodDef color_methods[] =
 {
-  { "escape_sequence", colorpy_escape_sequence, METH_O,
+  { "escape_sequence", (PyCFunction) colorpy_escape_sequence,
+    METH_VARARGS | METH_KEYWORDS,
     "escape_sequence (is_foreground) -> str.\n\
 Return the ANSI escape sequence for this color.\n\
 IS_FOREGROUND indicates whether this is a foreground or background color."},
diff --git a/gdb/testsuite/gdb.python/py-color.exp b/gdb/testsuite/gdb.python/py-color.exp
index 7f711583b2c..3563d221200 100644
--- a/gdb/testsuite/gdb.python/py-color.exp
+++ b/gdb/testsuite/gdb.python/py-color.exp
@@ -108,6 +108,12 @@ gdb_test [concat "python print (c_red.escape_sequence (True) + " \
     "c_none.escape_sequence (True))"] \
     "\033\\\[31m\033\\\[42mred on green\033\\\[49m red on default\033\\\[39m" \
     "escape sequences"
+gdb_test [concat "python print (c_red.escape_sequence (is_foreground = True) + " \
+    "c_green.escape_sequence (is_foreground = False) + 'red on green' + " \
+    "c_none.escape_sequence (is_foreground = False) + ' red on default' + " \
+    "c_none.escape_sequence (is_foreground = True))"] \
+    "\033\\\[31m\033\\\[42mred on green\033\\\[49m red on default\033\\\[39m" \
+    "escape sequences using keyword arguments"
 
 gdb_test_multiline "Try to sub-class gdb.Color" \
     "python" "" \
@@ -142,3 +148,8 @@ gdb_test "python color_param.value = bad_obj" \
 	 "Python Exception <class 'RuntimeError'>: color argument must be a gdb\\.Color object\\." \
 	 "Error occurred in Python: color argument must be a gdb\\.Color object\\."] \
     "set color parameter to a non-color type"
+
+gdb_test "python c_none.escape_sequence(c_red)" \
+    [multi_line \
+	 "Python Exception <class 'TypeError'>: argument 1 must be bool, not gdb.Color" \
+	 "Error occurred in Python: argument 1 must be bool, not gdb.Color"]