gdb/python: improve the auto help text for gdb.Parameter

This commit attempts to improve the help text that is generated for
gdb.Parameter objects when the user fails to provide their own
documentation.

Documentation for a gdb.Parameter is currently pulled from two
sources: the class documentation string, and the set_doc/show_doc
class attributes.  Thus, a fully documented parameter might look like
this:

  class Param_All (gdb.Parameter):
     """This is the class documentation string."""

     show_doc = "Show the state of this parameter"
     set_doc = "Set the state of this parameter"

     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 (Param_All, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)
        self._value = True

  Param_All ('param-all')

Then in GDB we see this:

  (gdb) help set param-all
  Set the state of this parameter
  This is the class documentation string.

Which is fine.  But, if the user skips both of the documentation parts
like this:

  class Param_None (gdb.Parameter):

     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 (Param_None, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)
        self._value = True

  Param_None ('param-none')

Now in GDB we see this:

  (gdb) help set param-none
  This command is not documented.
  This command is not documented.

That's not great, the duplicated text looks a bit weird.  If we drop
different parts we get different results.  Here's what we get if the
user drops the set_doc and show_doc attributes:

  (gdb) help set param-doc
  This command is not documented.
  This is the class documentation string.

That kind of sucks, we say it's undocumented, then proceed to print
the documentation.  Finally, if we drop the class documentation but
keep the set_doc and show_doc:

  (gdb) help set param-set-show
  Set the state of this parameter
  This command is not documented.

That seems OK.

So, I think there's room for improvement.

With this patch, for the four cases above we now see this:

  # All values provided by the user, no change in this case:
  (gdb) help set param-all
  Set the state of this parameter
  This is the class documentation string.

  # Nothing provided by the user, the first string is now different:
  (gdb) help set param-none
  Set the current value of 'param-none'.
  This command is not documented.

  # Only the class documentation is provided, the first string is
  # changed as in the previous case:
  (gdb) help set param-doc
  Set the current value of 'param-doc'.
  This is the class documentation string.

  # Only the set_doc and show_doc are provided, this case is unchanged
  # from before the patch:
  (gdb) help set param-set-show
  Set the state of this parameter
  This command is not documented.

The one place where this change might be considered a negative is when
dealing with prefix commands.  If we create a prefix command but don't
supply the set_doc / show_doc strings, then this is what we saw before
my patch:

  (gdb) python Param_None ('print param-none')
  (gdb) help set print
  set print, set pr, set p
  Generic command for setting how things print.

  List of set print subcommands:

  ... snip ...
  set print param-none -- This command is not documented.
  ... snip ...

And after my patch:

  (gdb) python Param_None ('print param-none')
  (gdb) help set print
  set print, set pr, set p
  Generic command for setting how things print.

  List of set print subcommands:

  ... snip ...
  set print param-none -- Set the current value of 'print param-none'.
  ... snip ...

This seems slightly less helpful than before, but I don't think its
terrible.

Additionally, I've changed what we print when the get_show_string
method is not provided in Python.

Back when gdb.Parameter was first added to GDB, we didn't provide a
show function when registering the internal command object within
GDB.  As a result, GDB would make use of its "magic" mangling of the
show_doc string to create a sentence that would display the current
value (see deprecated_show_value_hack in cli/cli-setshow.c).

However, when we added support for the get_show_string method to
gdb.Parameter, there was an attempt to maintain backward compatibility
by displaying the show_doc string with the current value appended, see
get_show_value in py-param.c.  Unfortunately, this isn't anywhere
close to what deprecated_show_value_hack does, and the results are
pretty poor, for example, this is GDB before my patch:

  (gdb) show param-none
  This command is not documented. off

I think we can all agree that this is pretty bad.

After my patch, we how show this:

  (gdb) show param-none
  The current value of 'param-none' is "off".

Which at least is a real sentence, even if it's not very informative.

This patch does change the way that the Python API behaves slightly,
but only in the cases when the user has missed providing GDB with some
information.  In most cases I think the new behaviour is a lot better,
there's the one case (noted above) which is a bit iffy, but I think is
still OK.

I've updated the existing gdb.python/py-parameter.exp test to cover
the modified behaviour.

Finally, I've updated the documentation to (I hope) make it clearer
how the various bits of help text come together.

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 5d9403068e7b94bcd2a7e7d658ae71ea91391f79..a71350ba68b2c0a22b5ad6fe97d105f41d9deca7 100644 (file)
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -145,6 +145,20 @@ cmd_list_element::prefixname () const
   return prefixname;
 }
 
+/* See cli/cli-decode.h.  */
+
+std::vector<std::string>
+cmd_list_element::command_components () const
+{
+  std::vector<std::string> result;
+
+  if (this->prefix != nullptr)
+    result = this->prefix->command_components ();
+
+  result.emplace_back (std::string (this->name));
+  return result;
+}
+
 /* Add element named NAME.
    Space for NAME and DOC must be allocated by the caller.
    CLASS is the top level category into which commands are broken down

diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 658142d338da49f05b760794a23032b4828307b9..db35ea4982bfadd19ba7ffe3051859c1d4db1d0b 100644 (file)
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -80,6 +80,12 @@ struct cmd_list_element
      For non-prefix commands, return an empty string.  */
   std::string prefixname () const;
 
+  /* Return a vector of strings describing the components of the full name
+     of this command.  For example, if this command is 'set AA BB CC',
+     then the vector will contain 4 elements 'set', 'AA', 'BB', and 'CC'
+     in that order.  */
+  std::vector<std::string> command_components () const;
+
   /* Return true if this command is an alias of another command.  */
   bool is_alias () const
   { return this->alias_target != nullptr; }

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index f1d182729634d9fda821fa83c1eb452fdf858158..da88b8a7690f85a6ac9fa9f07763d9c98c693d76 100644 (file)
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4175,23 +4175,46 @@ represent the possible values for the parameter.
 If @var{parameter-class} is not @code{PARAM_ENUM}, then the presence
 of a fourth argument will cause an exception to be thrown.
 
-The help text for the new parameter is taken from the Python
-documentation string for the parameter's class, if there is one.  If
-there is no documentation string, a default value is used.
+The help text for the new parameter includes the Python documentation
+string from the parameter's class, if there is one.  If there is no
+documentation string, a default value is used.  The documentation
+string is included in the output of the parameters @code{help set} and
+@code{help show} commands, and should be written taking this into
+account.
 @end defun
 
 @defvar Parameter.set_doc
 If this attribute exists, and is a string, then its value is used as
-the help text for this parameter's @code{set} command.  The value is
-examined when @code{Parameter.__init__} is invoked; subsequent changes
-have no effect.
+the first part of the help text for this parameter's @code{set}
+command.  The second part of the help text is taken from the
+documentation string for the parameter's class, if there is one.
+
+The value of @code{set_doc} should give a brief summary specific to
+the set action, this text is only displayed when the user runs the
+@code{help set} command for this parameter.  The class documentation
+should be used to give a fuller description of what the parameter
+does, this text is displayed for both the @code{help set} and
+@code{help show} commands.
+
+The @code{set_doc} value is examined when @code{Parameter.__init__} is
+invoked; subsequent changes have no effect.
 @end defvar
 
 @defvar Parameter.show_doc
 If this attribute exists, and is a string, then its value is used as
-the help text for this parameter's @code{show} command.  The value is
-examined when @code{Parameter.__init__} is invoked; subsequent changes
-have no effect.
+the first part of the help text for this parameter's @code{show}
+command.  The second part of the help text is taken from the
+documentation string for the parameter's class, if there is one.
+
+The value of @code{show_doc} should give a brief summary specific to
+the show action, this text is only displayed when the user runs the
+@code{help show} command for this parameter.  The class documentation
+should be used to give a fuller description of what the parameter
+does, this text is displayed for both the @code{help set} and
+@code{help show} commands.
+
+The @code{show_doc} value is examined when @code{Parameter.__init__}
+is invoked; subsequent changes have no effect.
 @end defvar
 
 @defvar Parameter.value

diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index f5f0eca6d1a55d63bd54400dfb8b1f8f6e6131a8..00f917edb308c9af41b103916eb69d43977c6a79 100644 (file)
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -331,14 +331,59 @@ set_attr (PyObject *obj, PyObject *attr_name, PyObject *val)
   return PyObject_GenericSetAttr (obj, attr_name, val);
 }
 
+/* Build up the path to command C, but drop the first component of the
+   command prefix.  This is only intended for use with the set/show
+   parameters this file deals with, the first prefix should always be
+   either 'set' or 'show'.
+
+   As an example, if this full command is 'set prefix_a prefix_b command'
+   this function will return the string 'prefix_a prefix_b command'.  */
+
+static std::string
+full_cmd_name_without_first_prefix (struct cmd_list_element *c)
+{
+  std::vector<std::string> components
+    = c->command_components ();
+  gdb_assert (components.size () > 1);
+  std::string result = components[1];
+  for (int i = 2; i < components.size (); ++i)
+    result += " " + components[i];
+  return result;
+}
+
+/* The different types of documentation string.  */
+
+enum doc_string_type
+{
+  doc_string_set,
+  doc_string_show,
+  doc_string_description
+};
+
 /* A helper function which returns a documentation string for an
    object. */
 
 static gdb::unique_xmalloc_ptr<char>
-get_doc_string (PyObject *object, PyObject *attr)
+get_doc_string (PyObject *object, enum doc_string_type doc_type,
+               const char *cmd_name)
 {
   gdb::unique_xmalloc_ptr<char> result;
 
+  PyObject *attr = nullptr;
+  switch (doc_type)
+    {
+    case doc_string_set:
+      attr = set_doc_cst;
+      break;
+    case doc_string_show:
+      attr = show_doc_cst;
+      break;
+    case doc_string_description:
+      attr = gdbpy_doc_cst;
+      break;
+    }
+  gdb_assert (attr != nullptr);
+
   if (PyObject_HasAttr (object, attr))
     {
       gdbpy_ref<> ds_obj (PyObject_GetAttr (object, attr));
@@ -350,8 +395,21 @@ get_doc_string (PyObject *object, PyObject *attr)
            gdbpy_print_stack ();
        }
     }
-  if (! result)
-    result.reset (xstrdup (_("This command is not documented.")));
+
+  if (result == nullptr)
+    {
+      if (doc_type == doc_string_description)
+       result.reset (xstrdup (_("This command is not documented.")));
+      else
+       {
+         if (doc_type == doc_string_show)
+           result = xstrprintf (_("Show the current value of '%s'."),
+                                cmd_name);
+         else
+           result = xstrprintf (_("Set the current value of '%s'."),
+                                cmd_name);
+       }
+    }
   return result;
 }
 
@@ -462,11 +520,15 @@ get_show_value (struct ui_file *file, int from_tty,
     }
   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);
-      fprintf_filtered (file, "%s %s\n", show_doc_string.get (), value);
+      /* If there is no 'get_show_string' callback then we want to show
+        something sensible here.  In older versions of GDB (< 7.3) we
+        didn't support 'get_show_string', and instead we just made use of
+        GDB's builtin use of the show_doc.  However, GDB's builtin
+        show_doc adjustment is not i18n friendly, so, instead, we just
+        print this generic string.  */
+      std::string cmd_path = full_cmd_name_without_first_prefix (c);
+      fprintf_filtered (file, _("The current value of '%s' is \"%s\".\n"),
+                               cmd_path.c_str (), value);
     }
 }
 \f
@@ -737,9 +799,9 @@ parmpy_init (PyObject *self, PyObject *args, PyObject *kwds)
   if (cmd_name == nullptr)
     return -1;
 
-  set_doc = get_doc_string (self, set_doc_cst);
-  show_doc = get_doc_string (self, show_doc_cst);
-  doc = get_doc_string (self, gdbpy_doc_cst);
+  set_doc = get_doc_string (self, doc_string_set, name);
+  show_doc = get_doc_string (self, doc_string_show, name);
+  doc = get_doc_string (self, doc_string_description, cmd_name.get ());
 
   Py_INCREF (self);
 

diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
index 2de148c1b27c25d85f25ec08429b7b5f9c3a190b..98d4b2d4684cee56602bffa67a9fc7c934926fb8 100644 (file)
--- a/gdb/testsuite/gdb.python/py-parameter.exp
+++ b/gdb/testsuite/gdb.python/py-parameter.exp
@@ -127,7 +127,10 @@ proc_with_prefix test_boolean_parameter { } {
     gdb_test "python print (test_param.value)" "False" \
        "test boolean parameter value is False"
     gdb_test "help show print test-param" \
-       "Show the state of the boolean test-param.*" "test show help"
+       [multi_line \
+            "Show the state of the boolean test-param" \
+            "When enabled, test param does something useful\\. When disabled, does nothing\\."] \
+       "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" \
@@ -233,11 +236,14 @@ proc_with_prefix test_undocumented_parameter { } {
     gdb_test "python print (test_undoc_param.value)" \
        "False" "test undocumented parameter value is False"
     gdb_test "help show print test-undoc-param" \
-       "This command is not documented.*" "test show help"
+       [multi_line \
+            "Show the current value of '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.*" \
+       "set print test-undoc-param -- Set the current value of 'print test-undoc-param'\\..*" \
        "test general help"
 }
 
@@ -255,19 +261,24 @@ proc_with_prefix test_really_undocumented_parameter { } {
        "end"
 
     gdb_test "show print test-nodoc-param" \
-       "This command is not documented.*" "show parameter on"
+       "The current value of 'print test-nodoc-param' is \"on\"\\." \
+       "show parameter on"
     gdb_test_no_output "set print test-nodoc-param off" \
        "turn off parameter"
     gdb_test "show print test-nodoc-param" \
-       "This command is not documented.*.*" "show parameter off"
+       "The current value of 'print test-nodoc-param' is \"off\"\\." \
+       "show parameter off"
     gdb_test "python print (test_nodoc_param.value)" \
        "False" "test really undocumented parameter value is False"
     gdb_test "help show print test-nodoc-param" \
-       "This command is not documented.*" "test show help"
+       [multi_line \
+            "Show the current value of '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.*" \
+       "set print test-nodoc-param -- Set the current value of 'print test-nodoc-param'\\..*" \
        "test general help"
 }
 
@@ -290,14 +301,19 @@ proc_with_prefix test_deprecated_api_parameter { } {
     gdb_test "python print (test_param.value)" "True" \
        "test deprecated API parameter value is True"
     gdb_test "show print test-param" \
-       "State of the Test Parameter on.*" "show parameter on"
+       "The current value of 'print test-param' is \"on\"\\." \
+       "show parameter on"
     gdb_test_no_output "set print test-param off" "turn off parameter"
     gdb_test "show print test-param" \
-       "State of the Test Parameter off.*" "show parameter off"
+       "The current value of 'print test-param' is \"off\"\\." \
+       "show parameter off"
     gdb_test "python print (test_param.value)" "False" \
        "test deprecated API parameter value is False"
     gdb_test "help show print test-param" \
-       "State of the Test Parameter.*" "test show help"
+       [multi_line \
+            "State of the Test Parameter" \
+            "When enabled, test param does something useful\\. When disabled, does nothing\\."] \
+       "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" \