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

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

[Andrew Burgess]

I'm aware of Tom's currently inflight series that also relates to
gdb.Parameter[1], but this patch doesn't conflict.

[1] https://sourceware.org/pipermail/gdb-patches/2022-January/184826.html

---

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.
---
 gdb/cli/cli-decode.c                      | 14 ++++
 gdb/cli/cli-decode.h                      |  6 ++
 gdb/doc/python.texi                       | 41 ++++++++---
 gdb/python/py-param.c                     | 84 ++++++++++++++++++++---
 gdb/testsuite/gdb.python/py-parameter.exp | 36 +++++++---
 5 files changed, 151 insertions(+), 30 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 7266fb36d0d..af6828620eb 100644
--- 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 8f367bf4634..085c92de5bf 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -79,6 +79,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 6bd5f6b90ac..ff0103db8e3 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4136,23 +4136,46 @@
 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 592ecbb41f9..3e1e0615b4d 100644
--- 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 b025c2562f0..1c84db7b78b 100644
--- 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" \
-- 
2.25.4

Re: [PATCH] gdb/python: improve the auto help text for gdb.Parameter

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

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

Thanks for the comprehensive explanation.  This looks good to me.

Tom

Re: [PATCH] gdb/python: improve the auto help text for gdb.Parameter

[Andrew Burgess]

Eli,

Do you have any docs feedback on this?

Thanks,
Andrew

* Andrew Burgess <aburgess@redhat.com> [2022-01-10 12:45:58 +0000]:

> I'm aware of Tom's currently inflight series that also relates to
> gdb.Parameter[1], but this patch doesn't conflict.
> 
> [1] https://sourceware.org/pipermail/gdb-patches/2022-January/184826.html
> 
> ---
> 
> 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.
> ---
>  gdb/cli/cli-decode.c                      | 14 ++++
>  gdb/cli/cli-decode.h                      |  6 ++
>  gdb/doc/python.texi                       | 41 ++++++++---
>  gdb/python/py-param.c                     | 84 ++++++++++++++++++++---
>  gdb/testsuite/gdb.python/py-parameter.exp | 36 +++++++---
>  5 files changed, 151 insertions(+), 30 deletions(-)
> 
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index 7266fb36d0d..af6828620eb 100644
> --- 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 8f367bf4634..085c92de5bf 100644
> --- a/gdb/cli/cli-decode.h
> +++ b/gdb/cli/cli-decode.h
> @@ -79,6 +79,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 6bd5f6b90ac..ff0103db8e3 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -4136,23 +4136,46 @@
>  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 592ecbb41f9..3e1e0615b4d 100644
> --- 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 b025c2562f0..1c84db7b78b 100644
> --- 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" \
> -- 
> 2.25.4
> 

Re: [PATCH] gdb/python: improve the auto help text for gdb.Parameter

[Eli Zaretskii]

> Date: Tue, 25 Jan 2022 17:50:18 +0000
> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> Eli,
> 
> Do you have any docs feedback on this?

Sorry for missing this.  The patch is okay.

Re: [PATCH] gdb/python: improve the auto help text for gdb.Parameter

[Andrew Burgess]

Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Date: Tue, 25 Jan 2022 17:50:18 +0000
>> From: Andrew Burgess <aburgess@redhat.com>
>> Cc: gdb-patches@sourceware.org
>> 
>> Eli,
>> 
>> Do you have any docs feedback on this?
>
> Sorry for missing this.  The patch is okay.

No problem.

I've now pushed this patch.

Thanks,
Andrew