[1/2] gdb/guile: improve auto-generated strings for parameters

Message ID bec1168ecbe4cdab11e20652faf7cf728301ae86.1744535962.git.aburgess@redhat.com
State New
Headers
Series guile parameter doc string improvements |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed

Commit Message

Andrew Burgess April 13, 2025, 9:23 a.m. UTC
  Consider this user defined parameter created in Python:

  class test_param(gdb.Parameter):
     def __init__(self, name):
        super ().__init__(name, gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)
        self.value = True

  test_param('print test')

If this is loaded into GDB, then we observe the following behaviour:

  (gdb) show print test
  The current value of 'print test' is "on".
  (gdb) help show print test
  Show the current value of 'print test'.
  This command is not documented.
  (gdb) help set print test
  Set the current value of 'print test'.
  This command is not documented.
  (gdb)

If we now define the same parameter using Guile:

  (use-modules (gdb))
  (register-parameter! (make-parameter
                        "print test"
                        #:command-class COMMAND_NONE
                        #:parameter-type PARAM_BOOLEAN))

And load this into a fresh GDB session, we see the following:

  (gdb) show print test
  Command is not documented is off.
  (gdb) help show print test
  This command is not documented.
  (gdb) help set print test
  This command is not documented.
  (gdb)

The output of 'show print test' doesn't make much sense, and is
certainly worse than the Python equivalent.  For both the 'help'
commands it appears as if the first line is missing, but what is
actually happening is that the first line has become 'This command is
not documented.', and the second line is then missing.

The problems can all be traced back to 'get_doc_string' in
guile/scm-param.c.  This is the guile version of this function.  There
is a similar function in python/py-param.c, however, the Python
version returns one of three different strings depending on the use
case.  In contrast, the Guile version just returns 'This command is
not documented.' in all cases.

The three cases that the Python code handles are, the 'set' string,
the 'show' string, and the general 'description' string.

Right now the Guile get_doc_string only returns the general
'description' string, which is funny, because, in
gdbscm_make_parameter, where get_doc_string is used, the one case that
we currently don't need is the general 'description' string.  Instead,
right now, the general 'description' string is used for both the 'set'
and 'show' cases.

In this commit I plan to bring the Guile API a little more inline with
the Python API.  I will update get_doc_string (in scm-param.c) to
return either a 'set' or 'show' string, and gdbscm_make_parameter will
make use of these strings.

The changes to the Guile get_doc_string are modelled on the Python
version of this function.  It is also worth checking out the next
commit, which is related, and helps motivate how the changes have been
implemented in this commit.

After this commit, the same Guile parameter description shown above,
now gives this behaviour:

  (gdb) show print test
  The current value of 'print test' is off.
  (gdb) help show print test
  Show the current value of 'print test'.
  (gdb) help set print test
  Set the current value of 'print test'.
  (gdb)

The 'show print test' output now matches the Python behaviour, and is
much more descriptive.  The set and show 'help' output are now missing
the second line when compared to the Python output, but the first line
is now correct, and I think is better than the previous Guile output.

In the next commit I'll address the problem of the missing second
line.

Existing tests have been updated to expect the new output.
---
 gdb/guile/scm-param.c                     | 28 ++++++++++++++++++-----
 gdb/testsuite/gdb.guile/scm-parameter.exp | 26 ++++++++++++++-------
 2 files changed, 40 insertions(+), 14 deletions(-)
  

Patch

diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
index 65226ec4d10..bc6c605b59c 100644
--- a/gdb/guile/scm-param.c
+++ b/gdb/guile/scm-param.c
@@ -308,13 +308,29 @@  pascm_is_valid (param_smob *p_smob)
   return p_smob->commands.set != nullptr;
 }
 
-/* A helper function which return the default documentation string for
-   a parameter (which is to say that it's undocumented).  */
+
+/* The different types of documentation string.  */
+
+enum doc_string_type
+{
+  doc_string_set,
+  doc_string_show,
+};
+
+/* A helper function which returns the default documentation string for
+   a parameter CMD_NAME.  The DOC_TYPE indicates which type of
+   documentation string is needed.  The returned string is dynamically
+   allocated.  */
 
 static char *
-get_doc_string (void)
+get_doc_string (doc_string_type doc_type, const char *cmd_name)
 {
-  return xstrdup (_("This command is not documented."));
+  if (doc_type == doc_string_show)
+    return xstrprintf (_("Show the current value of '%s'."),
+		       cmd_name).release ();
+  else
+    return xstrprintf (_("Set the current value of '%s'."),
+		       cmd_name).release ();
 }
 
 /* Subroutine of pascm_set_func, pascm_show_func to simplify them.
@@ -992,9 +1008,9 @@  gdbscm_make_parameter (SCM name_scm, SCM rest)
 
   /* If doc is NULL, leave it NULL.  See add_setshow_cmd_full.  */
   if (set_doc == NULL)
-    set_doc = get_doc_string ();
+    set_doc = get_doc_string (doc_string_set, name);
   if (show_doc == NULL)
-    show_doc = get_doc_string ();
+    show_doc = get_doc_string (doc_string_show, name);
 
   s = name;
   name = gdbscm_canonicalize_command_name (s, 0);
diff --git a/gdb/testsuite/gdb.guile/scm-parameter.exp b/gdb/testsuite/gdb.guile/scm-parameter.exp
index 8ab5d939e2a..70e715f57fc 100644
--- a/gdb/testsuite/gdb.guile/scm-parameter.exp
+++ b/gdb/testsuite/gdb.guile/scm-parameter.exp
@@ -314,9 +314,13 @@  with_test_prefix "test-undocumented-param" {
     gdb_test "show print test-undoc-param" "The state of the Test Parameter is on." "show parameter on"
     gdb_test_no_output "set print test-undoc-param off"
     gdb_test "show print test-undoc-param" "The state of the Test Parameter is off." "show parameter off"
-    gdb_test "help show print test-undoc-param" "This command is not documented." "show help"
-    gdb_test "help set print test-undoc-param" "This command is not documented." "set help"
-    gdb_test "help set print" "set print test-undoc-param -- This command is not documented.*" "general help"
+    gdb_test "help show print test-undoc-param" \
+	"Show the current value of 'print test-undoc-param'\\." "show help"
+    gdb_test "help set print test-undoc-param" \
+	"Set the current value of 'print test-undoc-param'\\." "set help"
+    gdb_test "help set print" \
+	"set print test-undoc-param -- Set the current value of 'print test-undoc-param'\\..*" \
+	"general help"
 }
 
 # Test a parameter with a restricted range, where we need to notify the user
@@ -379,13 +383,19 @@  gdb_test_no_output "guile (register-parameter! prev-ambig)"
 
 with_test_prefix "previously-ambiguous" {
     gdb_test "guile (print (parameter-value prev-ambig))" "= #f" "parameter value, false"
-    gdb_test "show print s" "Command is not documented is off." "show parameter off"
+    gdb_test "show print s" \
+	"The current value of 'print s' is off\\." "show parameter off"
     gdb_test_no_output "set print s on"
-    gdb_test "show print s" "Command is not documented is on." "show parameter on"
+    gdb_test "show print s" \
+	"The current value of 'print s' is on\\." "show parameter on"
     gdb_test "guile (print (parameter-value prev-ambig))" "= #t" "parameter value, true"
-    gdb_test "help show print s" "This command is not documented." "show help"
-    gdb_test "help set print s" "This command is not documented." "set help"
-    gdb_test "help set print" "set print s -- This command is not documented.*" "general help"
+    gdb_test "help show print s" \
+	"Show the current value of 'print s'\\." "show help"
+    gdb_test "help set print s" \
+	"Set the current value of 'print s'\\." "set help"
+    gdb_test "help set print" \
+	"set print s -- Set the current value of 'print s'\\..*" \
+	"general help"
 }
 
 rename scm_param_test_maybe_no_output ""