[PATCH 1/3] [gdb/cli] Fix typo in logging overwrite help text

[PATCH 1/3] [gdb/cli] Fix typo in logging overwrite help text

[Tom de Vries]

Currently we have:
...
$ gdb -q -batch -ex "help set logging overwrite"
Set whether logging overwrites or appends to the log file.
If set, logging overrides the log file.
...

Fix overrides -> overwrites typo.
---
 gdb/cli/cli-logging.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
index f0ee09180f9..8efefa0c5d0 100644
--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -178,7 +178,7 @@ _initialize_cli_logging ()
   add_setshow_boolean_cmd ("overwrite", class_support, &logging_overwrite, _("\
 Set whether logging overwrites or appends to the log file."), _("\
 Show whether logging overwrites or appends to the log file."), _("\
-If set, logging overrides the log file."),
+If set, logging overwrites the log file."),
 			   set_logging_overwrite,
 			   show_logging_overwrite,
 			   &set_logging_cmdlist, &show_logging_cmdlist);

base-commit: 95db489df60d870f5f8f2b32debd6bb0b50c3c5e
-- 
2.26.2

[PATCH 2/3] [gdb/cli] Add "set logging enabled", deprecate "set logging on/off"

[Tom de Vries]

Before commit 3b6acaee895 "Update more calls to add_prefix_cmd" we had the
following output for "show logging file":
...
$ gdb -q -batch -ex "set trace-commands on" \
    -ex "set logging off" \
    -ex "show logging file" \
    -ex "set logging on" \
    -ex "show logging file"
+set logging off
+show logging file
Future logs will be written to gdb.txt.
+set logging on
+show logging file
Currently logging to "gdb.txt".
...

After that commit we have instead:
...
+set logging off
+show logging file
The current logfile is "gdb.txt".
+set logging on
+show logging file
The current logfile is "gdb.txt".
...

Before the commit, whether logging is enabled or not can be deduced from the
output of the command.  After the commit, the message is unified and it's no
longer clear whether logging is enabled or not.

Fix this by:
- adding a new command "show logging enabled"
- adding a corresponding new command "set logging enabled on/off"
- making the commands "set logging on/off" deprecated aliases of the
  "set logging enabled on/off".

Update the docs and testsuite to use "set logging enabled".  Mention the new
and deprecated commands in NEWS.

Tested on x86_64-linux.
---
 gdb/NEWS                                      |  7 +++
 gdb/cli/cli-logging.c                         | 58 +++++++++++++++++--
 gdb/doc/gdb.texinfo                           | 10 ++--
 .../gdb.ada/access_to_packed_array.exp        |  4 +-
 gdb/testsuite/gdb.base/style-logging.exp      |  4 +-
 gdb/testsuite/gdb.base/ui-redirect.exp        | 24 ++++----
 gdb/testsuite/gdb.mi/mi-logging.exp           | 12 ++--
 gdb/testsuite/lib/gdb.exp                     |  4 +-
 8 files changed, 89 insertions(+), 34 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 9e950d2f80d..f6ed934e362 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -32,6 +32,13 @@ maint show internal-warning backtrace
   internal-error, or an internal-warning.  This is on by default for
   internal-error and off by default for internal-warning.
 
+set logging on|off
+  Deprecated and replaced by "set logging enable on|off".
+
+set logging enable on|off
+show logging enable
+  These commands set or show whether logging is enabled or disabled.
+
 * Python API
 
   ** New function gdb.add_history(), which takes a gdb.Value object
diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
index 8efefa0c5d0..ab7b2773d43 100644
--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -22,6 +22,7 @@
 #include "ui-out.h"
 #include "interps.h"
 #include "cli/cli-style.h"
+#include "cli/cli-decode.h"
 
 static char *saved_filename;
 
@@ -163,18 +164,42 @@ set_logging_off (const char *args, int from_tty)
   saved_filename = NULL;
 }
 
+static bool logging_enabled;
+
+static void
+set_logging_enabled (const char *args,
+		     int from_tty, struct cmd_list_element *c)
+{
+  if (logging_enabled)
+    set_logging_on (args, from_tty);
+  else
+    set_logging_off (args, from_tty);
+}
+
+static void
+show_logging_enabled (struct ui_file *file, int from_tty,
+		       struct cmd_list_element *c, const char *value)
+{
+  if (logging_enabled)
+    printf_unfiltered (_("Logging is enabled.\n"));
+  else
+    printf_unfiltered (_("Logging is disabled.\n"));
+}
+
 void _initialize_cli_logging ();
 void
 _initialize_cli_logging ()
 {
   static struct cmd_list_element *set_logging_cmdlist, *show_logging_cmdlist;
 
+  /* Set/show logging.  */
   add_setshow_prefix_cmd ("logging", class_support,
 			  _("Set logging options."),
 			  _("Show logging options."),
 			  &set_logging_cmdlist, &show_logging_cmdlist,
 			  &setlist, &showlist);
 
+  /* Set/show logging overwrite.  */
   add_setshow_boolean_cmd ("overwrite", class_support, &logging_overwrite, _("\
 Set whether logging overwrites or appends to the log file."), _("\
 Show whether logging overwrites or appends to the log file."), _("\
@@ -182,6 +207,8 @@ If set, logging overwrites the log file."),
 			   set_logging_overwrite,
 			   show_logging_overwrite,
 			   &set_logging_cmdlist, &show_logging_cmdlist);
+
+  /* Set/show logging redirect.  */
   add_setshow_boolean_cmd ("redirect", class_support, &logging_redirect, _("\
 Set the logging output mode."), _("\
 Show the logging output mode."), _("\
@@ -190,6 +217,8 @@ If redirect is on, output will go only to the log file."),
 			   set_logging_redirect,
 			   show_logging_redirect,
 			   &set_logging_cmdlist, &show_logging_cmdlist);
+
+  /* Set/show logging debugredirect.  */
   add_setshow_boolean_cmd ("debugredirect", class_support,
 			   &debug_redirect, _("\
 Set the logging debug output mode."), _("\
@@ -200,6 +229,7 @@ If debug redirect is on, debug will go only to the log file."),
 			   show_logging_redirect,
 			   &set_logging_cmdlist, &show_logging_cmdlist);
 
+  /* Set/show logging file.  */
   add_setshow_filename_cmd ("file", class_support, &logging_filename, _("\
 Set the current logfile."), _("\
 Show the current logfile."), _("\
@@ -207,8 +237,28 @@ The logfile is used when directing GDB's output."),
 			    NULL,
 			    show_logging_filename,
 			    &set_logging_cmdlist, &show_logging_cmdlist);
-  add_cmd ("on", class_support, set_logging_on,
-	   _("Enable logging."), &set_logging_cmdlist);
-  add_cmd ("off", class_support, set_logging_off,
-	   _("Disable logging."), &set_logging_cmdlist);
+
+  /* Set/show logging enabled.  */
+  set_show_commands setshow_logging_enabled_cmds
+    = add_setshow_boolean_cmd ("enabled", class_support, &logging_enabled,
+			       _("Enable logging."),
+			       _("Show whether logging is enabled."),
+			       _("help doc"),
+			       set_logging_enabled,
+			       show_logging_enabled,
+			       &set_logging_cmdlist, &show_logging_cmdlist);
+
+  /* Set logging on, deprecated alias.  */
+  cmd_list_element *set_logging_on_cmd
+    = add_alias_cmd ("on", setshow_logging_enabled_cmds.set, class_support,
+		     false, &set_logging_cmdlist);
+  deprecate_cmd (set_logging_on_cmd, "set logging enabled on");
+  set_logging_on_cmd->default_args = "on";
+
+  /* Set logging off, deprecated alias.  */
+  cmd_list_element *set_logging_off_cmd
+    = add_alias_cmd ("off", setshow_logging_enabled_cmds.set, class_support,
+		     false, &set_logging_cmdlist);
+  deprecate_cmd (set_logging_off_cmd, "set logging enabled off");
+  set_logging_off_cmd->default_args = "off";
 }
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1b13973cdc5..cc87d1b24f6 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1705,17 +1705,15 @@ You may want to save the output of @value{GDBN} commands to a file.
 There are several commands to control @value{GDBN}'s logging.
 
 @table @code
-@kindex set logging
-@item set logging on
-Enable logging.
-@item set logging off
-Disable logging.
+@kindex set logging enable
+@item set logging enable [on|off]
+Enable or disable logging.
 @cindex logging file name
 @item set logging file @var{file}
 Change the name of the current logfile.  The default logfile is @file{gdb.txt}.
 @item set logging overwrite [on|off]
 By default, @value{GDBN} will append to the logfile.  Set @code{overwrite} if
-you want @code{set logging on} to overwrite the logfile instead.
+you want @code{set logging enable on} to overwrite the logfile instead.
 @item set logging redirect [on|off]
 By default, @value{GDBN} output will go to both the terminal and the logfile.
 Set @code{redirect} if you want output to go only to the log file.
diff --git a/gdb/testsuite/gdb.ada/access_to_packed_array.exp b/gdb/testsuite/gdb.ada/access_to_packed_array.exp
index 72bb2aa4649..d6e7fc734a9 100644
--- a/gdb/testsuite/gdb.ada/access_to_packed_array.exp
+++ b/gdb/testsuite/gdb.ada/access_to_packed_array.exp
@@ -30,9 +30,9 @@ gdb_test_no_output "maint expand-symtabs"
 set file [standard_output_file gdb.txt]
 gdb_test_no_output "set logging file $file" "set logging file"
 gdb_test_no_output "set logging redirect on"
-gdb_test "set logging on"
+gdb_test "set logging enabled on"
 gdb_test_no_output "maint print symbols"
-gdb_test "set logging off"
+gdb_test "set logging enabled off"
 file delete $file
 
 set bp_location [gdb_get_line_number "BREAK" ${testdir}/foo.adb]
diff --git a/gdb/testsuite/gdb.base/style-logging.exp b/gdb/testsuite/gdb.base/style-logging.exp
index cd0c99aeba8..626b2f0e0bb 100644
--- a/gdb/testsuite/gdb.base/style-logging.exp
+++ b/gdb/testsuite/gdb.base/style-logging.exp
@@ -46,7 +46,7 @@ save_vars { env(TERM) } {
     gdb_test_no_output "set logging file $log_name" \
 	"set logging filename"
     gdb_test_no_output "set logging overwrite on"
-    gdb_test "set logging on" "Copying output to .*"
+    gdb_test "set logging enabled on" "Copying output to .*"
 
     set main_expr [style main function]
     set base_file_expr [style ".*style\\.c" file]
@@ -55,7 +55,7 @@ save_vars { env(TERM) } {
     gdb_test "frame" \
 	"$main_expr.*$arg_expr.*$arg_expr.*$file_expr.*"
 
-    gdb_test "set logging off" "Done logging to .*"
+    gdb_test "set logging enabled off" "Done logging to .*"
 
     set fd [open $log_name]
     set data [read -nonewline $fd]
diff --git a/gdb/testsuite/gdb.base/ui-redirect.exp b/gdb/testsuite/gdb.base/ui-redirect.exp
index 363c563a5a1..de4d9fe886b 100644
--- a/gdb/testsuite/gdb.base/ui-redirect.exp
+++ b/gdb/testsuite/gdb.base/ui-redirect.exp
@@ -76,30 +76,30 @@ with_test_prefix "userdefined" {
 
 with_test_prefix "logging" {
     gdb_test_no_output "set logging file /dev/null"
-    gdb_test "set logging on" \
+    gdb_test "set logging enabled on" \
     "Copying output to /dev/null.*Copying debug output to /dev/null\\."
     gdb_test "save breakpoints $cmds_file" "Saved to file '$cmds_file'\\." \
 	"save breakpoints cmds.txt"
     cmp_file_string "$cmds_file" "$cmds" "cmds.txt"
     gdb_test "userdefined" "#0  main ().*"
-    gdb_test "set logging off" "Done logging to /dev/null\\."
+    gdb_test "set logging enabled off" "Done logging to /dev/null\\."
     gdb_test "help" "List of classes of commands:.*"
 }
 
 with_test_prefix "redirect" {
     gdb_test "set logging redirect on"
-    gdb_test "set logging on" \
+    gdb_test "set logging enabled on" \
     "Redirecting output to /dev/null.*Copying debug output to /dev/null\\."
     gdb_test_no_output "save breakpoints $cmds_file" "save breakpoints cmds.txt"
     cmp_file_string "$cmds_file" "$cmds" "cmds.txt"
     gdb_test_no_output "userdefined"
-    gdb_test "set logging off" "Done logging to /dev/null\\."
+    gdb_test "set logging enabled off" "Done logging to /dev/null\\."
     gdb_test "help" "List of classes of commands:.*"
 }
 
 with_test_prefix "redirect while already logging" {
     gdb_test_no_output "set logging redirect off"
-    gdb_test "set logging on" \
+    gdb_test "set logging enabled on" \
     "Copying output to /dev/null.*Copying debug output to /dev/null\\."
     gdb_test "set logging redirect on" \
     ".*warning: Currently logging .*Turn the logging off and on to make the new setting effective.*"
@@ -107,14 +107,14 @@ with_test_prefix "redirect while already logging" {
 	"save breakpoints cmds.txt"
     cmp_file_string "$cmds_file" "$cmds" "cmds.txt"
     gdb_test "userdefined" "#0  main ().*"
-    gdb_test "set logging off" "Done logging to /dev/null\\."
+    gdb_test "set logging enabled off" "Done logging to /dev/null\\."
     gdb_test "help" "List of classes of commands:.*"
     gdb_test_no_output "set logging redirect off"
 }
 
 with_test_prefix "debugging" {
     gdb_test "set debug infrun 1"
-    gdb_test "set logging on" \
+    gdb_test "set logging enabled on" \
     "Copying output to /dev/null.*Copying debug output to /dev/null\\."
 
     set prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: exit\r\n$"
@@ -125,25 +125,25 @@ with_test_prefix "debugging" {
     }
 
     gdb_test "set debug infrun 0"
-    gdb_test "set logging off" "Done logging to /dev/null\\."
+    gdb_test "set logging enabled off" "Done logging to /dev/null\\."
     gdb_test "help" "List of classes of commands:.*"
 }
 
 with_test_prefix "redirect debugging" {
     gdb_test_no_output "set logging debugredirect on"
     gdb_test "set debug infrun 1"
-    gdb_test "set logging on" \
+    gdb_test "set logging enabled on" \
     "Copying output to /dev/null.*Redirecting debug output to /dev/null\\."
     gdb_test "continue" "Continuing.*((?!infrun).).*Breakpoint \[0-9\]+, bar.*"
     gdb_test "set debug infrun 0"
-    gdb_test "set logging off" "Done logging to /dev/null\\."
+    gdb_test "set logging enabled off" "Done logging to /dev/null\\."
     gdb_test "help" "List of classes of commands:.*"
 }
 
 with_test_prefix "redirect logging and debuging" {
     gdb_test_no_output "set logging redirect on"
     gdb_test_no_output "set logging debugredirect on"
-    gdb_test "set logging on" \
+    gdb_test "set logging enabled on" \
     "Redirecting output to /dev/null.*Redirecting debug output to /dev/null\\."
-    gdb_test "set logging off" "Done logging to /dev/null\\."
+    gdb_test "set logging enabled off" "Done logging to /dev/null\\."
 }
diff --git a/gdb/testsuite/gdb.mi/mi-logging.exp b/gdb/testsuite/gdb.mi/mi-logging.exp
index 35b95969d9d..8672bcbdb9d 100644
--- a/gdb/testsuite/gdb.mi/mi-logging.exp
+++ b/gdb/testsuite/gdb.mi/mi-logging.exp
@@ -37,13 +37,13 @@ mi_gdb_test "-gdb-set logging file $milogfile" "\\^done" \
 
 mi_gdb_test "-gdb-set logging overwrite on" ".*"
 
-mi_gdb_test "-gdb-set logging on" ".*" "logging on"
+mi_gdb_test "-gdb-set logging enabled on" ".*" "logging on"
 
 mi_step "logged step"
 
 mi_next "logged next"
 
-mi_gdb_test "-gdb-set logging off" ".*" "logging off"
+mi_gdb_test "-gdb-set logging enabled off" ".*" "logging off"
 
 set chan [open $milogfile]
 set logcontent [read $chan]
@@ -64,11 +64,11 @@ mi_gdb_test "-gdb-set logging redirect on" ".*" "redirect logging on"
 # Since all output will be going into the file, just keep sending commands
 # and don't expect anything to appear until logging is turned off.
 
-send_gdb "1001-gdb-set logging on\n"
+send_gdb "1001-gdb-set logging enabled on\n"
 send_gdb "1002-exec-step\n"
 send_gdb "1003-exec-next\n"
 
-mi_gdb_test "1004-gdb-set logging off" ".*" "redirect logging off"
+mi_gdb_test "1004-gdb-set logging enabled off" ".*" "redirect logging off"
 
 set chan [open $milogfile]
 set logcontent [read $chan]
@@ -85,12 +85,12 @@ if [regexp "1001\\^done\[\r\n\]+$mi_log_prompt.*1002\\^running\[\r\n\]+\\*runnin
 with_test_prefix "redirect while already logging" {
     mi_gdb_test "-gdb-set logging redirect off" ".*" \
 	"logging redirect off"
-    mi_gdb_test "-gdb-set logging on" ".*" \
+    mi_gdb_test "-gdb-set logging enabled on" ".*" \
 	"logging on"
     mi_gdb_test "-gdb-set logging redirect on" \
 	".*warning: Currently logging .*Turn the logging off and on to make the new setting effective.*" \
 	"logging redirect on"
-    mi_gdb_test "-gdb-set logging off" ".*" \
+    mi_gdb_test "-gdb-set logging enabled off" ".*" \
 	"logging off"
 }
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index deb1b8f2138..77f889e7404 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -7661,7 +7661,7 @@ proc gdb_debug_init { } {
     }
 
     # First ensure logging is off.
-    send_gdb "set logging off\n"
+    send_gdb "set logging enabled off\n"
 
     set debugfile [standard_output_file gdb.debug]
     send_gdb "set logging file $debugfile\n"
@@ -7674,7 +7674,7 @@ proc gdb_debug_init { } {
     }
 
     # Now that everything is set, enable logging.
-    send_gdb "set logging on\n"
+    send_gdb "set logging enabled on\n"
     gdb_expect 10 {
 	-re "Copying output to $debugfile.*Redirecting debug output to $debugfile.*$gdb_prompt $" {}
 	timeout { warning "Couldn't set logging file" }
-- 
2.26.2

[PATCH 3/3] [gdb/cli] Improve show logging output

[Tom de Vries]

Before commit 3b6acaee895 "Update more calls to add_prefix_cmd" we had the
following output for "show logging":
...
$ gdb -q -batch -ex "set trace-commands on" \
    -ex "set logging off" \
    -ex "show logging" \
    -ex "set logging on" \
    -ex "show logging"
+set logging off
+show logging
Future logs will be written to gdb.txt.
Logs will be appended to the log file.
Output will be logged and displayed.
Debug output will be logged and displayed.
+set logging on
+show logging
Currently logging to "gdb.txt".
Logs will be appended to the log file.
Output will be logged and displayed.
Debug output will be logged and displayed.
...

After that commit we have instead:
...
+set logging off
+show logging
debugredirect:  The logging output mode is off.
file:  The current logfile is "gdb.txt".
overwrite:  Whether logging overwrites or appends to the log file is off.
redirect:  The logging output mode is off.
+set logging on
+show logging
debugredirect:  The logging output mode is off.
file:  The current logfile is "gdb.txt".
overwrite:  Whether logging overwrites or appends to the log file is off.
redirect:  The logging output mode is off.
...
which gives less clear output for some subcommands.

OTOH, it's explicit about whether boolean values are on or off.

The new text seems to have been chosen to match the set/show help texts:
...
(gdb) help show logging
Show logging options.

List of show logging subcommands:

show logging debugredirect -- Show the logging debug output mode.
show logging file -- Show the current logfile.
show logging overwrite -- \
  Show whether logging overwrites or appends to the log file.
show logging redirect -- Show the logging output mode.
...

Make the show logging messages more clear, while still keep the boolean
values explicit, such that we have:
...
$ ./gdb.sh -q -batch -ex "show logging"
logging debugredirect:  off: \
  Debug output will go to both the screen and the log file.
logging enabled:  off: Logging is disabled.
logging file:  The current logfile is "gdb.txt".
logging overwrite:  off: Logging appends to the log file.
logging redirect:  off: Output will go to both the screen and the log file.
...

Tested on x86_64-linux.
---
 gdb/cli/cli-logging.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
index ab7b2773d43..3778b88865b 100644
--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -56,10 +56,10 @@ static void
 show_logging_overwrite (struct ui_file *file, int from_tty,
 			struct cmd_list_element *c, const char *value)
 {
-  fprintf_filtered (file,
-		    _("Whether logging overwrites or "
-		      "appends to the log file is %s.\n"),
-		    value);
+  if (logging_overwrite)
+    fprintf_filtered (file, _("on: Logging overwrites the log file.\n"));
+  else
+    fprintf_filtered (file, _("off: Logging appends to the log file.\n"));
 }
 
 /* Value as configured by the user.  */
@@ -77,7 +77,25 @@ static void
 show_logging_redirect (struct ui_file *file, int from_tty,
 		       struct cmd_list_element *c, const char *value)
 {
-  fprintf_filtered (file, _("The logging output mode is %s.\n"), value);
+  if (logging_redirect)
+    fprintf_filtered(file, _("on: Output will go only to the log file.\n"));
+  else
+    fprintf_filtered
+      (file,
+       _("off: Output will go to both the screen and the log file.\n"));
+}
+
+static void
+show_logging_debug_redirect (struct ui_file *file, int from_tty,
+		       struct cmd_list_element *c, const char *value)
+{
+  if (debug_redirect)
+    fprintf_filtered(file,
+		     _("on: Debug output will go only to the log file.\n"));
+  else
+    fprintf_filtered
+      (file,
+       _("off: Debug output will go to both the screen and the log file.\n"));
 }
 
 /* If we've pushed output files, close them and pop them.  */
@@ -181,9 +199,9 @@ show_logging_enabled (struct ui_file *file, int from_tty,
 		       struct cmd_list_element *c, const char *value)
 {
   if (logging_enabled)
-    printf_unfiltered (_("Logging is enabled.\n"));
+    printf_unfiltered (_("on: Logging is enabled.\n"));
   else
-    printf_unfiltered (_("Logging is disabled.\n"));
+    printf_unfiltered (_("off: Logging is disabled.\n"));
 }
 
 void _initialize_cli_logging ();
@@ -226,7 +244,7 @@ Show the logging debug output mode."), _("\
 If debug redirect is off, debug will go to both the screen and the log file.\n\
 If debug redirect is on, debug will go only to the log file."),
 			   set_logging_redirect,
-			   show_logging_redirect,
+			   show_logging_debug_redirect,
 			   &set_logging_cmdlist, &show_logging_cmdlist);
 
   /* Set/show logging file.  */
-- 
2.26.2

Re: [PATCH 2/3] [gdb/cli] Add "set logging enabled", deprecate "set logging on/off"

[Tom de Vries]

On 11/24/21 11:03 AM, Tom de Vries via Gdb-patches wrote:
> +  /* Set/show logging enabled.  */
> +  set_show_commands setshow_logging_enabled_cmds
> +    = add_setshow_boolean_cmd ("enabled", class_support, &logging_enabled,
> +			       _("Enable logging."),
> +			       _("Show whether logging is enabled."),
> +			       _("help doc"),
> +			       set_logging_enabled,
> +			       show_logging_enabled,
> +			       &set_logging_cmdlist, &show_logging_cmdlist);
> +

I propose this fixup:
...
diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
index 3778b88865b..3fbca6ff0b0 100644
--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -261,7 +261,7 @@ The logfile is used when directing GDB's output."),
     = add_setshow_boolean_cmd ("enabled", class_support, &logging_enabled,
                               _("Enable logging."),
                               _("Show whether logging is enabled."),
-                              _("help doc"),
+                              _("When on, enable logging."),
                               set_logging_enabled,
                               show_logging_enabled,
                               &set_logging_cmdlist, &show_logging_cmdlist);
...

Thanks,
- Tom

Re: [PATCH 3/3] [gdb/cli] Improve show logging output

[Luis Machado]

On 11/24/21 7:03 AM, Tom de Vries via Gdb-patches wrote:
> Before commit 3b6acaee895 "Update more calls to add_prefix_cmd" we had the
> following output for "show logging":
> ...
> $ gdb -q -batch -ex "set trace-commands on" \
>      -ex "set logging off" \
>      -ex "show logging" \
>      -ex "set logging on" \
>      -ex "show logging"
> +set logging off
> +show logging
> Future logs will be written to gdb.txt.
> Logs will be appended to the log file.
> Output will be logged and displayed.
> Debug output will be logged and displayed.
> +set logging on
> +show logging
> Currently logging to "gdb.txt".
> Logs will be appended to the log file.
> Output will be logged and displayed.
> Debug output will be logged and displayed.
> ...
> 
> After that commit we have instead:
> ...
> +set logging off
> +show logging
> debugredirect:  The logging output mode is off.
> file:  The current logfile is "gdb.txt".
> overwrite:  Whether logging overwrites or appends to the log file is off.
> redirect:  The logging output mode is off.
> +set logging on
> +show logging
> debugredirect:  The logging output mode is off.
> file:  The current logfile is "gdb.txt".
> overwrite:  Whether logging overwrites or appends to the log file is off.
> redirect:  The logging output mode is off.
> ...
> which gives less clear output for some subcommands.
> 
> OTOH, it's explicit about whether boolean values are on or off.
> 
> The new text seems to have been chosen to match the set/show help texts:
> ...
> (gdb) help show logging
> Show logging options.
> 
> List of show logging subcommands:
> 
> show logging debugredirect -- Show the logging debug output mode.
> show logging file -- Show the current logfile.
> show logging overwrite -- \
>    Show whether logging overwrites or appends to the log file.
> show logging redirect -- Show the logging output mode.
> ... >
> Make the show logging messages more clear, while still keep the boolean
> values explicit, such that we have:
> ...
> $ ./gdb.sh -q -batch -ex "show logging"
> logging debugredirect:  off: \
>    Debug output will go to both the screen and the log file.
> logging enabled:  off: Logging is disabled.
> logging file:  The current logfile is "gdb.txt".
> logging overwrite:  off: Logging appends to the log file.
> logging redirect:  off: Output will go to both the screen and the log file.
> ...
> 

Thanks. I think the output looks much better. The "logging enabled" 
output is still a little convoluted with this new format, but I suppose 
there is not much we can do given the restrictions of the new format.

The patch looks OK to me.

Re: [PATCH 1/3] [gdb/cli] Fix typo in logging overwrite help text

[Simon Marchi]

On 2021-11-24 5:03 a.m., Tom de Vries via Gdb-patches wrote:
> Currently we have:
> ...
> $ gdb -q -batch -ex "help set logging overwrite"
> Set whether logging overwrites or appends to the log file.
> If set, logging overrides the log file.
> ...
> 
> Fix overrides -> overwrites typo.
> ---
>  gdb/cli/cli-logging.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
> index f0ee09180f9..8efefa0c5d0 100644
> --- a/gdb/cli/cli-logging.c
> +++ b/gdb/cli/cli-logging.c
> @@ -178,7 +178,7 @@ _initialize_cli_logging ()
>    add_setshow_boolean_cmd ("overwrite", class_support, &logging_overwrite, _("\
>  Set whether logging overwrites or appends to the log file."), _("\
>  Show whether logging overwrites or appends to the log file."), _("\
> -If set, logging overrides the log file."),
> +If set, logging overwrites the log file."),
>  			   set_logging_overwrite,
>  			   show_logging_overwrite,
>  			   &set_logging_cmdlist, &show_logging_cmdlist);
> 
> base-commit: 95db489df60d870f5f8f2b32debd6bb0b50c3c5e
> -- 
> 2.26.2
> 


LGTM.

Simon

Re: [PATCH 2/3] [gdb/cli] Add "set logging enabled", deprecate "set logging on/off"

[Simon Marchi]

LGTM, just a few spots that say "enable" instead of "enabled".

On 2021-11-24 5:03 a.m., Tom de Vries via Gdb-patches wrote:
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 9e950d2f80d..f6ed934e362 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -32,6 +32,13 @@ maint show internal-warning backtrace
>    internal-error, or an internal-warning.  This is on by default for
>    internal-error and off by default for internal-warning.
>
> +set logging on|off
> +  Deprecated and replaced by "set logging enable on|off".
> +
> +set logging enable on|off
> +show logging enable
> +  These commands set or show whether logging is enabled or disabled.

enable -> enabled (3 times)

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 1b13973cdc5..cc87d1b24f6 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -1705,17 +1705,15 @@ You may want to save the output of @value{GDBN} commands to a file.
>  There are several commands to control @value{GDBN}'s logging.
>
>  @table @code
> -@kindex set logging
> -@item set logging on
> -Enable logging.
> -@item set logging off
> -Disable logging.
> +@kindex set logging enable
> +@item set logging enable [on|off]

enable -> enabled (twice)

> +Enable or disable logging.
>  @cindex logging file name
>  @item set logging file @var{file}
>  Change the name of the current logfile.  The default logfile is @file{gdb.txt}.
>  @item set logging overwrite [on|off]
>  By default, @value{GDBN} will append to the logfile.  Set @code{overwrite} if
> -you want @code{set logging on} to overwrite the logfile instead.
> +you want @code{set logging enable on} to overwrite the logfile instead.

enable -> enabled

Simon

[PING][PATCH 3/3] [gdb/cli] Improve show logging output

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 2917 bytes --]

On 11/24/21 13:04, Luis Machado wrote:
> On 11/24/21 7:03 AM, Tom de Vries via Gdb-patches wrote:
>> Before commit 3b6acaee895 "Update more calls to add_prefix_cmd" we had 
>> the
>> following output for "show logging":
>> ...
>> $ gdb -q -batch -ex "set trace-commands on" \
>>      -ex "set logging off" \
>>      -ex "show logging" \
>>      -ex "set logging on" \
>>      -ex "show logging"
>> +set logging off
>> +show logging
>> Future logs will be written to gdb.txt.
>> Logs will be appended to the log file.
>> Output will be logged and displayed.
>> Debug output will be logged and displayed.
>> +set logging on
>> +show logging
>> Currently logging to "gdb.txt".
>> Logs will be appended to the log file.
>> Output will be logged and displayed.
>> Debug output will be logged and displayed.
>> ...
>>
>> After that commit we have instead:
>> ...
>> +set logging off
>> +show logging
>> debugredirect:  The logging output mode is off.
>> file:  The current logfile is "gdb.txt".
>> overwrite:  Whether logging overwrites or appends to the log file is off.
>> redirect:  The logging output mode is off.
>> +set logging on
>> +show logging
>> debugredirect:  The logging output mode is off.
>> file:  The current logfile is "gdb.txt".
>> overwrite:  Whether logging overwrites or appends to the log file is off.
>> redirect:  The logging output mode is off.
>> ...
>> which gives less clear output for some subcommands.
>>
>> OTOH, it's explicit about whether boolean values are on or off.
>>
>> The new text seems to have been chosen to match the set/show help texts:
>> ...
>> (gdb) help show logging
>> Show logging options.
>>
>> List of show logging subcommands:
>>
>> show logging debugredirect -- Show the logging debug output mode.
>> show logging file -- Show the current logfile.
>> show logging overwrite -- \
>>    Show whether logging overwrites or appends to the log file.
>> show logging redirect -- Show the logging output mode.
>> ... >
>> Make the show logging messages more clear, while still keep the boolean
>> values explicit, such that we have:
>> ...
>> $ ./gdb.sh -q -batch -ex "show logging"
>> logging debugredirect:  off: \
>>    Debug output will go to both the screen and the log file.
>> logging enabled:  off: Logging is disabled.
>> logging file:  The current logfile is "gdb.txt".
>> logging overwrite:  off: Logging appends to the log file.
>> logging redirect:  off: Output will go to both the screen and the log 
>> file.
>> ...
>>
> 
> Thanks. I think the output looks much better. The "logging enabled" 
> output is still a little convoluted with this new format, but I suppose 
> there is not much we can do given the restrictions of the new format.
> 
> The patch looks OK to me.

Thanks for the review.

Updated patch for current trunk.

Ping for review from somebody else, to get a confirmation that this 
style is ok.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-cli-Improve-show-logging-output.patch --]
[-- Type: text/x-patch, Size: 4749 bytes --]

[gdb/cli] Improve show logging output

Before commit 3b6acaee895 "Update more calls to add_prefix_cmd" we had the
following output for "show logging":
...
$ gdb -q -batch -ex "set trace-commands on" \
    -ex "set logging off" \
    -ex "show logging" \
    -ex "set logging on" \
    -ex "show logging"
+set logging off
+show logging
Future logs will be written to gdb.txt.
Logs will be appended to the log file.
Output will be logged and displayed.
Debug output will be logged and displayed.
+set logging on
+show logging
Currently logging to "gdb.txt".
Logs will be appended to the log file.
Output will be logged and displayed.
Debug output will be logged and displayed.
...

After that commit we have instead:
...
+set logging off
+show logging
debugredirect:  The logging output mode is off.
file:  The current logfile is "gdb.txt".
overwrite:  Whether logging overwrites or appends to the log file is off.
redirect:  The logging output mode is off.
+set logging on
+show logging
debugredirect:  The logging output mode is off.
file:  The current logfile is "gdb.txt".
overwrite:  Whether logging overwrites or appends to the log file is off.
redirect:  The logging output mode is off.
...
which gives less clear output for some subcommands.

OTOH, it's explicit about whether boolean values are on or off.

The new text seems to have been chosen to match the set/show help texts:
...
(gdb) help show logging
Show logging options.

List of show logging subcommands:

show logging debugredirect -- Show the logging debug output mode.
show logging file -- Show the current logfile.
show logging overwrite -- \
  Show whether logging overwrites or appends to the log file.
show logging redirect -- Show the logging output mode.
...

Make the show logging messages more clear, while still keep the boolean
values explicit, such that we have:
...
$ ./gdb.sh -q -batch -ex "show logging"
logging debugredirect:  off: \
  Debug output will go to both the screen and the log file.
logging enabled:  off: Logging is disabled.
logging file:  The current logfile is "gdb.txt".
logging overwrite:  off: Logging appends to the log file.
logging redirect:  off: Output will go to both the screen and the log file.
...

Tested on x86_64-linux.

---
 gdb/cli/cli-logging.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
index d659a0dc2ef..193a87331db 100644
--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -56,10 +56,10 @@ static void
 show_logging_overwrite (struct ui_file *file, int from_tty,
 			struct cmd_list_element *c, const char *value)
 {
-  fprintf_filtered (file,
-		    _("Whether logging overwrites or "
-		      "appends to the log file is %s.\n"),
-		    value);
+  if (logging_overwrite)
+    fprintf_filtered (file, _("on: Logging overwrites the log file.\n"));
+  else
+    fprintf_filtered (file, _("off: Logging appends to the log file.\n"));
 }
 
 /* Value as configured by the user.  */
@@ -77,7 +77,25 @@ static void
 show_logging_redirect (struct ui_file *file, int from_tty,
 		       struct cmd_list_element *c, const char *value)
 {
-  fprintf_filtered (file, _("The logging output mode is %s.\n"), value);
+  if (logging_redirect)
+    fprintf_filtered(file, _("on: Output will go only to the log file.\n"));
+  else
+    fprintf_filtered
+      (file,
+       _("off: Output will go to both the screen and the log file.\n"));
+}
+
+static void
+show_logging_debug_redirect (struct ui_file *file, int from_tty,
+		       struct cmd_list_element *c, const char *value)
+{
+  if (debug_redirect)
+    fprintf_filtered(file,
+		     _("on: Debug output will go only to the log file.\n"));
+  else
+    fprintf_filtered
+      (file,
+       _("off: Debug output will go to both the screen and the log file.\n"));
 }
 
 /* If we've pushed output files, close them and pop them.  */
@@ -181,9 +199,9 @@ show_logging_enabled (struct ui_file *file, int from_tty,
 		       struct cmd_list_element *c, const char *value)
 {
   if (logging_enabled)
-    fprintf_filtered (file, _("Logging is enabled.\n"));
+    fprintf_unfiltered (_("on: Logging is enabled.\n"));
   else
-    fprintf_filtered (file, _("Logging is disabled.\n"));
+    fprintf_unfiltered (_("off: Logging is disabled.\n"));
 }
 
 void _initialize_cli_logging ();
@@ -226,7 +244,7 @@ Show the logging debug output mode."), _("\
 If debug redirect is off, debug will go to both the screen and the log file.\n\
 If debug redirect is on, debug will go only to the log file."),
 			   set_logging_redirect,
-			   show_logging_redirect,
+			   show_logging_debug_redirect,
 			   &set_logging_cmdlist, &show_logging_cmdlist);
 
   /* Set/show logging file.  */

Re: [PING][PATCH 3/3] [gdb/cli] Improve show logging output

[Tom Tromey]

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> Ping for review from somebody else, to get a confirmation that this
Tom> style is ok.

It looks reasonable to me.

Sometimes I wish gdb went a little farther in this direction and had all
"show xyz" commands use a simplified formatting, like:

    (gdb) show xyz
    xyz: value

Ideally this would be combined with having some kind of "get the current
value" callback, so this could handle the "auto" case nicely.

However:

Tom> +  if (logging_overwrite)
Tom> +    fprintf_filtered (file, _("on: Logging overwrites the log file.\n"));
Tom> +  else
Tom> +    fprintf_filtered (file, _("off: Logging appends to the log file.\n"));

... with your patch I see that it's nice in this situation to sometimes
also print a bit of documentation.  Maybe this too could be
standardized, though I don't see how right away.

Tom