[rfa] clean up output of "info set" command.

[rfa] clean up output of "info set" command.

[Michael Snyder]

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

"info set" is meant to show the state of all "set"-able
debugger state variables, but it really executes each and
every "show" command in alphabetical order.

Several of these commands (notably "copying" and "warranty")
have no corresponding "set" command, are not "set"-able, and
produce a lot of output.  Especially "show copying" which
produces pages and pages of output.

This patch excludes "copying", "warranty", and "version" from
the output of "info set'.


[-- Attachment #2: infoset.txt --]
[-- Type: text/plain, Size: 1608 bytes --]

2011-02-10  Michael Snyder  <msnyder@msnyder-server.eng.vmware.com>

	* cli/cli-setshow.c (cmd_show_list): Skip commands show copying, 
	show warranty and show version.

Index: cli/cli-setshow.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-setshow.c,v
retrieving revision 1.43
diff -u -p -u -p -r1.43 cli-setshow.c
--- cli/cli-setshow.c	6 Jan 2011 00:57:02 -0000	1.43
+++ cli/cli-setshow.c	11 Feb 2011 20:14:22 -0000
@@ -434,18 +434,23 @@ cmd_show_list (struct cmd_list_element *
 	}
       else
 	{
-	  struct cleanup *option_chain
-	    = make_cleanup_ui_out_tuple_begin_end (uiout, "option");
+	  if (strcmp (list->name, "copying") != 0
+	      && strcmp (list->name, "version") != 0
+	      && strcmp (list->name, "warranty") != 0)
+	    {
+	      struct cleanup *option_chain
+		= make_cleanup_ui_out_tuple_begin_end (uiout, "option");
 
-	  ui_out_text (uiout, prefix);
-	  ui_out_field_string (uiout, "name", list->name);
-	  ui_out_text (uiout, ":  ");
-	  if (list->type == show_cmd)
-	    do_setshow_command ((char *) NULL, from_tty, list);
-	  else
-	    cmd_func (list, NULL, from_tty);
-          /* Close the tuple.  */
-	  do_cleanups (option_chain);
+	      ui_out_text (uiout, prefix);
+	      ui_out_field_string (uiout, "name", list->name);
+	      ui_out_text (uiout, ":  ");
+	      if (list->type == show_cmd)
+		do_setshow_command ((char *) NULL, from_tty, list);
+	      else
+		cmd_func (list, NULL, from_tty);
+	      /* Close the tuple.  */
+	      do_cleanups (option_chain);
+	    }
 	}
     }
   /* Close the tuple.  */

Re: [rfa] clean up output of "info set" command.

[Pedro Alves]

On Friday 11 February 2011 20:19:52, Michael Snyder wrote:
> "info set" is meant to show the state of all "set"-able
> debugger state variables, but it really executes each and
> every "show" command in alphabetical order.
> 
> Several of these commands (notably "copying" and "warranty")
> have no corresponding "set" command, are not "set"-able, and
> produce a lot of output.  Especially "show copying" which
> produces pages and pages of output.

Isn't there a property of the command we could check
instead of hardcoding specific command names?

> 
> This patch excludes "copying", "warranty", and "version" from
> the output of "info set'.

-- 
Pedro Alves

Re: [rfa] clean up output of "info set" command.

[Michael Snyder]

Pedro Alves wrote:
> On Friday 11 February 2011 20:19:52, Michael Snyder wrote:
>> "info set" is meant to show the state of all "set"-able
>> debugger state variables, but it really executes each and
>> every "show" command in alphabetical order.
>>
>> Several of these commands (notably "copying" and "warranty")
>> have no corresponding "set" command, are not "set"-able, and
>> produce a lot of output.  Especially "show copying" which
>> produces pages and pages of output.
> 
> Isn't there a property of the command we could check
> instead of hardcoding specific command names?

I'm open to suggestions.  The only property I can think of is that
there is no corresponding entry in "setlist".  I could search
setlist every time...

>> This patch excludes "copying", "warranty", and "version" from
>> the output of "info set'.
> 

Re: [rfa] clean up output of "info set" command.

[Tom Tromey]

>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:

Pedro> Isn't there a property of the command we could check
Pedro> instead of hardcoding specific command names?

Michael> I'm open to suggestions.  The only property I can think of is that
Michael> there is no corresponding entry in "setlist".  I could search
Michael> setlist every time...

You could stick a new flag on the command object.

Or there is cmd_cfunc_eq, which is used for a similar purpose in some
places.  I don't think this is super, but OTOH it isn't any worse than
existing code.

Tom

Re: [rfa] clean up output of "info set" command.

[Pedro Alves]

On Friday 11 February 2011 21:17:08, Tom Tromey wrote:
> >>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:
> 
> Pedro> Isn't there a property of the command we could check
> Pedro> instead of hardcoding specific command names?
> 
> Michael> I'm open to suggestions.  The only property I can think of is that
> Michael> there is no corresponding entry in "setlist".  I could search
> Michael> setlist every time...
> 
> You could stick a new flag on the command object.

Yeah.  You may even be able to set the flag from within
the add_setshow_... functions and friends.  The flag could
mean "this show command shows something that is settable
in some way" (or the reverse).

> Or there is cmd_cfunc_eq, which is used for a similar purpose in some
> places.  I don't think this is super, but OTOH it isn't any worse than
> existing code.

I think we shouldn't allow ourselves to broadcast bad design
when it's easy not to.  The function Michael touched is within
gdb/cli/cli-setshow.c.  I'd prefer to keep this and the other
core command files clean of specific knowledge of 
any specific commands their clients register.

I notice that "info set" is basically an alias of "show".
Does the change make sense in the context of "show",
or should "show" keep showing everything showable under
the show command?  "show foo|bar|..."

-- 
Pedro Alves

Re: [rfa] clean up output of "info set" command.

[Michael Snyder]

Pedro Alves wrote:
> On Friday 11 February 2011 21:17:08, Tom Tromey wrote:
>>>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:
>> Pedro> Isn't there a property of the command we could check
>> Pedro> instead of hardcoding specific command names?
>>
>> Michael> I'm open to suggestions.  The only property I can think of is that
>> Michael> there is no corresponding entry in "setlist".  I could search
>> Michael> setlist every time...
>>
>> You could stick a new flag on the command object.
> 
> Yeah.  You may even be able to set the flag from within
> the add_setshow_... functions and friends.  The flag could
> mean "this show command shows something that is settable
> in some way" (or the reverse).
> 
>> Or there is cmd_cfunc_eq, which is used for a similar purpose in some
>> places.  I don't think this is super, but OTOH it isn't any worse than
>> existing code.
> 
> I think we shouldn't allow ourselves to broadcast bad design
> when it's easy not to.  The function Michael touched is within
> gdb/cli/cli-setshow.c.  I'd prefer to keep this and the other
> core command files clean of specific knowledge of 
> any specific commands their clients register.
> 
> I notice that "info set" is basically an alias of "show".
> Does the change make sense in the context of "show",
> or should "show" keep showing everything showable under
> the show command?  "show foo|bar|..."
> 

They're synonyms.  But I still don't think that "show copying"
is at all useful when presented in the middle of dozens of other
show commands.

Re: [rfa] clean up output of "info set" command.

[Michael Snyder]

Michael Snyder wrote:
> Pedro Alves wrote:
>> On Friday 11 February 2011 21:17:08, Tom Tromey wrote:
>>>>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:
>>> Pedro> Isn't there a property of the command we could check
>>> Pedro> instead of hardcoding specific command names?
>>>
>>> Michael> I'm open to suggestions.  The only property I can think of is that
>>> Michael> there is no corresponding entry in "setlist".  I could search
>>> Michael> setlist every time...
>>>
>>> You could stick a new flag on the command object.
>> Yeah.  You may even be able to set the flag from within
>> the add_setshow_... functions and friends.  The flag could
>> mean "this show command shows something that is settable
>> in some way" (or the reverse).
>>
>>> Or there is cmd_cfunc_eq, which is used for a similar purpose in some
>>> places.  I don't think this is super, but OTOH it isn't any worse than
>>> existing code.
>> I think we shouldn't allow ourselves to broadcast bad design
>> when it's easy not to.  The function Michael touched is within
>> gdb/cli/cli-setshow.c.  I'd prefer to keep this and the other
>> core command files clean of specific knowledge of 
>> any specific commands their clients register.
>>
>> I notice that "info set" is basically an alias of "show".
>> Does the change make sense in the context of "show",
>> or should "show" keep showing everything showable under
>> the show command?  "show foo|bar|..."
>>
> 
> They're synonyms.  But I still don't think that "show copying"
> is at all useful when presented in the middle of dozens of other
> show commands.

More specifically, "info set" is a synonym for "show" with no arguments.
However "info set" does not accept any arguments itself.  It's really a
pretty useless command, when you think about it.  I would have proposed
deleting it, but that would require modifying help.exp and default.exp,
and still wouldn't help the case of "show" with no arguments.

Re: [rfa] clean up output of "info set" command.

[Michael Snyder]

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

Pedro Alves wrote:
> On Friday 11 February 2011 21:17:08, Tom Tromey wrote:
>>>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:
>> Pedro> Isn't there a property of the command we could check
>> Pedro> instead of hardcoding specific command names?
>>
>> Michael> I'm open to suggestions.  The only property I can think of is that
>> Michael> there is no corresponding entry in "setlist".  I could search
>> Michael> setlist every time...
>>
>> You could stick a new flag on the command object.
> 
> Yeah.  You may even be able to set the flag from within
> the add_setshow_... functions and friends.  The flag could
> mean "this show command shows something that is settable
> in some way" (or the reverse).
> 
>> Or there is cmd_cfunc_eq, which is used for a similar purpose in some
>> places.  I don't think this is super, but OTOH it isn't any worse than
>> existing code.
> 
> I think we shouldn't allow ourselves to broadcast bad design
> when it's easy not to.  The function Michael touched is within
> gdb/cli/cli-setshow.c.  I'd prefer to keep this and the other
> core command files clean of specific knowledge of 
> any specific commands their clients register.

OK, here's a new implementation in which I use the "class" field
to flag the offending show commands.

Better?


[-- Attachment #2: no_set.txt --]
[-- Type: text/plain, Size: 4918 bytes --]

2011-02-10  Michael Snyder  <msnyder@msnyder-server.eng.vmware.com>

	* command.h (enum command_class): New class 'no_set_class', for
	"show" commands without a corresponding "set" command.
	* value.c (_initialize_values): Use 'no_set_class' for "show values".
	* copying.c (_initialize_copying): Ditto for "show copying" and
	"show warranty".
	* cli/cli-cmds.c (init_cli_cmds): Ditto for "show commands" and
	"show version".
	* cli/cli-setshow.c (cmd_show_list): Skip "show" commands for
	which there is no corresponding "set" command (eg. "show copying").

Index: command.h
===================================================================
RCS file: /cvs/src/src/gdb/command.h,v
retrieving revision 1.73
diff -u -p -u -p -r1.73 command.h
--- command.h	31 Jan 2011 16:52:34 -0000	1.73
+++ command.h	12 Feb 2011 01:22:59 -0000
@@ -34,7 +34,7 @@ enum command_class
   no_class = -1, class_run = 0, class_vars, class_stack, class_files,
   class_support, class_info, class_breakpoint, class_trace,
   class_alias, class_bookmark, class_obscure, class_maintenance,
-  class_pseudo, class_tui, class_user, class_xdb
+  class_pseudo, class_tui, class_user, class_xdb, no_set_class
 };
 
 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum
Index: value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.125
diff -u -p -u -p -r1.125 value.c
--- value.c	25 Jan 2011 15:18:35 -0000	1.125
+++ value.c	12 Feb 2011 01:22:59 -0000
@@ -2533,7 +2533,7 @@ A few convenience variables are given va
 \"$__\" holds the contents of the last address examined with \"x\"."),
 	   &showlist);
 
-  add_cmd ("values", no_class, show_values, _("\
+  add_cmd ("values", no_set_class, show_values, _("\
 Elements of value history around item number IDX (or last ten)."),
 	   &showlist);
 
Index: copying.c
===================================================================
RCS file: /cvs/src/src/gdb/copying.c,v
retrieving revision 1.6
diff -u -p -u -p -r1.6 copying.c
--- copying.c	23 Aug 2007 20:33:48 -0000	1.6
+++ copying.c	12 Feb 2011 01:22:59 -0000
@@ -649,10 +649,10 @@ show_warranty_command (char *ignore, int
 void
 _initialize_copying (void)
 {
-  add_cmd ("copying", no_class, show_copying_command,
+  add_cmd ("copying", no_set_class, show_copying_command,
 	   _("Conditions for redistributing copies of GDB."),
 	   &showlist);
-  add_cmd ("warranty", no_class, show_warranty_command,
+  add_cmd ("warranty", no_set_class, show_warranty_command,
 	   _("Various kinds of warranty you do not have."),
 	   &showlist);
 
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.110
diff -u -p -u -p -r1.110 cli-cmds.c
--- cli/cli-cmds.c	17 Jan 2011 16:50:42 -0000	1.110
+++ cli/cli-cmds.c	12 Feb 2011 01:22:59 -0000
@@ -1533,13 +1533,13 @@ Generic command for showing things about
   /* Another way to get at the same thing.  */
   add_info ("set", show_command, _("Show all GDB settings."));
 
-  add_cmd ("commands", no_class, show_commands, _("\
+  add_cmd ("commands", no_set_class, show_commands, _("\
 Show the history of commands you typed.\n\
 You can supply a command number to start with, or a `+' to start after\n\
 the previous command number shown."),
 	   &showlist);
 
-  add_cmd ("version", no_class, show_version,
+  add_cmd ("version", no_set_class, show_version,
 	   _("Show what version of GDB this is."), &showlist);
 
   add_com ("while", class_support, while_command, _("\
Index: cli/cli-setshow.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-setshow.c,v
retrieving revision 1.43
diff -u -p -u -p -r1.43 cli-setshow.c
--- cli/cli-setshow.c	6 Jan 2011 00:57:02 -0000	1.43
+++ cli/cli-setshow.c	12 Feb 2011 01:22:59 -0000
@@ -434,18 +434,21 @@ cmd_show_list (struct cmd_list_element *
 	}
       else
 	{
-	  struct cleanup *option_chain
-	    = make_cleanup_ui_out_tuple_begin_end (uiout, "option");
+	  if (list->class != no_set_class)
+	    {
+	      struct cleanup *option_chain
+		= make_cleanup_ui_out_tuple_begin_end (uiout, "option");
 
-	  ui_out_text (uiout, prefix);
-	  ui_out_field_string (uiout, "name", list->name);
-	  ui_out_text (uiout, ":  ");
-	  if (list->type == show_cmd)
-	    do_setshow_command ((char *) NULL, from_tty, list);
-	  else
-	    cmd_func (list, NULL, from_tty);
-          /* Close the tuple.  */
-	  do_cleanups (option_chain);
+	      ui_out_text (uiout, prefix);
+	      ui_out_field_string (uiout, "name", list->name);
+	      ui_out_text (uiout, ":  ");
+	      if (list->type == show_cmd)
+		do_setshow_command ((char *) NULL, from_tty, list);
+	      else
+		cmd_func (list, NULL, from_tty);
+	      /* Close the tuple.  */
+	      do_cleanups (option_chain);
+	    }
 	}
     }
   /* Close the tuple.  */

Re: [rfa] clean up output of "info set" command.

[Eli Zaretskii]

> Date: Fri, 11 Feb 2011 17:30:31 -0800
> From: Michael Snyder <msnyder@vmware.com>
> CC: Tom Tromey <tromey@redhat.com>,  "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> -  class_pseudo, class_tui, class_user, class_xdb
> +  class_pseudo, class_tui, class_user, class_xdb, no_set_class
>  };

I think it would be good to have a comment here explaining what is
no_set_class used for.

Re: [rfa] clean up output of "info set" command.

[Michael Snyder]

Eli Zaretskii wrote:
>> Date: Fri, 11 Feb 2011 17:30:31 -0800
>> From: Michael Snyder <msnyder@vmware.com>
>> CC: Tom Tromey <tromey@redhat.com>,  "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>>
>> -  class_pseudo, class_tui, class_user, class_xdb
>> +  class_pseudo, class_tui, class_user, class_xdb, no_set_class
>>  };
> 
> I think it would be good to have a comment here explaining what is
> no_set_class used for.

Comment added, and committed.