[2/2] gdb/guile: generate general description string for parameters

Message ID ef853684b8c3fb494de659e57c44901f53a4fa1c.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
  This commit builds on the previous one, and auto-generates a general
description string for parameters defined via the Guile API.  This
brings the Guile API closer inline with the Python API.  It is worth
reading the previous commit to see some motivating examples.

This commit updates get_doc_string in guile/scm-param.c to allow for
the generation of a general description string.  Then in
gdbscm_make_parameter, if '#:doc' was not given, then get_doc_string
is used to generate a suitable default.

This does invalidate (and so the commit removes) this comment that was
in gdbscm_make_parameter:

  /* If doc is NULL, leave it NULL.  See add_setshow_cmd_full.  */

First, Python already does exactly what I'm proposing here, and has
done for a while, with no issues reported.  And second, I've gone and
read add_setshow_cmd_full, and some of the functions it calls, and can
see no reasoning behind this comment...

... well, there is one reason that I can think of, but I'll discuss
that more below.

With this commit, if I define a parameter like this:

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

Then, in GDB, I now see this behaviour:

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

The two 'This command is not documented.' lines are new.  This output
is what we get from a similarly defined parameter using the Python
API.

I mentioned above that I can think of one reason for the (now deleted)
command in gdbscm_make_parameter about leaving the doc field as NULL,
and that is this: consider the following GDB behaviour:

  (gdb) help show style filename foreground
  Show the foreground color for this property.
  (gdb)

Notice there is only a single line of output.  If I want to get the
same behaviour from a parameter defined in Guile, I might try skipping
the #:doc argument, but (after this commit), if I do that, GDB will
auto-generate some text for me, giving two lines of output (see
above).

So, next, maybe I try setting #:doc to the empty string, but if I do
that, then I get this:

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

  (gdb) help show print test
  Show the current value of 'print test'.

  (gdb)

Notice the blank line, that's not what I wanted.  In fact, the only
way to get rid of the second line is to leave the 'doc' variable as
NULL in gdbscm_make_parameter, which, due to the new auto-generation,
is no longer possible.

However, I have a solution for this.  I already posted a similar idea
of the Python API here:

  https://inbox.sourceware.org/gdb-patches/e80097d463ea9989f455e53851e1d155697ecadc.1744412484.git.aburgess@redhat.com

and this commit includes the same solution for the Guile API.  The
proposal is that, an empty doc string should be treated as no doc
string.  So now, with this commit, the Guile parameter (with the empty
'#:doc' string) gives the following behaviour:

  (gdb) help show print test
  Show the current value of 'print test'.
  (gdb)

This is inline with the Python API when the patch linked above is
applied.
---
 gdb/NEWS                                  |  5 ++
 gdb/doc/guile.texi                        |  6 ++-
 gdb/guile/scm-param.c                     | 23 +++++++---
 gdb/testsuite/gdb.guile/scm-parameter.exp | 56 +++++++++++++++++++----
 4 files changed, 72 insertions(+), 18 deletions(-)
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 99ec392d4c4..8319ff69a88 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -86,6 +86,11 @@  info sharedlibrary
   ** New constant PARAM_COLOR represents color type of a value
      of a <gdb:parameter> object.  Parameter's value is <gdb::color> instance.
 
+  ** Eliding the #:doc string from make-parameter now means that GDB
+     will use a default documentation string.  Setting #:doc to the
+     empty string for make-parameter means GDB will only display the
+     #:set_doc or #:show_doc strings in the set/show help output.
+
 * New remote packets
 
 binary-upload in qSupported reply
diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
index c6808efa432..4b36a934023 100644
--- a/gdb/doc/guile.texi
+++ b/gdb/doc/guile.texi
@@ -2098,8 +2098,10 @@  Parameters In Guile
 This function must return a string, and will be displayed to the user.
 @value{GDBN} will add a trailing newline.
 
-The argument @var{doc} is the help text for the new parameter.
-If there is no documentation string, a default value is used.
+The argument @var{doc} is the help text for the new parameter.  If
+there is no documentation string, a default value is used.  If the
+documentation string is empty, then @value{GDBN} will print just the
+@var{set-doc} and @var{show-doc} strings (see below).
 
 The argument @var{set-doc} is the help text for this parameter's
 @code{set} command.
diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
index bc6c605b59c..51378474844 100644
--- a/gdb/guile/scm-param.c
+++ b/gdb/guile/scm-param.c
@@ -315,6 +315,7 @@  enum doc_string_type
 {
   doc_string_set,
   doc_string_show,
+  doc_string_description
 };
 
 /* A helper function which returns the default documentation string for
@@ -325,12 +326,19 @@  enum doc_string_type
 static char *
 get_doc_string (doc_string_type doc_type, const char *cmd_name)
 {
-  if (doc_type == doc_string_show)
-    return xstrprintf (_("Show the current value of '%s'."),
-		       cmd_name).release ();
+  if (doc_type == doc_string_description)
+    return xstrdup (_("This command is not documented."));
   else
-    return xstrprintf (_("Set the current value of '%s'."),
-		       cmd_name).release ();
+    {
+      gdb_assert (cmd_name != nullptr);
+
+      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.
@@ -1006,7 +1014,10 @@  gdbscm_make_parameter (SCM name_scm, SCM rest)
 			      &show_doc_arg_pos, &show_doc,
 			      &initial_value_arg_pos, &initial_value_scm);
 
-  /* If doc is NULL, leave it NULL.  See add_setshow_cmd_full.  */
+  if (doc == nullptr)
+    doc = get_doc_string (doc_string_description, nullptr);
+  else if (*doc == '\0')
+    doc = nullptr;
   if (set_doc == NULL)
     set_doc = get_doc_string (doc_string_set, name);
   if (show_doc == NULL)
diff --git a/gdb/testsuite/gdb.guile/scm-parameter.exp b/gdb/testsuite/gdb.guile/scm-parameter.exp
index 70e715f57fc..d1b19b08b53 100644
--- a/gdb/testsuite/gdb.guile/scm-parameter.exp
+++ b/gdb/testsuite/gdb.guile/scm-parameter.exp
@@ -67,9 +67,19 @@  with_test_prefix "test-param" {
     gdb_test_no_output "set print test-param off"
     gdb_test "show print test-param" "The state of the Test Parameter is off." "show parameter off"
     gdb_test "guile (print (parameter-value test-param))" "= #f" "parameter value, false"
-    gdb_test "help show print test-param" "Show the state of the boolean test-param.*" "show help"
-    gdb_test "help set print test-param" "Set the state of the boolean test-param.*" "set help"
-    gdb_test "help set print" "set print test-param -- Set the state of the boolean test-param.*" "general help"
+    gdb_test "help show print test-param" \
+	[multi_line \
+	     "^Show the state of the boolean test-param\\." \
+	     "When enabled, test param does something useful\\. When disabled, does nothing\\."] \
+	"show help"
+    gdb_test "help set print test-param" \
+	[multi_line \
+	     "^Set the state of the boolean test-param\\." \
+	     "When enabled, test param does something useful\\. When disabled, does nothing\\."] \
+	 "set help"
+    gdb_test "help set print" \
+	"set print test-param -- Set the state of the boolean test-param.*" \
+	"general help"
 
     gdb_test "guile (print (parameter? test-param))" "= #t"
     gdb_test "guile (print (parameter? 42))" "= #f"
@@ -315,12 +325,16 @@  with_test_prefix "test-undocumented-param" {
     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" \
-	"Show the current value of 'print test-undoc-param'\\." "show help"
+	[multi_line \
+	     "^Show the current value of 'print test-undoc-param'\\." \
+	     "This command is not documented\\."] \
+	"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"
+	[multi_line \
+	     "Set the current value of 'print test-undoc-param'\\." \
+	     "This command is not documented\\."] \
+	 "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
@@ -390,14 +404,36 @@  with_test_prefix "previously-ambiguous" {
 	"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" \
-	"Show the current value of 'print s'\\." "show help"
+	[multi_line \
+	     "^Show the current value of 'print s'\\." \
+	     "This command is not documented\\."] \
+	"show help"
     gdb_test "help set print s" \
-	"Set the current value of 'print s'\\." "set help"
+	[multi_line \
+	     "Set the current value of 'print s'\\." \
+	     "This command is not documented\\."] \
+	"set help"
     gdb_test "help set print" \
 	"set print s -- Set the current value of 'print s'\\..*" \
 	"general help"
 }
 
+with_test_prefix "empty doc string" {
+    gdb_test_multiline "empty doc string parameter" \
+	"guile" "" \
+	"(register-parameter! (make-parameter \"empty-doc-string\"" "" \
+	"   #:command-class COMMAND_NONE" "" \
+	"   #:parameter-type PARAM_ZINTEGER" "" \
+	"   #:doc \"\"" "" \
+	"   #:set-doc \"Set doc string.\"" "" \
+	"   #:show-doc \"Show doc string.\"))" "" \
+	"end"
+
+    gdb_test "help set empty-doc-string" "^Set doc string\\."
+    gdb_test "help show empty-doc-string" "^Show doc string\\."
+
+}
+
 rename scm_param_test_maybe_no_output ""
 
 # Test a color parameter.