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

Message ID 9c6fccb4-e6ce-e322-a082-8072d4699ebc@suse.de
State New
Headers show
Series
  • Untitled series #45869
Related show

Commit Message

Simon Marchi via Gdb-patches Jan. 3, 2022, 2:50 p.m.
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

Comments

Tom Tromey Jan. 3, 2022, 9:46 p.m. | #1
>>>>> "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

Patch

[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.  */