diff mbox

[RFA] Fix 'help set/show style' strange layouts/results.

Message ID 20181229155116.6603-1-philippe.waroquiers@skynet.be
State New, archived
Headers

Commit Message

Philippe Waroquiers Dec. 29, 2018, 3:51 p.m. UTC 
  The layout for 'help set address|variable' is strange, e.g.:
  (gdb) help set style address
  style address

  List of show Address display styling
  Configure address colors and display intensity subcommands:

  show Address display styling
  Configure address colors and display intensity background -- Set the background color for this property
  show Address display styling
  Configure address colors and display intensity foreground -- Set the foreground color for this property
  show Address display styling
  Configure address colors and display intensity intensity -- Set the display intensity color for this property

  Type "help show Address display styling
  Configure address colors and display intensity" followed by show Address display styling
  Configure address colors and display intensity subcommand name for full documentation.
  Type "apropos word" to search for commands related to "word".
  Command name abbreviations are allowed if unambiguous.
  (gdb)

The help for 'set style function|filename' gives help for 'Show':
  (gdb) help set style filename
  Filename display styling
  Configure filename colors and display intensity.

  List of show style filename subcommands:

  show style filename background -- Set the background color for this property
  show style filename foreground -- Set the foreground color for this property
  show style filename intensity -- Set the display intensity color for this property

The help for 'show style function|filename' is equally strange, as it speaks
about commands, instead of sub commands:
  (gdb) help show style filename
  Filename display styling
  Configure filename colors and display intensity.

  List of commands:

  background -- Show the background color for this property
  foreground -- Show the foreground color for this property
  intensity -- Show the display intensity color for this property

  Type "help" followed by command name for full documentation.
  Type "apropos word" to search for commands related to "word".
  Command name abbreviations are allowed if unambiguous.
  (gdb)

This patch fixes all this.

Note that the 'set style' and 'show style' have the same prefix_doc:
  (gdb) help show style
  Style-specific settings
  Configure various style-related variables, such as colors
  ...
  (gdb) help set style
  Style-specific settings
  Configure various style-related variables, such as colors
  ...

Other similar commands (such as set|show history) have typically
a more specific prefix:
  (gdb) help show history
  Generic command for showing command history parameters.
  ...
  (gdb) help set history
  Generic command for setting command history parameters.
  ...

This could be fixed by having set_prefix_doc and show_prefix_doc instead of
the single prefix_doc argument to cli_style_option::add_setshow_commands.
That could be improved if deemed better.

2018-12-29  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli/cli-style.c (cli_style_option::add_setshow_commands):
	Initialize m_set_prefix with "set", instead of re-assigning
	m_show_prefix.  Use m_set_prefix for set_list and m_show_prefix
	for show_list.
	(_initialize_cli_style): Correct the order of arguments in
	variable_name_style.add_setshow_commands and
	address_style.add_setshow_commands calls.
---
 gdb/cli/cli-style.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Philippe Waroquiers Dec. 29, 2018, 4:58 p.m. UTC | #1 
Note that this also seems to fix a crash when doing

(gdb) info set
ada print-signatures:  Whether the output of formal and return types for functions in the
overloads selection menu is activated is on.
...
struct-convention:  The convention for returning small structs is "default".
Segmentation fault

(the set style follows set struct-convention).

Philippe

On Sat, 2018-12-29 at 16:51 +0100, Philippe Waroquiers wrote:
> The layout for 'help set address|variable' is strange, e.g.:
>   (gdb) help set style address
>   style address
> 
>   List of show Address display styling
>   Configure address colors and display intensity subcommands:
> 
>   show Address display styling
>   Configure address colors and display intensity background -- Set the background color for this property
>   show Address display styling
>   Configure address colors and display intensity foreground -- Set the foreground color for this property
>   show Address display styling
>   Configure address colors and display intensity intensity -- Set the display intensity color for this property
> 
>   Type "help show Address display styling
>   Configure address colors and display intensity" followed by show Address display styling
>   Configure address colors and display intensity subcommand name for full documentation.
>   Type "apropos word" to search for commands related to "word".
>   Command name abbreviations are allowed if unambiguous.
>   (gdb)
> 
> The help for 'set style function|filename' gives help for 'Show':
>   (gdb) help set style filename
>   Filename display styling
>   Configure filename colors and display intensity.
> 
>   List of show style filename subcommands:
> 
>   show style filename background -- Set the background color for this property
>   show style filename foreground -- Set the foreground color for this property
>   show style filename intensity -- Set the display intensity color for this property
> 
> The help for 'show style function|filename' is equally strange, as it speaks
> about commands, instead of sub commands:
>   (gdb) help show style filename
>   Filename display styling
>   Configure filename colors and display intensity.
> 
>   List of commands:
> 
>   background -- Show the background color for this property
>   foreground -- Show the foreground color for this property
>   intensity -- Show the display intensity color for this property
> 
>   Type "help" followed by command name for full documentation.
>   Type "apropos word" to search for commands related to "word".
>   Command name abbreviations are allowed if unambiguous.
>   (gdb)
> 
> This patch fixes all this.
> 
> Note that the 'set style' and 'show style' have the same prefix_doc:
>   (gdb) help show style
>   Style-specific settings
>   Configure various style-related variables, such as colors
>   ...
>   (gdb) help set style
>   Style-specific settings
>   Configure various style-related variables, such as colors
>   ...
> 
> Other similar commands (such as set|show history) have typically
> a more specific prefix:
>   (gdb) help show history
>   Generic command for showing command history parameters.
>   ...
>   (gdb) help set history
>   Generic command for setting command history parameters.
>   ...
> 
> This could be fixed by having set_prefix_doc and show_prefix_doc instead of
> the single prefix_doc argument to cli_style_option::add_setshow_commands.
> That could be improved if deemed better.
> 
> 2018-12-29  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* cli/cli-style.c (cli_style_option::add_setshow_commands):
> 	Initialize m_set_prefix with "set", instead of re-assigning
> 	m_show_prefix.  Use m_set_prefix for set_list and m_show_prefix
> 	for show_list.
> 	(_initialize_cli_style): Correct the order of arguments in
> 	variable_name_style.add_setshow_commands and
> 	address_style.add_setshow_commands calls.
> ---
>  gdb/cli/cli-style.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
> index 0308e1ccfc..13f02579bc 100644
> --- a/gdb/cli/cli-style.c
> +++ b/gdb/cli/cli-style.c
> @@ -172,13 +172,13 @@ cli_style_option::add_setshow_commands (const char *name,
>  					struct cmd_list_element **set_list,
>  					struct cmd_list_element **show_list)
>  {
> -  m_show_prefix = std::string ("set ") + prefixname + " ";
> +  m_set_prefix = std::string ("set ") + prefixname + " ";
>    m_show_prefix = std::string ("show ") + prefixname + " ";
>  
>    add_prefix_cmd (name, no_class, do_set, prefix_doc, &m_set_list,
> -		  m_show_prefix.c_str (), 0, set_list);
> +		  m_set_prefix.c_str (), 0, set_list);
>    add_prefix_cmd (name, no_class, do_show, prefix_doc, &m_show_list,
> -		  m_set_prefix.c_str (), 0, show_list);
> +		  m_show_prefix.c_str (), 0, show_list);
>  
>    add_setshow_enum_cmd ("foreground", theclass, cli_colors,
>  			&m_foreground,
> @@ -270,17 +270,17 @@ Configure function name colors and display intensity"),
>  					    &style_set_list,
>  					    &style_show_list);
>    variable_name_style.add_setshow_commands ("variable", no_class,
> -					    "style variable",
>  					    _("\
>  Variable name display styling\n\
>  Configure variable name colors and display intensity"),
> +					    "style variable",
>  					    &style_set_list,
>  					    &style_show_list);
>    address_style.add_setshow_commands ("address", no_class,
> -				      "style address",
>  				      _("\
>  Address display styling\n\
>  Configure address colors and display intensity"),
> +				      "style address",
>  				      &style_set_list,
>  				      &style_show_list);
>  }
Tom Tromey Dec. 29, 2018, 7:19 p.m. UTC | #2 
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> This could be fixed by having set_prefix_doc and show_prefix_doc instead of
Philippe> the single prefix_doc argument to cli_style_option::add_setshow_commands.
Philippe> That could be improved if deemed better.

Up to you.

Philippe> 2018-12-29  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* cli/cli-style.c (cli_style_option::add_setshow_commands):
Philippe> 	Initialize m_set_prefix with "set", instead of re-assigning
Philippe> 	m_show_prefix.  Use m_set_prefix for set_list and m_show_prefix
Philippe> 	for show_list.
Philippe> 	(_initialize_cli_style): Correct the order of arguments in
Philippe> 	variable_name_style.add_setshow_commands and
Philippe> 	address_style.add_setshow_commands calls.

This is ok.  Thanks for noticing and fixing this.

Tom
Philippe Waroquiers Dec. 30, 2018, 8:31 a.m. UTC | #3 
On Sat, 2018-12-29 at 12:19 -0700, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> This could be fixed by having set_prefix_doc and show_prefix_doc instead of
> Philippe> the single prefix_doc argument to cli_style_option::add_setshow_commands.
> Philippe> That could be improved if deemed better.
> 
> Up to you.
> 
> Philippe> 2018-12-29  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> Philippe> 	* cli/cli-style.c (cli_style_option::add_setshow_commands):
> Philippe> 	Initialize m_set_prefix with "set", instead of re-assigning
> Philippe> 	m_show_prefix.  Use m_set_prefix for set_list and m_show_prefix
> Philippe> 	for show_list.
> Philippe> 	(_initialize_cli_style): Correct the order of arguments in
> Philippe> 	variable_name_style.add_setshow_commands and
> Philippe> 	address_style.add_setshow_commands calls.
> 
> This is ok.  Thanks for noticing and fixing this.
Thanks for the review, pushed.
Philippe
diff mbox

Patch

diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index 0308e1ccfc..13f02579bc 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -172,13 +172,13 @@  cli_style_option::add_setshow_commands (const char *name,
 					struct cmd_list_element **set_list,
 					struct cmd_list_element **show_list)
 {
-  m_show_prefix = std::string ("set ") + prefixname + " ";
+  m_set_prefix = std::string ("set ") + prefixname + " ";
   m_show_prefix = std::string ("show ") + prefixname + " ";
 
   add_prefix_cmd (name, no_class, do_set, prefix_doc, &m_set_list,
-		  m_show_prefix.c_str (), 0, set_list);
+		  m_set_prefix.c_str (), 0, set_list);
   add_prefix_cmd (name, no_class, do_show, prefix_doc, &m_show_list,
-		  m_set_prefix.c_str (), 0, show_list);
+		  m_show_prefix.c_str (), 0, show_list);
 
   add_setshow_enum_cmd ("foreground", theclass, cli_colors,
 			&m_foreground,
@@ -270,17 +270,17 @@  Configure function name colors and display intensity"),
 					    &style_set_list,
 					    &style_show_list);
   variable_name_style.add_setshow_commands ("variable", no_class,
-					    "style variable",
 					    _("\
 Variable name display styling\n\
 Configure variable name colors and display intensity"),
+					    "style variable",
 					    &style_set_list,
 					    &style_show_list);
   address_style.add_setshow_commands ("address", no_class,
-				      "style address",
 				      _("\
 Address display styling\n\
 Configure address colors and display intensity"),
+				      "style address",
 				      &style_set_list,
 				      &style_show_list);
 }