[patch] Implement set/show callback functions in gdb.Parameter

[patch] Implement set/show callback functions in gdb.Parameter

[Phil Muldoon]

This patch allows the user to implement two methods in the gdb.Parameter
object. 

get_set_string is called when the 'set' API of the gdb parameter is
called.  GDB will call this Python function, and use the output as the
output from the 'set' command. Similar happens with get_show_string and
'show'. This allows us to remove the deprecated hooks we were calling
with the old add_setshow_* library of parameters. It also allows the
user to return data/information on the real-time state of the object as
the callbacks are executed each time.

Comments?

Cheers

Phil

--


2011-02-15  Phil Muldoon  <pmuldoon@redhat.com>

	* python/py-param.c (add_setshow_generic): Add set/show callback
	parameters. Register Python object context.
	(get_show_value): New function.
	(get_set_value): New function.
	(call_doc_function): New function.
	(get_doc_string): Move behind get_show_value/get_set_value.

2011-02-15  Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.texinfo (Parameters In Python): Document get_set_string and
	get_show_string methods.

2011-02-15  Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.python/py-parameter.exp: Update tests to the new Python
	parameter API. Add "no documentation" test.  Add deprecated API
	backward compatibility test.

--

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bed074f..0ad1186 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22397,6 +22397,22 @@ parameter.  It can be read and assigned to just as any other
 attribute.  @value{GDBN} does validation when assignments are made.
 @end defivar
 
+There are two methods that should be implemented in any Parameter
+class.  These are:
+
+@defop Operation {parameter} get_set_string (self)
+@value{GDBN} will call this method when a parameter has been
+invoked via the set API (for example, set foo off).  The @code{value}
+attribute has already been populated with the new value and may be
+used in output.  This method must return a string.
+@end defop
+
+@defop Operation {parameter} get_show_string (self, svalue)
+@value{GDBN} will call this method when the parameter has been
+invoked via the show API (for example, show foo).  The argument
+@code{svalue} contains a string representing what @value{GDBN} has
+stored for this parameter.  This method must return a string.
+@end defop
 
 When a new parameter is defined, its type must be specified.  The
 available types are represented by constants defined in the @code{gdb}
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index 22aa91d..a1c7ddd 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -26,6 +26,8 @@
 #include "gdbcmd.h"
 #include "cli/cli-decode.h"
 #include "completer.h"
+#include "language.h"
+#include "arch-utils.h"
 
 /* Parameter constants and their values.  */
 struct parm_constant
@@ -289,6 +291,165 @@ set_attr (PyObject *obj, PyObject *attr_name, PyObject *val)
   return PyObject_GenericSetAttr (obj, attr_name, val);
 }
 
+/* A helper function which returns a documentation string for an
+   object. */
+
+static char *
+get_doc_string (PyObject *object, PyObject *attr)
+{
+  char *result = NULL;
+
+  if (PyObject_HasAttr (object, attr))
+    {
+      PyObject *ds_obj = PyObject_GetAttr (object, attr);
+
+      if (ds_obj && gdbpy_is_string (ds_obj))
+	{
+	  result = python_string_to_host_string (ds_obj);
+	  if (result == NULL)
+	    gdbpy_print_stack ();
+	}
+    }
+  if (! result)
+    result = xstrdup (_("This command is not documented."));
+  return result;
+}
+
+/* Helper function which will execute a METHOD in OBJ passing the
+   argument ARG.  ARG can be NULL.  METHOD should return a Python
+   string.  If this function returns NULL, there has been an error and
+   the appropriate exception set.  */
+static char *
+call_doc_function (PyObject *obj, PyObject *method, PyObject *arg)
+{
+  char *data = NULL;
+  struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
+  PyObject *result = PyObject_CallMethodObjArgs (obj, method, arg, NULL);
+
+  if (! result)
+    return NULL;
+
+  make_cleanup_py_decref (result);
+  if (gdbpy_is_string (result))
+    {
+      data = python_string_to_host_string (result);
+      if (data == NULL)
+	goto error;
+    }
+  else
+    {
+      PyErr_SetString (PyExc_RuntimeError,
+		       _("Parameter must return a string value."));
+      goto error;
+    }
+
+  do_cleanups (cleanup);
+  return data;
+
+ error:
+  do_cleanups (cleanup);
+  return NULL;
+}
+
+/* A callback function that is registered against the respective
+   add_setshow_* set_doc prototype.  This function will either call
+   the Python function "get_set_string" or extract the Python
+   attribute "set_doc" and return the contents as a string.  If
+   neither exist, insert a string indicating the Parameter is not
+   documented.  */
+static void
+get_set_value (char *args, int from_tty,
+	       struct cmd_list_element *c)
+{
+  PyObject *obj = (PyObject *) get_cmd_context (c);
+  char *set_doc_string;
+  PyObject *set_doc_func = PyString_FromString ("get_set_string");
+  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
+					       current_language);
+
+  make_cleanup_py_decref (set_doc_func);
+
+  if (PyObject_HasAttr (obj, set_doc_func))
+    {
+      set_doc_string = call_doc_function (obj, set_doc_func, NULL);
+      if (! set_doc_string)
+	goto error;
+    }
+  else
+    {
+      /* We have to preserve the existing < GDB 7.3 API.  If a
+	 callback function does not exist, then attempt to read the
+	 set_doc attribute.  */
+      set_doc_string  = get_doc_string (obj, set_doc_cst);
+    }
+
+  make_cleanup (xfree, set_doc_string);
+  fprintf_filtered (gdb_stdout,"%s\n", set_doc_string);
+
+  do_cleanups (cleanup);
+  return;
+
+ error:
+  gdbpy_print_stack ();
+  do_cleanups (cleanup);
+  return;
+}
+
+/* A callback function that is registered against the respective
+   add_setshow_* show_doc prototype.  This function will either call
+   the Python function "get_show_string" or extract the Python
+   attribute "show_doc" and return the contents as a string.  If
+   neither exist, insert a string indicating the Parameter is not
+   documented.  */
+static void
+get_show_value (struct ui_file *file, int from_tty,
+		struct cmd_list_element *c,
+		const char *value)
+{
+  PyObject *obj = (PyObject *) get_cmd_context (c);
+  char *show_doc_string = NULL;
+  PyObject *show_doc_func = PyString_FromString ("get_show_string");
+  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
+					       current_language);
+
+  make_cleanup_py_decref (show_doc_func);
+
+  if (PyObject_HasAttr (obj, show_doc_func))
+    {
+      PyObject *result;
+      PyObject *val_obj = PyString_FromString (value);
+
+      if (! val_obj)
+	  goto error;
+
+      make_cleanup_py_decref (val_obj);
+
+      show_doc_string = call_doc_function (obj, show_doc_func, val_obj);
+      if (! show_doc_string)
+	goto error;
+
+      make_cleanup (xfree, show_doc_string);
+
+      fprintf_filtered (file,"%s\n", show_doc_string);
+    }
+  else
+    {
+      /* We have to preserve the existing < GDB 7.3 API.  If a
+	 callback function does not exist, then attempt to read the
+	 show_doc attribute.  */
+      show_doc_string  = get_doc_string (obj, show_doc_cst);
+      make_cleanup(xfree, show_doc_string);
+      fprintf_filtered (file,"%s %s\n", show_doc_string, value);
+    }
+
+  do_cleanups (cleanup);
+  return;
+
+ error:
+  gdbpy_print_stack ();
+  do_cleanups (cleanup);
+  return;
+}
 \f
 
 /* A helper function that dispatches to the appropriate add_setshow
@@ -300,74 +461,95 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
 		     struct cmd_list_element **set_list,
 		     struct cmd_list_element **show_list)
 {
-  switch (parmclass)
-    {
-    case var_boolean:
-      add_setshow_boolean_cmd (cmd_name, cmdclass, &self->value.intval,
-			       set_doc, show_doc, help_doc,
-			       NULL, NULL, set_list, show_list);
+  struct cmd_list_element *param = NULL;
+  char *tmp_name = NULL;
+
+  switch (parmclass) { case var_boolean:
+
+      add_setshow_boolean_cmd (cmd_name, cmdclass,
+			       &self->value.intval, set_doc, show_doc,
+			       help_doc, get_set_value, get_show_value,
+			       set_list, show_list);
+
       break;
 
     case var_auto_boolean:
       add_setshow_auto_boolean_cmd (cmd_name, cmdclass,
 				    &self->value.autoboolval,
 				    set_doc, show_doc, help_doc,
-				    NULL, NULL, set_list, show_list);
+				    get_set_value, get_show_value,
+				    set_list, show_list);
       break;
 
     case var_uinteger:
-      add_setshow_uinteger_cmd (cmd_name, cmdclass, &self->value.uintval,
-				set_doc, show_doc, help_doc,
-				NULL, NULL, set_list, show_list);
+      add_setshow_uinteger_cmd (cmd_name, cmdclass,
+				&self->value.uintval, set_doc, show_doc,
+				help_doc, get_set_value, get_show_value,
+				set_list, show_list);
       break;
 
     case var_integer:
-      add_setshow_integer_cmd (cmd_name, cmdclass, &self->value.intval,
-			       set_doc, show_doc, help_doc,
-			       NULL, NULL, set_list, show_list);
-      break;
+      add_setshow_integer_cmd (cmd_name, cmdclass,
+			       &self->value.intval, set_doc, show_doc,
+			       help_doc, get_set_value, get_show_value,
+			       set_list, show_list); break;
 
     case var_string:
-      add_setshow_string_cmd (cmd_name, cmdclass, &self->value.stringval,
-			      set_doc, show_doc, help_doc,
-			      NULL, NULL, set_list, show_list);
-      break;
+      add_setshow_string_cmd (cmd_name, cmdclass,
+			      &self->value.stringval, set_doc, show_doc,
+			      help_doc, get_set_value, get_show_value,
+			      set_list, show_list); break;
 
     case var_string_noescape:
       add_setshow_string_noescape_cmd (cmd_name, cmdclass,
 				       &self->value.stringval,
 				       set_doc, show_doc, help_doc,
-				       NULL, NULL, set_list, show_list);
+				       get_set_value, get_show_value,
+				       set_list, show_list);
+
       break;
 
     case var_optional_filename:
       add_setshow_optional_filename_cmd (cmd_name, cmdclass,
-					 &self->value.stringval,
-					 set_doc, show_doc, help_doc,
-					 NULL, NULL, set_list, show_list);
+					 &self->value.stringval, set_doc,
+					 show_doc, help_doc, get_set_value,
+					 get_show_value, set_list,
+					 show_list);
       break;
 
     case var_filename:
-      add_setshow_filename_cmd (cmd_name, cmdclass, &self->value.stringval,
-				set_doc, show_doc, help_doc,
-				NULL, NULL, set_list, show_list);
-      break;
+      add_setshow_filename_cmd (cmd_name, cmdclass,
+				&self->value.stringval, set_doc, show_doc,
+				help_doc, get_set_value, get_show_value,
+				set_list, show_list); break;
 
     case var_zinteger:
-      add_setshow_zinteger_cmd (cmd_name, cmdclass, &self->value.intval,
-				set_doc, show_doc, help_doc,
-				NULL, NULL, set_list, show_list);
+      add_setshow_zinteger_cmd (cmd_name, cmdclass,
+				&self->value.intval, set_doc, show_doc,
+				help_doc, get_set_value, get_show_value,
+				set_list, show_list);
       break;
 
     case var_enum:
       add_setshow_enum_cmd (cmd_name, cmdclass, self->enumeration,
-			    &self->value.cstringval,
-			    set_doc, show_doc, help_doc,
-			    NULL, NULL, set_list, show_list);
+			    &self->value.cstringval, set_doc, show_doc,
+			    help_doc, get_set_value, get_show_value,
+			    set_list, show_list);
       /* Initialize the value, just in case.  */
       self->value.cstringval = self->enumeration[0];
       break;
     }
+
+  /* Lookup created parameter, and register Python object against the
+     parameter context.  Perform this task against both lists.  */
+  tmp_name = cmd_name;
+  param = lookup_cmd (&tmp_name, *show_list, "", 0, 1);
+  if (param)
+    set_cmd_context (param, self);
+
+  tmp_name = cmd_name;
+  param = lookup_cmd (&tmp_name, *set_list, "", 0, 1);
+  if (param)
+    set_cmd_context (param, self);
 }
 
 /* A helper which computes enum values.  Returns 1 on success.  Returns 0 on
@@ -435,29 +617,6 @@ compute_enum_values (parmpy_object *self, PyObject *enum_values)
   return 1;
 }
 
-/* A helper function which returns a documentation string for an
-   object.  */
-static char *
-get_doc_string (PyObject *object, PyObject *attr)
-{
-  char *result = NULL;
-
-  if (PyObject_HasAttr (object, attr))
-    {
-      PyObject *ds_obj = PyObject_GetAttr (object, attr);
-
-      if (ds_obj && gdbpy_is_string (ds_obj))
-	{
-	  result = python_string_to_host_string (ds_obj);
-	  if (result == NULL)
-	    gdbpy_print_stack ();
-	}
-    }
-  if (! result)
-    result = xstrdup (_("This command is not documented."));
-  return result;
-}
-
 /* Object initializer; sets up gdb-side structures for command.
 
    Use: __init__(NAME, CMDCLASS, PARMCLASS, [ENUM])
diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
index 261c0d5..63f4ff2 100644
--- a/gdb/testsuite/gdb.python/py-parameter.exp
+++ b/gdb/testsuite/gdb.python/py-parameter.exp
@@ -38,8 +38,15 @@ gdb_py_test_multiple "Simple gdb booleanparameter" \
    "python" "" \
    "class TestParam (gdb.Parameter):" "" \
    "   \"\"\"When enabled, test param does something useful. When disabled, does nothing.\"\"\"" "" \
-   "   show_doc = \"Show whether the state of the Test Parameter does something useful\"" ""\
-   "   set_doc = \"Set whether the state of the Test Parameter does something useful\"" "" \
+   "   show_doc = \"Show the state of the boolean test-param\"" ""\
+   "   set_doc = \"Set the state of the boolean test-param\"" "" \
+   "   def get_show_string (self, pvalue):" ""\
+   "      return \"The state of the Test Parameter is \" + pvalue" ""\
+   "   def get_set_string (self):" ""\
+   "      val = \"on\"" ""\
+   "      if (self.value == False):" ""\
+   "         val = \"off\"" ""\
+   "      return \"Test Parameter has been set to \" + val" ""\
    "   def __init__ (self, name):" "" \
    "      super (TestParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
    "      self.value = True" "" \
@@ -47,13 +54,14 @@ gdb_py_test_multiple "Simple gdb booleanparameter" \
    "end"
 
 gdb_test "python print test_param.value" "True" "Test parameter value"
-gdb_test "show print test-param" "Whether the state of the Test Parameter does something useful is on.*" "Show parameter on"
-gdb_py_test_silent_cmd "set print test-param off" "Turn off parameter" 1
-gdb_test "show print test-param" "Whether the state of the Test Parameter does something useful is off.*" "Show parameter off"
+gdb_test "show print test-param" "The state of the Test Parameter is on.*" "Show parameter on"
+gdb_test "set print test-param off" "Test Parameter has been set to off" "Turn off parameter"
+gdb_test "show print test-param" "The state of the Test Parameter is off.*" "Show parameter off"
 gdb_test "python print test_param.value" "False" "Test parameter value"
-gdb_test "help show print test-param" "Show whether the state of the Test Parameter does something useful.*" "Test show help"
-gdb_test "help set print test-param" "Set whether the state of the Test Parameter does something useful.*" "Test set help"
-gdb_test "help set print" "set print test-param -- Set whether the state of the Test Parameter.*" "Test general help"
+gdb_test "help show print test-param" "Show the state of the boolean test-param.*" "Test show help"
+gdb_test "help set print test-param" "Set the state of the boolean test-param.*" "Test set help"
+gdb_test "help set print" "set print test-param -- Set the state of the boolean test-param.*" "Test general help"
+
 
 # Test an enum parameter.
 gdb_py_test_multiple "enum gdb parameter" \
@@ -62,6 +70,10 @@ gdb_py_test_multiple "enum gdb parameter" \
    "   \"\"\"When set, test param does something useful. When disabled, does nothing.\"\"\"" "" \
    "   show_doc = \"Show the state of the enum\"" ""\
    "   set_doc = \"Set the state of the enum\"" "" \
+   "   def get_show_string (self, pvalue):" ""\
+   "      return \"The state of the enum is \" + pvalue" ""\
+   "   def get_set_string (self):" ""\
+   "      return \"The state of the enum has been set to \" + self.value" ""\
    "   def __init__ (self, name):" "" \
    "      super (TestEnumParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_ENUM, \[\"one\", \"two\"\])" "" \
    "      self.value = \"one\"" "" \
@@ -69,9 +81,9 @@ gdb_py_test_multiple "enum gdb parameter" \
    "end"
 
 gdb_test "python print test_enum_param.value" "one" "Test enum parameter value"
-gdb_test "show print test-enum-param" "The state of the enum is \"one\".*" "Show parameter is initial value"
-gdb_py_test_silent_cmd "set print test-enum-param two" "Set parameter to enum value" 1
-gdb_test "show print test-enum-param" "The state of the enum is \"two\".*" "Show parameter is new value"
+gdb_test "show print test-enum-param" "The state of the enum is one.*" "Show parameter is initial value"
+gdb_test "set print test-enum-param two" "The state of the enum has been set to two" "Set enum to two"
+gdb_test "show print test-enum-param" "The state of the enum is two.*" "Show parameter is new value"
 gdb_test "python print test_enum_param.value" "two" "Test enum parameter value"
 gdb_test "set print test-enum-param three" "Undefined item: \"three\".*" "Set invalid enum parameter" 
 
@@ -82,6 +94,10 @@ gdb_py_test_multiple "file gdb parameter" \
    "   \"\"\"When set, test param does something useful. When disabled, does nothing.\"\"\"" "" \
    "   show_doc = \"Show the name of the file\"" ""\
    "   set_doc = \"Set the name of the file\"" "" \
+   "   def get_show_string (self, pvalue):" ""\
+   "      return \"The name of the file is \" + pvalue" ""\
+   "   def get_set_string (self):" ""\
+   "      return \"The name of the file has been changed to \" + self.value" ""\
    "   def __init__ (self, name):" "" \
    "      super (TestFileParam, self).__init__ (name, gdb.COMMAND_FILES, gdb.PARAM_FILENAME)" "" \
    "      self.value = \"foo.txt\"" "" \
@@ -89,28 +105,73 @@ gdb_py_test_multiple "file gdb parameter" \
    "end"
 
 gdb_test "python print test_file_param.value" "foo.txt" "Test file parameter value"
-gdb_test "show test-file-param" "The name of the file is \"foo.txt\".*" "Show initial file value"
-gdb_py_test_silent_cmd "set test-file-param bar.txt" "Set new file parameter" 1
-gdb_test "show test-file-param" "The name of the file is \"bar.txt\".*" "Show new file value"
+gdb_test "show test-file-param" "The name of the file is foo.txt.*" "Show initial file value"
+gdb_test "set test-file-param bar.txt" "The name of the file has been changed to bar.txt" "Set new file parameter" 1
+gdb_test "show test-file-param" "The name of the file is bar.txt.*" "Show new file value"
 gdb_test "python print test_file_param.value" "bar.txt" "Test new file parameter value"
 gdb_test "set test-file-param" "Argument required.*" 
 
-# Test a file parameter.
-gdb_py_test_multiple "file gdb parameter" \
+# Test a parameter that is not documented.
+gdb_py_test_multiple "Simple gdb booleanparameter" \
    "python" "" \
-   "class TestFileParam (gdb.Parameter):" "" \
-   "   \"\"\"When set, test param does something useful. When disabled, does nothing.\"\"\"" "" \
-   "   show_doc = \"Show the name of the file\"" ""\
-   "   set_doc = \"Set the name of the file\"" "" \
+   "class TestUndocParam (gdb.Parameter):" "" \
+   "   def get_show_string (self, pvalue):" ""\
+   "      return \"The state of the Test Parameter is \" + pvalue" ""\
+   "   def get_set_string (self):" ""\
+   "      val = \"on\"" ""\
+   "      if (self.value == False):" ""\
+   "         val = \"off\"" ""\
+   "      return \"Test Parameter has been set to \" + val" ""\
    "   def __init__ (self, name):" "" \
-   "      super (TestFileParam, self).__init__ (name, gdb.COMMAND_FILES, gdb.PARAM_FILENAME)" "" \
-   "      self.value = \"foo.txt\"" "" \
-   "test_file_param = TestFileParam ('test-file-param')" ""\
+   "      super (TestUndocParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
+   "      self.value = True" "" \
+   "test_undoc_param = TestUndocParam ('print test-undoc-param')" ""\
    "end"
 
-gdb_test "python print test_file_param.value" "foo.txt" "Test parameter value"
-gdb_test "show test-file-param" "The name of the file is \"foo.txt\".*" "Show parameter on"
-gdb_py_test_silent_cmd "set test-file-param bar.txt" "Turn off parameter" 1
-gdb_test "show test-file-param" "The name of the file is \"bar.txt\".*" "Show parameter on"
-gdb_test "python print test_file_param.value" "bar.txt" "Test parameter value"
-gdb_test "set test-file-param" "Argument required.*" 
+gdb_test "show print test-undoc-param" "The state of the Test Parameter is on.*" "Show parameter on"
+gdb_test "set print test-undoc-param off" "Test Parameter has been set to off" "Turn off parameter"
+gdb_test "show print test-undoc-param" "The state of the Test Parameter is off.*" "Show parameter off"
+gdb_test "python print test_undoc_param.value" "False" "Test parameter value"
+gdb_test "help show print test-undoc-param" "This command is not documented.*" "Test show help"
+gdb_test "help set print test-undoc-param" "This command is not documented.*" "Test set help"
+gdb_test "help set print" "set print test-undoc-param -- This command is not documented.*" "Test general help"
+
+# Test a parameter that is not documented in any way..
+gdb_py_test_multiple "Simple gdb booleanparameter" \
+   "python" "" \
+   "class TestNodocParam (gdb.Parameter):" "" \
+   "   def __init__ (self, name):" "" \
+   "      super (TestNodocParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
+   "      self.value = True" "" \
+   "test_nodoc_param = TestNodocParam ('print test-nodoc-param')" ""\
+   "end"
+
+gdb_test "show print test-nodoc-param" "This command is not documented.*" "Show parameter on"
+gdb_test "set print test-nodoc-param off" "This command is not documented.*" "Turn off parameter"
+gdb_test "show print test-nodoc-param" "This command is not documented.*.*" "Show parameter off"
+gdb_test "python print test_nodoc_param.value" "False" "Test parameter value"
+gdb_test "help show print test-nodoc-param" "This command is not documented.*" "Test show help"
+gdb_test "help set print test-nodoc-param" "This command is not documented.*" "Test set help"
+gdb_test "help set print" "set print test-nodoc-param -- This command is not documented.*" "Test general help"
+
+# Test deprecated API. Do not use in your own implementations.
+gdb_py_test_multiple "Simple gdb booleanparameter" \
+   "python" "" \
+   "class TestParam (gdb.Parameter):" "" \
+   "   \"\"\"When enabled, test param does something useful. When disabled, does nothing.\"\"\"" "" \
+   "   show_doc = \"State of the Test Parameter\"" ""\
+   "   set_doc = \"Set the state of the Test Parameter\"" "" \
+   "   def __init__ (self, name):" "" \
+   "      super (TestParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
+   "      self.value = True" "" \
+   "test_param = TestParam ('print test-param')" ""\
+   "end"
+
+gdb_test "python print test_param.value" "True" "Test parameter value"
+gdb_test "show print test-param" "State of the Test Parameter on.*" "Show parameter on"
+gdb_test "set print test-param off" "Set the state of the Test Parameter.*" "Turn off parameter"
+gdb_test "show print test-param" "State of the Test Parameter off.*" "Show parameter off"
+gdb_test "python print test_param.value" "False" "Test parameter value"
+gdb_test "help show print test-param" "State of the Test Parameter.*" "Test show help"
+gdb_test "help set print test-param" "Set the state of the Test Parameter.*" "Test set help"
+gdb_test "help set print" "set print test-param -- Set the state of the Test Parameter.*" "Test general help"

Re: [patch] Implement set/show callback functions in gdb.Parameter

[Eli Zaretskii]

> From: Phil Muldoon <pmuldoon@redhat.com>
> Date: Tue, 15 Feb 2011 16:24:47 +0000
> 
> 
> This patch allows the user to implement two methods in the gdb.Parameter
> object. 

Thanks.

> +There are two methods that should be implemented in any Parameter
> +class.  These are:

"Parameter" should be in @code, since it is a Python symbol.

> +@defop Operation {parameter} get_set_string (self)

The @def... commands in Texinfo don't use parentheses to enclose
argument.  Just use

  @defop Operation {parameter} get_set_string self

> +@value{GDBN} will call this method when a parameter has been
> +invoked via the set API (for example, set foo off).  The @code{value}

Are talking here about a Python API or about the CLI command "set"?
"set" should be in @code in both cases.  "set foo off" should be in
@kbd if it's about a CLI command; if it is about a Python API, I'm not
sure I understand what it means.

Similar issues with get_show_string.

> +                                                   The argument
> +@code{svalue} contains a string representing what @value{GDBN} has
> +stored for this parameter.

Suggest to rephrase:

  The argument @code{svalue} receives the string representation of the
  current value.

>                               This method must return a string.

You mean, in `svalue'?  If so, "return" is not really appropriate, is
it?

If you mean something else, then I think we should elaborate about
that.

Thanks.

Re: [patch] Implement set/show callback functions in gdb.Parameter

[Phil Muldoon]

Phil Muldoon <pmuldoon@redhat.com> writes:

ping

> This patch allows the user to implement two methods in the gdb.Parameter
> object. 
>
> get_set_string is called when the 'set' API of the gdb parameter is
> called.  GDB will call this Python function, and use the output as the
> output from the 'set' command. Similar happens with get_show_string and
> 'show'. This allows us to remove the deprecated hooks we were calling
> with the old add_setshow_* library of parameters. It also allows the
> user to return data/information on the real-time state of the object as
> the callbacks are executed each time.
>
> Comments?
>
> Cheers
>
> Phil
>
> --
>
>
> 2011-02-15  Phil Muldoon  <pmuldoon@redhat.com>
>
> 	* python/py-param.c (add_setshow_generic): Add set/show callback
> 	parameters. Register Python object context.
> 	(get_show_value): New function.
> 	(get_set_value): New function.
> 	(call_doc_function): New function.
> 	(get_doc_string): Move behind get_show_value/get_set_value.
>
> 2011-02-15  Phil Muldoon  <pmuldoon@redhat.com>
>
> 	* gdb.texinfo (Parameters In Python): Document get_set_string and
> 	get_show_string methods.
>
> 2011-02-15  Phil Muldoon  <pmuldoon@redhat.com>
>
> 	* gdb.python/py-parameter.exp: Update tests to the new Python
> 	parameter API. Add "no documentation" test.  Add deprecated API
> 	backward compatibility test.
>
> --
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index bed074f..0ad1186 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -22397,6 +22397,22 @@ parameter.  It can be read and assigned to just as any other
>  attribute.  @value{GDBN} does validation when assignments are made.
>  @end defivar
>  
> +There are two methods that should be implemented in any Parameter
> +class.  These are:
> +
> +@defop Operation {parameter} get_set_string (self)
> +@value{GDBN} will call this method when a parameter has been
> +invoked via the set API (for example, set foo off).  The @code{value}
> +attribute has already been populated with the new value and may be
> +used in output.  This method must return a string.
> +@end defop
> +
> +@defop Operation {parameter} get_show_string (self, svalue)
> +@value{GDBN} will call this method when the parameter has been
> +invoked via the show API (for example, show foo).  The argument
> +@code{svalue} contains a string representing what @value{GDBN} has
> +stored for this parameter.  This method must return a string.
> +@end defop
>  
>  When a new parameter is defined, its type must be specified.  The
>  available types are represented by constants defined in the @code{gdb}
> diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
> index 22aa91d..a1c7ddd 100644
> --- a/gdb/python/py-param.c
> +++ b/gdb/python/py-param.c
> @@ -26,6 +26,8 @@
>  #include "gdbcmd.h"
>  #include "cli/cli-decode.h"
>  #include "completer.h"
> +#include "language.h"
> +#include "arch-utils.h"
>  
>  /* Parameter constants and their values.  */
>  struct parm_constant
> @@ -289,6 +291,165 @@ set_attr (PyObject *obj, PyObject *attr_name, PyObject *val)
>    return PyObject_GenericSetAttr (obj, attr_name, val);
>  }
>  
> +/* A helper function which returns a documentation string for an
> +   object. */
> +
> +static char *
> +get_doc_string (PyObject *object, PyObject *attr)
> +{
> +  char *result = NULL;
> +
> +  if (PyObject_HasAttr (object, attr))
> +    {
> +      PyObject *ds_obj = PyObject_GetAttr (object, attr);
> +
> +      if (ds_obj && gdbpy_is_string (ds_obj))
> +	{
> +	  result = python_string_to_host_string (ds_obj);
> +	  if (result == NULL)
> +	    gdbpy_print_stack ();
> +	}
> +    }
> +  if (! result)
> +    result = xstrdup (_("This command is not documented."));
> +  return result;
> +}
> +
> +/* Helper function which will execute a METHOD in OBJ passing the
> +   argument ARG.  ARG can be NULL.  METHOD should return a Python
> +   string.  If this function returns NULL, there has been an error and
> +   the appropriate exception set.  */
> +static char *
> +call_doc_function (PyObject *obj, PyObject *method, PyObject *arg)
> +{
> +  char *data = NULL;
> +  struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
> +  PyObject *result = PyObject_CallMethodObjArgs (obj, method, arg, NULL);
> +
> +  if (! result)
> +    return NULL;
> +
> +  make_cleanup_py_decref (result);
> +  if (gdbpy_is_string (result))
> +    {
> +      data = python_string_to_host_string (result);
> +      if (data == NULL)
> +	goto error;
> +    }
> +  else
> +    {
> +      PyErr_SetString (PyExc_RuntimeError,
> +		       _("Parameter must return a string value."));
> +      goto error;
> +    }
> +
> +  do_cleanups (cleanup);
> +  return data;
> +
> + error:
> +  do_cleanups (cleanup);
> +  return NULL;
> +}
> +
> +/* A callback function that is registered against the respective
> +   add_setshow_* set_doc prototype.  This function will either call
> +   the Python function "get_set_string" or extract the Python
> +   attribute "set_doc" and return the contents as a string.  If
> +   neither exist, insert a string indicating the Parameter is not
> +   documented.  */
> +static void
> +get_set_value (char *args, int from_tty,
> +	       struct cmd_list_element *c)
> +{
> +  PyObject *obj = (PyObject *) get_cmd_context (c);
> +  char *set_doc_string;
> +  PyObject *set_doc_func = PyString_FromString ("get_set_string");
> +  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
> +					       current_language);
> +
> +  make_cleanup_py_decref (set_doc_func);
> +
> +  if (PyObject_HasAttr (obj, set_doc_func))
> +    {
> +      set_doc_string = call_doc_function (obj, set_doc_func, NULL);
> +      if (! set_doc_string)
> +	goto error;
> +    }
> +  else
> +    {
> +      /* We have to preserve the existing < GDB 7.3 API.  If a
> +	 callback function does not exist, then attempt to read the
> +	 set_doc attribute.  */
> +      set_doc_string  = get_doc_string (obj, set_doc_cst);
> +    }
> +
> +  make_cleanup (xfree, set_doc_string);
> +  fprintf_filtered (gdb_stdout,"%s\n", set_doc_string);
> +
> +  do_cleanups (cleanup);
> +  return;
> +
> + error:
> +  gdbpy_print_stack ();
> +  do_cleanups (cleanup);
> +  return;
> +}
> +
> +/* A callback function that is registered against the respective
> +   add_setshow_* show_doc prototype.  This function will either call
> +   the Python function "get_show_string" or extract the Python
> +   attribute "show_doc" and return the contents as a string.  If
> +   neither exist, insert a string indicating the Parameter is not
> +   documented.  */
> +static void
> +get_show_value (struct ui_file *file, int from_tty,
> +		struct cmd_list_element *c,
> +		const char *value)
> +{
> +  PyObject *obj = (PyObject *) get_cmd_context (c);
> +  char *show_doc_string = NULL;
> +  PyObject *show_doc_func = PyString_FromString ("get_show_string");
> +  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
> +					       current_language);
> +
> +  make_cleanup_py_decref (show_doc_func);
> +
> +  if (PyObject_HasAttr (obj, show_doc_func))
> +    {
> +      PyObject *result;
> +      PyObject *val_obj = PyString_FromString (value);
> +
> +      if (! val_obj)
> +	  goto error;
> +
> +      make_cleanup_py_decref (val_obj);
> +
> +      show_doc_string = call_doc_function (obj, show_doc_func, val_obj);
> +      if (! show_doc_string)
> +	goto error;
> +
> +      make_cleanup (xfree, show_doc_string);
> +
> +      fprintf_filtered (file,"%s\n", show_doc_string);
> +    }
> +  else
> +    {
> +      /* We have to preserve the existing < GDB 7.3 API.  If a
> +	 callback function does not exist, then attempt to read the
> +	 show_doc attribute.  */
> +      show_doc_string  = get_doc_string (obj, show_doc_cst);
> +      make_cleanup(xfree, show_doc_string);
> +      fprintf_filtered (file,"%s %s\n", show_doc_string, value);
> +    }
> +
> +  do_cleanups (cleanup);
> +  return;
> +
> + error:
> +  gdbpy_print_stack ();
> +  do_cleanups (cleanup);
> +  return;
> +}
>  \f
>  
>  /* A helper function that dispatches to the appropriate add_setshow
> @@ -300,74 +461,95 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
>  		     struct cmd_list_element **set_list,
>  		     struct cmd_list_element **show_list)
>  {
> -  switch (parmclass)
> -    {
> -    case var_boolean:
> -      add_setshow_boolean_cmd (cmd_name, cmdclass, &self->value.intval,
> -			       set_doc, show_doc, help_doc,
> -			       NULL, NULL, set_list, show_list);
> +  struct cmd_list_element *param = NULL;
> +  char *tmp_name = NULL;
> +
> +  switch (parmclass) { case var_boolean:
> +
> +      add_setshow_boolean_cmd (cmd_name, cmdclass,
> +			       &self->value.intval, set_doc, show_doc,
> +			       help_doc, get_set_value, get_show_value,
> +			       set_list, show_list);
> +
>        break;
>  
>      case var_auto_boolean:
>        add_setshow_auto_boolean_cmd (cmd_name, cmdclass,
>  				    &self->value.autoboolval,
>  				    set_doc, show_doc, help_doc,
> -				    NULL, NULL, set_list, show_list);
> +				    get_set_value, get_show_value,
> +				    set_list, show_list);
>        break;
>  
>      case var_uinteger:
> -      add_setshow_uinteger_cmd (cmd_name, cmdclass, &self->value.uintval,
> -				set_doc, show_doc, help_doc,
> -				NULL, NULL, set_list, show_list);
> +      add_setshow_uinteger_cmd (cmd_name, cmdclass,
> +				&self->value.uintval, set_doc, show_doc,
> +				help_doc, get_set_value, get_show_value,
> +				set_list, show_list);
>        break;
>  
>      case var_integer:
> -      add_setshow_integer_cmd (cmd_name, cmdclass, &self->value.intval,
> -			       set_doc, show_doc, help_doc,
> -			       NULL, NULL, set_list, show_list);
> -      break;
> +      add_setshow_integer_cmd (cmd_name, cmdclass,
> +			       &self->value.intval, set_doc, show_doc,
> +			       help_doc, get_set_value, get_show_value,
> +			       set_list, show_list); break;
>  
>      case var_string:
> -      add_setshow_string_cmd (cmd_name, cmdclass, &self->value.stringval,
> -			      set_doc, show_doc, help_doc,
> -			      NULL, NULL, set_list, show_list);
> -      break;
> +      add_setshow_string_cmd (cmd_name, cmdclass,
> +			      &self->value.stringval, set_doc, show_doc,
> +			      help_doc, get_set_value, get_show_value,
> +			      set_list, show_list); break;
>  
>      case var_string_noescape:
>        add_setshow_string_noescape_cmd (cmd_name, cmdclass,
>  				       &self->value.stringval,
>  				       set_doc, show_doc, help_doc,
> -				       NULL, NULL, set_list, show_list);
> +				       get_set_value, get_show_value,
> +				       set_list, show_list);
> +
>        break;
>  
>      case var_optional_filename:
>        add_setshow_optional_filename_cmd (cmd_name, cmdclass,
> -					 &self->value.stringval,
> -					 set_doc, show_doc, help_doc,
> -					 NULL, NULL, set_list, show_list);
> +					 &self->value.stringval, set_doc,
> +					 show_doc, help_doc, get_set_value,
> +					 get_show_value, set_list,
> +					 show_list);
>        break;
>  
>      case var_filename:
> -      add_setshow_filename_cmd (cmd_name, cmdclass, &self->value.stringval,
> -				set_doc, show_doc, help_doc,
> -				NULL, NULL, set_list, show_list);
> -      break;
> +      add_setshow_filename_cmd (cmd_name, cmdclass,
> +				&self->value.stringval, set_doc, show_doc,
> +				help_doc, get_set_value, get_show_value,
> +				set_list, show_list); break;
>  
>      case var_zinteger:
> -      add_setshow_zinteger_cmd (cmd_name, cmdclass, &self->value.intval,
> -				set_doc, show_doc, help_doc,
> -				NULL, NULL, set_list, show_list);
> +      add_setshow_zinteger_cmd (cmd_name, cmdclass,
> +				&self->value.intval, set_doc, show_doc,
> +				help_doc, get_set_value, get_show_value,
> +				set_list, show_list);
>        break;
>  
>      case var_enum:
>        add_setshow_enum_cmd (cmd_name, cmdclass, self->enumeration,
> -			    &self->value.cstringval,
> -			    set_doc, show_doc, help_doc,
> -			    NULL, NULL, set_list, show_list);
> +			    &self->value.cstringval, set_doc, show_doc,
> +			    help_doc, get_set_value, get_show_value,
> +			    set_list, show_list);
>        /* Initialize the value, just in case.  */
>        self->value.cstringval = self->enumeration[0];
>        break;
>      }
> +
> +  /* Lookup created parameter, and register Python object against the
> +     parameter context.  Perform this task against both lists.  */
> +  tmp_name = cmd_name;
> +  param = lookup_cmd (&tmp_name, *show_list, "", 0, 1);
> +  if (param)
> +    set_cmd_context (param, self);
> +
> +  tmp_name = cmd_name;
> +  param = lookup_cmd (&tmp_name, *set_list, "", 0, 1);
> +  if (param)
> +    set_cmd_context (param, self);
>  }
>  
>  /* A helper which computes enum values.  Returns 1 on success.  Returns 0 on
> @@ -435,29 +617,6 @@ compute_enum_values (parmpy_object *self, PyObject *enum_values)
>    return 1;
>  }
>  
> -/* A helper function which returns a documentation string for an
> -   object.  */
> -static char *
> -get_doc_string (PyObject *object, PyObject *attr)
> -{
> -  char *result = NULL;
> -
> -  if (PyObject_HasAttr (object, attr))
> -    {
> -      PyObject *ds_obj = PyObject_GetAttr (object, attr);
> -
> -      if (ds_obj && gdbpy_is_string (ds_obj))
> -	{
> -	  result = python_string_to_host_string (ds_obj);
> -	  if (result == NULL)
> -	    gdbpy_print_stack ();
> -	}
> -    }
> -  if (! result)
> -    result = xstrdup (_("This command is not documented."));
> -  return result;
> -}
> -
>  /* Object initializer; sets up gdb-side structures for command.
>  
>     Use: __init__(NAME, CMDCLASS, PARMCLASS, [ENUM])
> diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
> index 261c0d5..63f4ff2 100644
> --- a/gdb/testsuite/gdb.python/py-parameter.exp
> +++ b/gdb/testsuite/gdb.python/py-parameter.exp
> @@ -38,8 +38,15 @@ gdb_py_test_multiple "Simple gdb booleanparameter" \
>     "python" "" \
>     "class TestParam (gdb.Parameter):" "" \
>     "   \"\"\"When enabled, test param does something useful. When disabled, does nothing.\"\"\"" "" \
> -   "   show_doc = \"Show whether the state of the Test Parameter does something useful\"" ""\
> -   "   set_doc = \"Set whether the state of the Test Parameter does something useful\"" "" \
> +   "   show_doc = \"Show the state of the boolean test-param\"" ""\
> +   "   set_doc = \"Set the state of the boolean test-param\"" "" \
> +   "   def get_show_string (self, pvalue):" ""\
> +   "      return \"The state of the Test Parameter is \" + pvalue" ""\
> +   "   def get_set_string (self):" ""\
> +   "      val = \"on\"" ""\
> +   "      if (self.value == False):" ""\
> +   "         val = \"off\"" ""\
> +   "      return \"Test Parameter has been set to \" + val" ""\
>     "   def __init__ (self, name):" "" \
>     "      super (TestParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
>     "      self.value = True" "" \
> @@ -47,13 +54,14 @@ gdb_py_test_multiple "Simple gdb booleanparameter" \
>     "end"
>  
>  gdb_test "python print test_param.value" "True" "Test parameter value"
> -gdb_test "show print test-param" "Whether the state of the Test Parameter does something useful is on.*" "Show parameter on"
> -gdb_py_test_silent_cmd "set print test-param off" "Turn off parameter" 1
> -gdb_test "show print test-param" "Whether the state of the Test Parameter does something useful is off.*" "Show parameter off"
> +gdb_test "show print test-param" "The state of the Test Parameter is on.*" "Show parameter on"
> +gdb_test "set print test-param off" "Test Parameter has been set to off" "Turn off parameter"
> +gdb_test "show print test-param" "The state of the Test Parameter is off.*" "Show parameter off"
>  gdb_test "python print test_param.value" "False" "Test parameter value"
> -gdb_test "help show print test-param" "Show whether the state of the Test Parameter does something useful.*" "Test show help"
> -gdb_test "help set print test-param" "Set whether the state of the Test Parameter does something useful.*" "Test set help"
> -gdb_test "help set print" "set print test-param -- Set whether the state of the Test Parameter.*" "Test general help"
> +gdb_test "help show print test-param" "Show the state of the boolean test-param.*" "Test show help"
> +gdb_test "help set print test-param" "Set the state of the boolean test-param.*" "Test set help"
> +gdb_test "help set print" "set print test-param -- Set the state of the boolean test-param.*" "Test general help"
> +
>  
>  # Test an enum parameter.
>  gdb_py_test_multiple "enum gdb parameter" \
> @@ -62,6 +70,10 @@ gdb_py_test_multiple "enum gdb parameter" \
>     "   \"\"\"When set, test param does something useful. When disabled, does nothing.\"\"\"" "" \
>     "   show_doc = \"Show the state of the enum\"" ""\
>     "   set_doc = \"Set the state of the enum\"" "" \
> +   "   def get_show_string (self, pvalue):" ""\
> +   "      return \"The state of the enum is \" + pvalue" ""\
> +   "   def get_set_string (self):" ""\
> +   "      return \"The state of the enum has been set to \" + self.value" ""\
>     "   def __init__ (self, name):" "" \
>     "      super (TestEnumParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_ENUM, \[\"one\", \"two\"\])" "" \
>     "      self.value = \"one\"" "" \
> @@ -69,9 +81,9 @@ gdb_py_test_multiple "enum gdb parameter" \
>     "end"
>  
>  gdb_test "python print test_enum_param.value" "one" "Test enum parameter value"
> -gdb_test "show print test-enum-param" "The state of the enum is \"one\".*" "Show parameter is initial value"
> -gdb_py_test_silent_cmd "set print test-enum-param two" "Set parameter to enum value" 1
> -gdb_test "show print test-enum-param" "The state of the enum is \"two\".*" "Show parameter is new value"
> +gdb_test "show print test-enum-param" "The state of the enum is one.*" "Show parameter is initial value"
> +gdb_test "set print test-enum-param two" "The state of the enum has been set to two" "Set enum to two"
> +gdb_test "show print test-enum-param" "The state of the enum is two.*" "Show parameter is new value"
>  gdb_test "python print test_enum_param.value" "two" "Test enum parameter value"
>  gdb_test "set print test-enum-param three" "Undefined item: \"three\".*" "Set invalid enum parameter" 
>  
> @@ -82,6 +94,10 @@ gdb_py_test_multiple "file gdb parameter" \
>     "   \"\"\"When set, test param does something useful. When disabled, does nothing.\"\"\"" "" \
>     "   show_doc = \"Show the name of the file\"" ""\
>     "   set_doc = \"Set the name of the file\"" "" \
> +   "   def get_show_string (self, pvalue):" ""\
> +   "      return \"The name of the file is \" + pvalue" ""\
> +   "   def get_set_string (self):" ""\
> +   "      return \"The name of the file has been changed to \" + self.value" ""\
>     "   def __init__ (self, name):" "" \
>     "      super (TestFileParam, self).__init__ (name, gdb.COMMAND_FILES, gdb.PARAM_FILENAME)" "" \
>     "      self.value = \"foo.txt\"" "" \
> @@ -89,28 +105,73 @@ gdb_py_test_multiple "file gdb parameter" \
>     "end"
>  
>  gdb_test "python print test_file_param.value" "foo.txt" "Test file parameter value"
> -gdb_test "show test-file-param" "The name of the file is \"foo.txt\".*" "Show initial file value"
> -gdb_py_test_silent_cmd "set test-file-param bar.txt" "Set new file parameter" 1
> -gdb_test "show test-file-param" "The name of the file is \"bar.txt\".*" "Show new file value"
> +gdb_test "show test-file-param" "The name of the file is foo.txt.*" "Show initial file value"
> +gdb_test "set test-file-param bar.txt" "The name of the file has been changed to bar.txt" "Set new file parameter" 1
> +gdb_test "show test-file-param" "The name of the file is bar.txt.*" "Show new file value"
>  gdb_test "python print test_file_param.value" "bar.txt" "Test new file parameter value"
>  gdb_test "set test-file-param" "Argument required.*" 
>  
> -# Test a file parameter.
> -gdb_py_test_multiple "file gdb parameter" \
> +# Test a parameter that is not documented.
> +gdb_py_test_multiple "Simple gdb booleanparameter" \
>     "python" "" \
> -   "class TestFileParam (gdb.Parameter):" "" \
> -   "   \"\"\"When set, test param does something useful. When disabled, does nothing.\"\"\"" "" \
> -   "   show_doc = \"Show the name of the file\"" ""\
> -   "   set_doc = \"Set the name of the file\"" "" \
> +   "class TestUndocParam (gdb.Parameter):" "" \
> +   "   def get_show_string (self, pvalue):" ""\
> +   "      return \"The state of the Test Parameter is \" + pvalue" ""\
> +   "   def get_set_string (self):" ""\
> +   "      val = \"on\"" ""\
> +   "      if (self.value == False):" ""\
> +   "         val = \"off\"" ""\
> +   "      return \"Test Parameter has been set to \" + val" ""\
>     "   def __init__ (self, name):" "" \
> -   "      super (TestFileParam, self).__init__ (name, gdb.COMMAND_FILES, gdb.PARAM_FILENAME)" "" \
> -   "      self.value = \"foo.txt\"" "" \
> -   "test_file_param = TestFileParam ('test-file-param')" ""\
> +   "      super (TestUndocParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
> +   "      self.value = True" "" \
> +   "test_undoc_param = TestUndocParam ('print test-undoc-param')" ""\
>     "end"
>  
> -gdb_test "python print test_file_param.value" "foo.txt" "Test parameter value"
> -gdb_test "show test-file-param" "The name of the file is \"foo.txt\".*" "Show parameter on"
> -gdb_py_test_silent_cmd "set test-file-param bar.txt" "Turn off parameter" 1
> -gdb_test "show test-file-param" "The name of the file is \"bar.txt\".*" "Show parameter on"
> -gdb_test "python print test_file_param.value" "bar.txt" "Test parameter value"
> -gdb_test "set test-file-param" "Argument required.*" 
> +gdb_test "show print test-undoc-param" "The state of the Test Parameter is on.*" "Show parameter on"
> +gdb_test "set print test-undoc-param off" "Test Parameter has been set to off" "Turn off parameter"
> +gdb_test "show print test-undoc-param" "The state of the Test Parameter is off.*" "Show parameter off"
> +gdb_test "python print test_undoc_param.value" "False" "Test parameter value"
> +gdb_test "help show print test-undoc-param" "This command is not documented.*" "Test show help"
> +gdb_test "help set print test-undoc-param" "This command is not documented.*" "Test set help"
> +gdb_test "help set print" "set print test-undoc-param -- This command is not documented.*" "Test general help"
> +
> +# Test a parameter that is not documented in any way..
> +gdb_py_test_multiple "Simple gdb booleanparameter" \
> +   "python" "" \
> +   "class TestNodocParam (gdb.Parameter):" "" \
> +   "   def __init__ (self, name):" "" \
> +   "      super (TestNodocParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
> +   "      self.value = True" "" \
> +   "test_nodoc_param = TestNodocParam ('print test-nodoc-param')" ""\
> +   "end"
> +
> +gdb_test "show print test-nodoc-param" "This command is not documented.*" "Show parameter on"
> +gdb_test "set print test-nodoc-param off" "This command is not documented.*" "Turn off parameter"
> +gdb_test "show print test-nodoc-param" "This command is not documented.*.*" "Show parameter off"
> +gdb_test "python print test_nodoc_param.value" "False" "Test parameter value"
> +gdb_test "help show print test-nodoc-param" "This command is not documented.*" "Test show help"
> +gdb_test "help set print test-nodoc-param" "This command is not documented.*" "Test set help"
> +gdb_test "help set print" "set print test-nodoc-param -- This command is not documented.*" "Test general help"
> +
> +# Test deprecated API. Do not use in your own implementations.
> +gdb_py_test_multiple "Simple gdb booleanparameter" \
> +   "python" "" \
> +   "class TestParam (gdb.Parameter):" "" \
> +   "   \"\"\"When enabled, test param does something useful. When disabled, does nothing.\"\"\"" "" \
> +   "   show_doc = \"State of the Test Parameter\"" ""\
> +   "   set_doc = \"Set the state of the Test Parameter\"" "" \
> +   "   def __init__ (self, name):" "" \
> +   "      super (TestParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
> +   "      self.value = True" "" \
> +   "test_param = TestParam ('print test-param')" ""\
> +   "end"
> +
> +gdb_test "python print test_param.value" "True" "Test parameter value"
> +gdb_test "show print test-param" "State of the Test Parameter on.*" "Show parameter on"
> +gdb_test "set print test-param off" "Set the state of the Test Parameter.*" "Turn off parameter"
> +gdb_test "show print test-param" "State of the Test Parameter off.*" "Show parameter off"
> +gdb_test "python print test_param.value" "False" "Test parameter value"
> +gdb_test "help show print test-param" "State of the Test Parameter.*" "Test show help"
> +gdb_test "help set print test-param" "Set the state of the Test Parameter.*" "Test set help"
> +gdb_test "help set print" "set print test-param -- Set the state of the Test Parameter.*" "Test general help"

Re: [patch] Implement set/show callback functions in gdb.Parameter

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> +      PyObject *ds_obj = PyObject_GetAttr (object, attr);
Phil> +
Phil> +      if (ds_obj && gdbpy_is_string (ds_obj))
Phil> +	{
Phil> +	  result = python_string_to_host_string (ds_obj);
Phil> +	  if (result == NULL)
Phil> +	    gdbpy_print_stack ();

You need to decref ds_obj.

Phil> +static char *
Phil> +call_doc_function (PyObject *obj, PyObject *method, PyObject *arg)
Phil> +{
Phil> +  char *data = NULL;
Phil> +  struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
Phil> +  PyObject *result = PyObject_CallMethodObjArgs (obj, method, arg, NULL);
Phil> +
Phil> +  if (! result)
Phil> +    return NULL;

This early return would leak the null cleanup.

It is somewhat weird to use cleanups in "pure Python-facing" code.
Here it is more normal to just use ordinary code, calling decref on the
right paths.

Phil> +static void
Phil> +get_set_value (char *args, int from_tty,
Phil> +	       struct cmd_list_element *c)
Phil> +{
Phil> +  PyObject *obj = (PyObject *) get_cmd_context (c);
Phil> +  char *set_doc_string;
Phil> +  PyObject *set_doc_func = PyString_FromString ("get_set_string");
Phil> +  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
Phil> +					       current_language);

You can't call any Python API, in this case PyString_FromString, before
calling ensure_python_env.

Phil> +  make_cleanup (xfree, set_doc_string);
Phil> +  fprintf_filtered (gdb_stdout,"%s\n", set_doc_string);

Missing space after the first ",".

Phil> +static void
Phil> +get_show_value (struct ui_file *file, int from_tty,
Phil> +		struct cmd_list_element *c,
Phil> +		const char *value)
Phil> +{
Phil> +  PyObject *obj = (PyObject *) get_cmd_context (c);
Phil> +  char *show_doc_string = NULL;
Phil> +  PyObject *show_doc_func = PyString_FromString ("get_show_string");
Phil> +  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
Phil> +					       current_language);

Likewise about the ordering.

Phil> +      PyObject *result;

I think this is unused.

Phil> +      fprintf_filtered (file,"%s\n", show_doc_string);

Missing space.

Phil> +      make_cleanup(xfree, show_doc_string);
Phil> +      fprintf_filtered (file,"%s %s\n", show_doc_string, value);

Missing a space on each line.

Phil> +  switch (parmclass) { case var_boolean:

These lines got joined by mistake.

Phil> +  /* Lookup created parameter, and register Python object against the
Phil> +     parameter context.  Perform this task against both lists.  */
Phil> +  tmp_name = cmd_name;
Phil> +  param = lookup_cmd (&tmp_name, *show_list, "", 0, 1);
Phil> +  if (param)
Phil> +    set_cmd_context (param, self);
Phil> +
Phil> +  tmp_name = cmd_name;
Phil> +  param = lookup_cmd (&tmp_name, *set_list, "", 0, 1);
Phil> +  if (param)
Phil> +    set_cmd_context (param, self);

This is pretty gross.

I don't really understand the logic behind why some add_* functions
return the command object and others do not.  I guess in this case you'd
have to have the code return two command objects, making the resulting
function signature really ridiculous.

No change needed here, IMO, I just wanted to complain.

Tom

Re: [patch] Implement set/show callback functions in gdb.Parameter

[Phil Muldoon]

Thanks for the reviews.  Here is the latest patch.

> Phil> +  /* Lookup created parameter, and register Python object against the
> Phil> +     parameter context.  Perform this task against both lists.  */
> Phil> +  tmp_name = cmd_name;
> Phil> +  param = lookup_cmd (&tmp_name, *show_list, "", 0, 1);
> Phil> +  if (param)
> Phil> +    set_cmd_context (param, self);
> Phil> +
> Phil> +  tmp_name = cmd_name;
> Phil> +  param = lookup_cmd (&tmp_name, *set_list, "", 0, 1);
> Phil> +  if (param)
> Phil> +    set_cmd_context (param, self);
>
> This is pretty gross.
>
> I don't really understand the logic behind why some add_* functions
> return the command object and others do not.  I guess in this case you'd
> have to have the code return two command objects, making the resulting
> function signature really ridiculous.
>
> No change needed here, IMO, I just wanted to complain.

Yeah I agonized over this for a bit.  I could not really see a way
around it without redesigning (at least) the parameter/command helper
functions, and probably a lot more.

Anyway patch attached.  What do you think?

Cheers

Phil

--

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a5eaa72..42fbe11 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22418,6 +22418,22 @@ parameter.  It can be read and assigned to just as any other
 attribute.  @value{GDBN} does validation when assignments are made.
 @end defivar
 
+There are two methods that should be implemented in any
+@code{Parameter} class.  These are:
+
+@defop Operation {parameter} get_set_string self
+@value{GDBN} will call this method when a parameter has been
+invoked via the @code{set} API (for example, @kbd{set foo off}).  The
+@code{value} attribute has already been populated with the new value
+and may be used in output.  This method must return a string. 
+@end defop
+
+@defop Operation {parameter} get_show_string self, svalue
+@value{GDBN} will call this method when the parameter has been
+invoked via the @code{show} API (for example, @kbd{show foo}).   The
+argument @code{svalue} receives the string representation of the
+current value. This method must return a string.
+@end defop
 
 When a new parameter is defined, its type must be specified.  The
 available types are represented by constants defined in the @code{gdb}
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index 644d1e2..eda73e5 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -26,6 +26,8 @@
 #include "gdbcmd.h"
 #include "cli/cli-decode.h"
 #include "completer.h"
+#include "language.h"
+#include "arch-utils.h"
 
 /* Parameter constants and their values.  */
 struct parm_constant
@@ -288,6 +290,158 @@ set_attr (PyObject *obj, PyObject *attr_name, PyObject *val)
   return PyObject_GenericSetAttr (obj, attr_name, val);
 }
 
+/* A helper function which returns a documentation string for an
+   object. */
+
+static char *
+get_doc_string (PyObject *object, PyObject *attr)
+{
+  char *result = NULL;
+
+  if (PyObject_HasAttr (object, attr))
+    {
+      PyObject *ds_obj = PyObject_GetAttr (object, attr);
+
+      if (ds_obj && gdbpy_is_string (ds_obj))
+	{
+	  result = python_string_to_host_string (ds_obj);
+	  if (result == NULL)
+	    gdbpy_print_stack ();	  
+	  Py_DECREF (ds_obj);
+	}
+    }
+  if (! result)
+    result = xstrdup (_("This command is not documented."));
+  return result;
+}
+
+/* Helper function which will execute a METHOD in OBJ passing the
+   argument ARG.  ARG can be NULL.  METHOD should return a Python
+   string.  If this function returns NULL, there has been an error and
+   the appropriate exception set.  */
+static char *
+call_doc_function (PyObject *obj, PyObject *method, PyObject *arg)
+{
+  char *data = NULL;
+  PyObject *result = PyObject_CallMethodObjArgs (obj, method, arg, NULL);
+
+  if (! result)
+    return NULL;
+
+  if (gdbpy_is_string (result))
+    {
+      data = python_string_to_host_string (result);
+      if (! data)
+	return NULL;
+    }
+  else
+    {
+      PyErr_SetString (PyExc_RuntimeError,
+		       _("Parameter must return a string value."));
+      return NULL;
+    }
+
+  return data;
+}
+
+/* A callback function that is registered against the respective
+   add_setshow_* set_doc prototype.  This function will either call
+   the Python function "get_set_string" or extract the Python
+   attribute "set_doc" and return the contents as a string.  If
+   neither exist, insert a string indicating the Parameter is not
+   documented.  */
+static void
+get_set_value (char *args, int from_tty,
+	       struct cmd_list_element *c)
+{
+  PyObject *obj = (PyObject *) get_cmd_context (c);
+  char *set_doc_string;
+  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
+					       current_language);  
+  PyObject *set_doc_func = PyString_FromString ("get_set_string");
+
+  make_cleanup_py_decref (set_doc_func);
+
+  if (PyObject_HasAttr (obj, set_doc_func))
+    {
+      set_doc_string = call_doc_function (obj, set_doc_func, NULL);
+      if (! set_doc_string)
+	goto error;
+    }
+  else
+    {
+      /* We have to preserve the existing < GDB 7.3 API.  If a
+	 callback function does not exist, then attempt to read the
+	 set_doc attribute.  */
+      set_doc_string  = get_doc_string (obj, set_doc_cst);
+    }
+
+  make_cleanup (xfree, set_doc_string);
+  fprintf_filtered (gdb_stdout, "%s\n", set_doc_string);
+
+  do_cleanups (cleanup);
+  return;
+
+ error:
+  gdbpy_print_stack ();
+  do_cleanups (cleanup);
+  return;
+}
+
+/* A callback function that is registered against the respective
+   add_setshow_* show_doc prototype.  This function will either call
+   the Python function "get_show_string" or extract the Python
+   attribute "show_doc" and return the contents as a string.  If
+   neither exist, insert a string indicating the Parameter is not
+   documented.  */
+static void
+get_show_value (struct ui_file *file, int from_tty,
+		struct cmd_list_element *c,
+		const char *value)
+{
+  PyObject *obj = (PyObject *) get_cmd_context (c);
+  char *show_doc_string = NULL;
+  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
+					       current_language);  
+  PyObject *show_doc_func = PyString_FromString ("get_show_string");
+  
+  make_cleanup_py_decref (show_doc_func);
+
+  if (PyObject_HasAttr (obj, show_doc_func))
+    {
+      PyObject *val_obj = PyString_FromString (value);
+
+      if (! val_obj)
+	  goto error;
+
+      make_cleanup_py_decref (val_obj);
+
+      show_doc_string = call_doc_function (obj, show_doc_func, val_obj);
+      if (! show_doc_string)
+	goto error;
+
+      make_cleanup (xfree, show_doc_string);
+
+      fprintf_filtered (file, "%s\n", show_doc_string);
+    }
+  else
+    {
+      /* We have to preserve the existing < GDB 7.3 API.  If a
+	 callback function does not exist, then attempt to read the
+	 show_doc attribute.  */
+      show_doc_string  = get_doc_string (obj, show_doc_cst);
+      make_cleanup (xfree, show_doc_string);
+      fprintf_filtered (file, "%s %s\n", show_doc_string, value);
+    }
+
+  do_cleanups (cleanup);
+  return;
+
+ error:
+  gdbpy_print_stack ();
+  do_cleanups (cleanup);
+  return;
+}
 \f
 
 /* A helper function that dispatches to the appropriate add_setshow
@@ -299,74 +453,98 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
 		     struct cmd_list_element **set_list,
 		     struct cmd_list_element **show_list)
 {
-  switch (parmclass)
-    {
+  struct cmd_list_element *param = NULL;
+  char *tmp_name = NULL;
+
+  switch (parmclass) 
+    { 
     case var_boolean:
-      add_setshow_boolean_cmd (cmd_name, cmdclass, &self->value.intval,
-			       set_doc, show_doc, help_doc,
-			       NULL, NULL, set_list, show_list);
+
+      add_setshow_boolean_cmd (cmd_name, cmdclass,
+			       &self->value.intval, set_doc, show_doc,
+			       help_doc, get_set_value, get_show_value,
+			       set_list, show_list);
+
       break;
 
     case var_auto_boolean:
       add_setshow_auto_boolean_cmd (cmd_name, cmdclass,
 				    &self->value.autoboolval,
 				    set_doc, show_doc, help_doc,
-				    NULL, NULL, set_list, show_list);
+				    get_set_value, get_show_value,
+				    set_list, show_list);
       break;
 
     case var_uinteger:
-      add_setshow_uinteger_cmd (cmd_name, cmdclass, &self->value.uintval,
-				set_doc, show_doc, help_doc,
-				NULL, NULL, set_list, show_list);
+      add_setshow_uinteger_cmd (cmd_name, cmdclass,
+				&self->value.uintval, set_doc, show_doc,
+				help_doc, get_set_value, get_show_value,
+				set_list, show_list);
       break;
 
     case var_integer:
-      add_setshow_integer_cmd (cmd_name, cmdclass, &self->value.intval,
-			       set_doc, show_doc, help_doc,
-			       NULL, NULL, set_list, show_list);
-      break;
+      add_setshow_integer_cmd (cmd_name, cmdclass,
+			       &self->value.intval, set_doc, show_doc,
+			       help_doc, get_set_value, get_show_value,
+			       set_list, show_list); break;
 
     case var_string:
-      add_setshow_string_cmd (cmd_name, cmdclass, &self->value.stringval,
-			      set_doc, show_doc, help_doc,
-			      NULL, NULL, set_list, show_list);
-      break;
+      add_setshow_string_cmd (cmd_name, cmdclass,
+			      &self->value.stringval, set_doc, show_doc,
+			      help_doc, get_set_value, get_show_value,
+			      set_list, show_list); break;
 
     case var_string_noescape:
       add_setshow_string_noescape_cmd (cmd_name, cmdclass,
 				       &self->value.stringval,
 				       set_doc, show_doc, help_doc,
-				       NULL, NULL, set_list, show_list);
+				       get_set_value, get_show_value,
+				       set_list, show_list);
+
       break;
 
     case var_optional_filename:
       add_setshow_optional_filename_cmd (cmd_name, cmdclass,
-					 &self->value.stringval,
-					 set_doc, show_doc, help_doc,
-					 NULL, NULL, set_list, show_list);
+					 &self->value.stringval, set_doc,
+					 show_doc, help_doc, get_set_value,
+					 get_show_value, set_list,
+					 show_list);
       break;
 
     case var_filename:
-      add_setshow_filename_cmd (cmd_name, cmdclass, &self->value.stringval,
-				set_doc, show_doc, help_doc,
-				NULL, NULL, set_list, show_list);
-      break;
+      add_setshow_filename_cmd (cmd_name, cmdclass,
+				&self->value.stringval, set_doc, show_doc,
+				help_doc, get_set_value, get_show_value,
+				set_list, show_list); break;
 
     case var_zinteger:
-      add_setshow_zinteger_cmd (cmd_name, cmdclass, &self->value.intval,
-				set_doc, show_doc, help_doc,
-				NULL, NULL, set_list, show_list);
+      add_setshow_zinteger_cmd (cmd_name, cmdclass,
+				&self->value.intval, set_doc, show_doc,
+				help_doc, get_set_value, get_show_value,
+				set_list, show_list);
       break;
 
     case var_enum:
       add_setshow_enum_cmd (cmd_name, cmdclass, self->enumeration,
-			    &self->value.cstringval,
-			    set_doc, show_doc, help_doc,
-			    NULL, NULL, set_list, show_list);
+			    &self->value.cstringval, set_doc, show_doc,
+			    help_doc, get_set_value, get_show_value,
+			    set_list, show_list);
       /* Initialize the value, just in case.  */
       self->value.cstringval = self->enumeration[0];
       break;
     }
+
+  /* Lookup created parameter, and register Python object against the
+     parameter context.  Perform this task against both lists.  */
+  tmp_name = cmd_name;
+  param = lookup_cmd (&tmp_name, *show_list, "", 0, 1);
+  if (param)
+    set_cmd_context (param, self);
+
+  tmp_name = cmd_name;
+  param = lookup_cmd (&tmp_name, *set_list, "", 0, 1);
+  if (param)
+    set_cmd_context (param, self);
 }
 
 /* A helper which computes enum values.  Returns 1 on success.  Returns 0 on
@@ -434,29 +612,6 @@ compute_enum_values (parmpy_object *self, PyObject *enum_values)
   return 1;
 }
 
-/* A helper function which returns a documentation string for an
-   object.  */
-static char *
-get_doc_string (PyObject *object, PyObject *attr)
-{
-  char *result = NULL;
-
-  if (PyObject_HasAttr (object, attr))
-    {
-      PyObject *ds_obj = PyObject_GetAttr (object, attr);
-
-      if (ds_obj && gdbpy_is_string (ds_obj))
-	{
-	  result = python_string_to_host_string (ds_obj);
-	  if (result == NULL)
-	    gdbpy_print_stack ();
-	}
-    }
-  if (! result)
-    result = xstrdup (_("This command is not documented."));
-  return result;
-}
-
 /* Object initializer; sets up gdb-side structures for command.
 
    Use: __init__(NAME, CMDCLASS, PARMCLASS, [ENUM])
diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
index 261c0d5..63f4ff2 100644
--- a/gdb/testsuite/gdb.python/py-parameter.exp
+++ b/gdb/testsuite/gdb.python/py-parameter.exp
@@ -38,8 +38,15 @@ gdb_py_test_multiple "Simple gdb booleanparameter" \
    "python" "" \
    "class TestParam (gdb.Parameter):" "" \
    "   \"\"\"When enabled, test param does something useful. When disabled, does nothing.\"\"\"" "" \
-   "   show_doc = \"Show whether the state of the Test Parameter does something useful\"" ""\
-   "   set_doc = \"Set whether the state of the Test Parameter does something useful\"" "" \
+   "   show_doc = \"Show the state of the boolean test-param\"" ""\
+   "   set_doc = \"Set the state of the boolean test-param\"" "" \
+   "   def get_show_string (self, pvalue):" ""\
+   "      return \"The state of the Test Parameter is \" + pvalue" ""\
+   "   def get_set_string (self):" ""\
+   "      val = \"on\"" ""\
+   "      if (self.value == False):" ""\
+   "         val = \"off\"" ""\
+   "      return \"Test Parameter has been set to \" + val" ""\
    "   def __init__ (self, name):" "" \
    "      super (TestParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
    "      self.value = True" "" \
@@ -47,13 +54,14 @@ gdb_py_test_multiple "Simple gdb booleanparameter" \
    "end"
 
 gdb_test "python print test_param.value" "True" "Test parameter value"
-gdb_test "show print test-param" "Whether the state of the Test Parameter does something useful is on.*" "Show parameter on"
-gdb_py_test_silent_cmd "set print test-param off" "Turn off parameter" 1
-gdb_test "show print test-param" "Whether the state of the Test Parameter does something useful is off.*" "Show parameter off"
+gdb_test "show print test-param" "The state of the Test Parameter is on.*" "Show parameter on"
+gdb_test "set print test-param off" "Test Parameter has been set to off" "Turn off parameter"
+gdb_test "show print test-param" "The state of the Test Parameter is off.*" "Show parameter off"
 gdb_test "python print test_param.value" "False" "Test parameter value"
-gdb_test "help show print test-param" "Show whether the state of the Test Parameter does something useful.*" "Test show help"
-gdb_test "help set print test-param" "Set whether the state of the Test Parameter does something useful.*" "Test set help"
-gdb_test "help set print" "set print test-param -- Set whether the state of the Test Parameter.*" "Test general help"
+gdb_test "help show print test-param" "Show the state of the boolean test-param.*" "Test show help"
+gdb_test "help set print test-param" "Set the state of the boolean test-param.*" "Test set help"
+gdb_test "help set print" "set print test-param -- Set the state of the boolean test-param.*" "Test general help"
+
 
 # Test an enum parameter.
 gdb_py_test_multiple "enum gdb parameter" \
@@ -62,6 +70,10 @@ gdb_py_test_multiple "enum gdb parameter" \
    "   \"\"\"When set, test param does something useful. When disabled, does nothing.\"\"\"" "" \
    "   show_doc = \"Show the state of the enum\"" ""\
    "   set_doc = \"Set the state of the enum\"" "" \
+   "   def get_show_string (self, pvalue):" ""\
+   "      return \"The state of the enum is \" + pvalue" ""\
+   "   def get_set_string (self):" ""\
+   "      return \"The state of the enum has been set to \" + self.value" ""\
    "   def __init__ (self, name):" "" \
    "      super (TestEnumParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_ENUM, \[\"one\", \"two\"\])" "" \
    "      self.value = \"one\"" "" \
@@ -69,9 +81,9 @@ gdb_py_test_multiple "enum gdb parameter" \
    "end"
 
 gdb_test "python print test_enum_param.value" "one" "Test enum parameter value"
-gdb_test "show print test-enum-param" "The state of the enum is \"one\".*" "Show parameter is initial value"
-gdb_py_test_silent_cmd "set print test-enum-param two" "Set parameter to enum value" 1
-gdb_test "show print test-enum-param" "The state of the enum is \"two\".*" "Show parameter is new value"
+gdb_test "show print test-enum-param" "The state of the enum is one.*" "Show parameter is initial value"
+gdb_test "set print test-enum-param two" "The state of the enum has been set to two" "Set enum to two"
+gdb_test "show print test-enum-param" "The state of the enum is two.*" "Show parameter is new value"
 gdb_test "python print test_enum_param.value" "two" "Test enum parameter value"
 gdb_test "set print test-enum-param three" "Undefined item: \"three\".*" "Set invalid enum parameter" 
 
@@ -82,6 +94,10 @@ gdb_py_test_multiple "file gdb parameter" \
    "   \"\"\"When set, test param does something useful. When disabled, does nothing.\"\"\"" "" \
    "   show_doc = \"Show the name of the file\"" ""\
    "   set_doc = \"Set the name of the file\"" "" \
+   "   def get_show_string (self, pvalue):" ""\
+   "      return \"The name of the file is \" + pvalue" ""\
+   "   def get_set_string (self):" ""\
+   "      return \"The name of the file has been changed to \" + self.value" ""\
    "   def __init__ (self, name):" "" \
    "      super (TestFileParam, self).__init__ (name, gdb.COMMAND_FILES, gdb.PARAM_FILENAME)" "" \
    "      self.value = \"foo.txt\"" "" \
@@ -89,28 +105,73 @@ gdb_py_test_multiple "file gdb parameter" \
    "end"
 
 gdb_test "python print test_file_param.value" "foo.txt" "Test file parameter value"
-gdb_test "show test-file-param" "The name of the file is \"foo.txt\".*" "Show initial file value"
-gdb_py_test_silent_cmd "set test-file-param bar.txt" "Set new file parameter" 1
-gdb_test "show test-file-param" "The name of the file is \"bar.txt\".*" "Show new file value"
+gdb_test "show test-file-param" "The name of the file is foo.txt.*" "Show initial file value"
+gdb_test "set test-file-param bar.txt" "The name of the file has been changed to bar.txt" "Set new file parameter" 1
+gdb_test "show test-file-param" "The name of the file is bar.txt.*" "Show new file value"
 gdb_test "python print test_file_param.value" "bar.txt" "Test new file parameter value"
 gdb_test "set test-file-param" "Argument required.*" 
 
-# Test a file parameter.
-gdb_py_test_multiple "file gdb parameter" \
+# Test a parameter that is not documented.
+gdb_py_test_multiple "Simple gdb booleanparameter" \
    "python" "" \
-   "class TestFileParam (gdb.Parameter):" "" \
-   "   \"\"\"When set, test param does something useful. When disabled, does nothing.\"\"\"" "" \
-   "   show_doc = \"Show the name of the file\"" ""\
-   "   set_doc = \"Set the name of the file\"" "" \
+   "class TestUndocParam (gdb.Parameter):" "" \
+   "   def get_show_string (self, pvalue):" ""\
+   "      return \"The state of the Test Parameter is \" + pvalue" ""\
+   "   def get_set_string (self):" ""\
+   "      val = \"on\"" ""\
+   "      if (self.value == False):" ""\
+   "         val = \"off\"" ""\
+   "      return \"Test Parameter has been set to \" + val" ""\
    "   def __init__ (self, name):" "" \
-   "      super (TestFileParam, self).__init__ (name, gdb.COMMAND_FILES, gdb.PARAM_FILENAME)" "" \
-   "      self.value = \"foo.txt\"" "" \
-   "test_file_param = TestFileParam ('test-file-param')" ""\
+   "      super (TestUndocParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
+   "      self.value = True" "" \
+   "test_undoc_param = TestUndocParam ('print test-undoc-param')" ""\
    "end"
 
-gdb_test "python print test_file_param.value" "foo.txt" "Test parameter value"
-gdb_test "show test-file-param" "The name of the file is \"foo.txt\".*" "Show parameter on"
-gdb_py_test_silent_cmd "set test-file-param bar.txt" "Turn off parameter" 1
-gdb_test "show test-file-param" "The name of the file is \"bar.txt\".*" "Show parameter on"
-gdb_test "python print test_file_param.value" "bar.txt" "Test parameter value"
-gdb_test "set test-file-param" "Argument required.*" 
+gdb_test "show print test-undoc-param" "The state of the Test Parameter is on.*" "Show parameter on"
+gdb_test "set print test-undoc-param off" "Test Parameter has been set to off" "Turn off parameter"
+gdb_test "show print test-undoc-param" "The state of the Test Parameter is off.*" "Show parameter off"
+gdb_test "python print test_undoc_param.value" "False" "Test parameter value"
+gdb_test "help show print test-undoc-param" "This command is not documented.*" "Test show help"
+gdb_test "help set print test-undoc-param" "This command is not documented.*" "Test set help"
+gdb_test "help set print" "set print test-undoc-param -- This command is not documented.*" "Test general help"
+
+# Test a parameter that is not documented in any way..
+gdb_py_test_multiple "Simple gdb booleanparameter" \
+   "python" "" \
+   "class TestNodocParam (gdb.Parameter):" "" \
+   "   def __init__ (self, name):" "" \
+   "      super (TestNodocParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
+   "      self.value = True" "" \
+   "test_nodoc_param = TestNodocParam ('print test-nodoc-param')" ""\
+   "end"
+
+gdb_test "show print test-nodoc-param" "This command is not documented.*" "Show parameter on"
+gdb_test "set print test-nodoc-param off" "This command is not documented.*" "Turn off parameter"
+gdb_test "show print test-nodoc-param" "This command is not documented.*.*" "Show parameter off"
+gdb_test "python print test_nodoc_param.value" "False" "Test parameter value"
+gdb_test "help show print test-nodoc-param" "This command is not documented.*" "Test show help"
+gdb_test "help set print test-nodoc-param" "This command is not documented.*" "Test set help"
+gdb_test "help set print" "set print test-nodoc-param -- This command is not documented.*" "Test general help"
+
+# Test deprecated API. Do not use in your own implementations.
+gdb_py_test_multiple "Simple gdb booleanparameter" \
+   "python" "" \
+   "class TestParam (gdb.Parameter):" "" \
+   "   \"\"\"When enabled, test param does something useful. When disabled, does nothing.\"\"\"" "" \
+   "   show_doc = \"State of the Test Parameter\"" ""\
+   "   set_doc = \"Set the state of the Test Parameter\"" "" \
+   "   def __init__ (self, name):" "" \
+   "      super (TestParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)" "" \
+   "      self.value = True" "" \
+   "test_param = TestParam ('print test-param')" ""\
+   "end"
+
+gdb_test "python print test_param.value" "True" "Test parameter value"
+gdb_test "show print test-param" "State of the Test Parameter on.*" "Show parameter on"
+gdb_test "set print test-param off" "Set the state of the Test Parameter.*" "Turn off parameter"
+gdb_test "show print test-param" "State of the Test Parameter off.*" "Show parameter off"
+gdb_test "python print test_param.value" "False" "Test parameter value"
+gdb_test "help show print test-param" "State of the Test Parameter.*" "Test show help"
+gdb_test "help set print test-param" "Set the state of the Test Parameter.*" "Test set help"
+gdb_test "help set print" "set print test-param -- Set the state of the Test Parameter.*" "Test general help"

Re: [patch] Implement set/show callback functions in gdb.Parameter

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> Anyway patch attached.  What do you think?

Very close now.

Phil> +  PyObject *set_doc_func = PyString_FromString ("get_set_string");
Phil> +
Phil> +  make_cleanup_py_decref (set_doc_func);

The PyString_FromString call can fail.
So, you need a check here.

Phil> +  PyObject *show_doc_func = PyString_FromString ("get_show_string");
Phil> +  
Phil> +  make_cleanup_py_decref (show_doc_func);

Likewise.

This is ok with those fixed.

Tom

Re: [patch] Implement set/show callback functions in gdb.Parameter

[Eli Zaretskii]

> From: Phil Muldoon <pmuldoon@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Mon, 07 Mar 2011 15:58:22 +0000
> 
> 
> +@defop Operation {parameter} get_set_string self
> +@value{GDBN} will call this method when a parameter has been
> +invoked via the @code{set} API (for example, @kbd{set foo off}).  The

Calling an operation a "parameter" is not the best idea.  Already you
need to talk about "invoking a parameter", which sounds awkward.  Can
we find a better word here?

> +@defop Operation {parameter} get_show_string self, svalue

Is the comma really necessary here?

Otherwise, okay.

Thanks.

Re: [patch] Implement set/show callback functions in gdb.Parameter

[Phil Muldoon]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Phil Muldoon <pmuldoon@redhat.com>
>> Cc: gdb-patches@sourceware.org
>> Date: Mon, 07 Mar 2011 15:58:22 +0000
>> 
>> 
>> +@defop Operation {parameter} get_set_string self
>> +@value{GDBN} will call this method when a parameter has been
>> +invoked via the @code{set} API (for example, @kbd{set foo off}).  The
>
> Calling an operation a "parameter" is not the best idea.  Already you
> need to talk about "invoking a parameter", which sounds awkward.  Can
> we find a better word here?


Well this is an operation in a gdb.Parameter.  Not sure what else to
call it.  What do you think?

>> +@defop Operation {parameter} get_show_string self, svalue
>
> Is the comma really necessary here?

Nope, deleted. 

Thanks

Phil

Re: [patch] Implement set/show callback functions in gdb.Parameter

[Eli Zaretskii]

> From: Phil Muldoon <pmuldoon@redhat.com>
> Cc: tromey@redhat.com, gdb-patches@sourceware.org
> Date: Mon, 07 Mar 2011 20:37:06 +0000
> 
> >> +@defop Operation {parameter} get_set_string self
> >> +@value{GDBN} will call this method when a parameter has been
> >> +invoked via the @code{set} API (for example, @kbd{set foo off}).  The
> >
> > Calling an operation a "parameter" is not the best idea.  Already you
> > need to talk about "invoking a parameter", which sounds awkward.  Can
> > we find a better word here?
> 
> 
> Well this is an operation in a gdb.Parameter.  Not sure what else to
> call it.  What do you think?

Then how about rephrasing the description like this:

  @value{GDBN} will call this method when @var{parameter}'s value has
  been changed via the @code{set} API (for example, @kbd{set foo off}).

Would this be accurate?

For the get_show_string, I would suggest

  @value{GDBN} will call this method when @var{parameter}'s
  @code{show} API has been invoked (for example, @kbd{show foo}).

WDYT?

Re: [patch] Implement set/show callback functions in gdb.Parameter

[Phil Muldoon]

Eli Zaretskii <eliz@gnu.org> writes:


>> > Calling an operation a "parameter" is not the best idea.  Already you
>> > need to talk about "invoking a parameter", which sounds awkward.  Can
>> > we find a better word here?
>> 
>> 
>> Well this is an operation in a gdb.Parameter.  Not sure what else to
>> call it.  What do you think?
>
> Then how about rephrasing the description like this:
>
>   @value{GDBN} will call this method when @var{parameter}'s value has
>   been changed via the @code{set} API (for example, @kbd{set foo off}).
>
> Would this be accurate?
>
> For the get_show_string, I would suggest
>
>   @value{GDBN} will call this method when @var{parameter}'s
>   @code{show} API has been invoked (for example, @kbd{show foo}).
>
> WDYT?

I think it is fine.  I'll add the changes that both you and Tom want,
and commit today.

Thanks

Cheers

Phil

Re: [patch] Implement set/show callback functions in gdb.Parameter

[Phil Muldoon]

Phil Muldoon <pmuldoon@redhat.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:

>> WDYT?
>
> I think it is fine.  I'll add the changes that both you and Tom want,
> and commit today.

So committed (last week),

Cheers

Phil