diff mbox

[RFA] Implement help/show values for 'set|show style'.

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

Commit Message

Philippe Waroquiers Dec. 31, 2018, 1:42 p.m. UTC 
  Currently, the behaviour is:
  (gdb) show style
  (gdb) set style
  (gdb) show style address
  (gdb) set style address
  (gdb)

With this patch, the behaviour is:
  (gdb) show style
  style address background:  The "address" background color is: none
  style address foreground:  The "address" foreground color is: blue
  style address intensity:  The "address" display intensity is: normal
  enabled:  CLI output styling is enabled.
  style filename background:  The "filename" background color is: none
  style filename foreground:  The "filename" foreground color is: green
  style filename intensity:  The "filename" display intensity is: normal
  style function background:  The "function" background color is: none
  style function foreground:  The "function" foreground color is: yellow
  style function intensity:  The "function" display intensity is: normal
  style variable background:  The "variable" background color is: none
  style variable foreground:  The "variable" foreground color is: cyan
  style variable intensity:  The "variable" display intensity is: normal
  (gdb) set style
  "set style" must be followed by an appropriate subcommand.
  List of set style subcommands:

  set style address -- Address display styling
  set style enabled -- Set whether CLI styling is enabled
  set style filename -- Filename display styling
  set style function -- Function name display styling
  set style variable -- Variable name display styling

  Type "help set style" followed by set style subcommand name for full documentation.
  Type "apropos word" to search for commands related to "word".
  Command name abbreviations are allowed if unambiguous.
  (gdb) show style address
  background:  The "filename" background color is: none
  foreground:  The "filename" foreground color is: green
  intensity:  The "filename" display intensity is: normal
  (gdb) set style address
  List of set style address subcommands:

  set style address background -- Set the background color for this property
  set style address foreground -- Set the foreground color for this property
  set style address intensity -- Set the display intensity color for this property

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

gdb/ChangeLog
	* cli/cli-style.h (class cli_style_option): <add_setshow_commands>
	Remove arg prefixname, add do_set and do_show.
	Add member functions set_list and show_list.
	* cli/cli-style.c (class cli_style_option): Update accordingly.
	(style_set_list): Move to file scope.
	(style_show_list): Likewise.
	(set_style): Call help_list.
	(show_style): Call cmd_show_list.
	(_initialize_cli_style): New macro STYLE_ADD_SETSHOW_COMMANDS.
	Update to use the new macro.
---
 gdb/cli/cli-style.c | 93 +++++++++++++++++++++++----------------------
 gdb/cli/cli-style.h | 15 +++++---
 2 files changed, 57 insertions(+), 51 deletions(-)

Comments

Tom Tromey Jan. 12, 2019, 5:17 p.m. UTC | #1 
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> -Show the display intensity color for this property"),
Philippe> +			_("Show the display intensity color for this property"),

Not your problem, but would you mind removing the "color" both here and
from the "set" command help?  That's a copy-paste bug from my original
patch.

Philippe> +#define STYLE_ADD_SETSHOW_COMMANDS(style, name, prefix_doc)	  \

It's more normal in gdb to use upper case for macro parameters.

I was hoping to avoid macros here but, meh, I guess it doesn't really
matter much.

Philippe> +  /* Return the 'set style NAME' command list, that can be used
Philippe> +     to build a lambda DO_SET to call add_setshow_commands.  */
Philippe> +  struct cmd_list_element *set_list () { return m_set_list; };
Philippe> +
Philippe> +  /* Same as SET_LIST but for the show command list.  */
Philippe> +  struct cmd_list_element *show_list () { return m_show_list; };
Philippe>  private:

I think a blank line before the "private:" would be good.

Thanks for doing this.  This is ok with these changes.

Tom
Philippe Waroquiers Jan. 12, 2019, 9:13 p.m. UTC | #2 
On Sat, 2019-01-12 at 10:17 -0700, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> -Show the display intensity color for this property"),
> Philippe> +			_("Show the display intensity color for this property"),
> 
> Not your problem, but would you mind removing the "color" both here and
> from the "set" command help?  That's a copy-paste bug from my original
> patch.
> 
> Philippe> +#define STYLE_ADD_SETSHOW_COMMANDS(style, name, prefix_doc)	  \
> 
> It's more normal in gdb to use upper case for macro parameters.
> 
> I was hoping to avoid macros here but, meh, I guess it doesn't really
> matter much.
Yes, macros are not that nice.  My C++ knowledge being very minimal,
I did not directly see how to do that without macros.

> 
> Philippe> +  /* Return the 'set style NAME' command list, that can be used
> Philippe> +     to build a lambda DO_SET to call add_setshow_commands.  */
> Philippe> +  struct cmd_list_element *set_list () { return m_set_list; };
> Philippe> +
> Philippe> +  /* Same as SET_LIST but for the show command list.  */
> Philippe> +  struct cmd_list_element *show_list () { return m_show_list; };
> Philippe>  private:
> 
> I think a blank line before the "private:" would be good.
> 
> Thanks for doing this.  This is ok with these changes.
Pushed after doing the fixes above (macro params, fixing the 'color'
copy paste, and fixing another copy/paste error I did).

Thanks for the review

Philippe
diff mbox

Patch

diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index 13f02579bc..67bc55e201 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -114,20 +114,6 @@  cli_style_option::style () const
 
 /* See cli-style.h.  */
 
-void
-cli_style_option::do_set (const char *args, int from_tty)
-{
-}
-
-/* See cli-style.h.  */
-
-void
-cli_style_option::do_show (const char *args, int from_tty)
-{
-}
-
-/* See cli-style.h.  */
-
 void
 cli_style_option::do_show_foreground (struct ui_file *file, int from_tty,
 				      struct cmd_list_element *cmd,
@@ -168,12 +154,15 @@  void
 cli_style_option::add_setshow_commands (const char *name,
 					enum command_class theclass,
 					const char *prefix_doc,
-					const char *prefixname,
 					struct cmd_list_element **set_list,
-					struct cmd_list_element **show_list)
+					void (*do_set) (const char *args,
+							int from_tty),
+					struct cmd_list_element **show_list,
+					void (*do_show) (const char *args,
+							 int from_tty))
 {
-  m_set_prefix = std::string ("set ") + prefixname + " ";
-  m_show_prefix = std::string ("show ") + prefixname + " ";
+  m_set_prefix = std::string ("set style ") + name + " ";
+  m_show_prefix = std::string ("show style ") + name + " ";
 
   add_prefix_cmd (name, no_class, do_set, prefix_doc, &m_set_list,
 		  m_set_prefix.c_str (), 0, set_list);
@@ -199,22 +188,28 @@  cli_style_option::add_setshow_commands (const char *name,
   add_setshow_enum_cmd ("intensity", theclass, cli_intensities,
 			&m_intensity,
 			_("Set the display intensity color for this property"),
-			_("\
-Show the display intensity color for this property"),
+			_("Show the display intensity color for this property"),
 			nullptr,
 			nullptr,
 			do_show_intensity,
 			&m_set_list, &m_show_list, (void *) name);
 }
 
+static cmd_list_element *style_set_list;
+static cmd_list_element *style_show_list;
+
 static void
 set_style (const char *arg, int from_tty)
 {
+  printf_unfiltered (_("\"set style\" must be followed "
+		       "by an appropriate subcommand.\n"));
+  help_list (style_set_list, "set style ", all_commands, gdb_stdout);
 }
 
 static void
 show_style (const char *arg, int from_tty)
 {
+  cmd_show_list (style_show_list, from_tty, "");
 }
 
 static void
@@ -236,9 +231,6 @@  show_style_enabled (struct ui_file *file, int from_tty,
 void
 _initialize_cli_style ()
 {
-  static cmd_list_element *style_set_list;
-  static cmd_list_element *style_show_list;
-
   add_prefix_cmd ("style", no_class, set_style, _("\
 Style-specific settings\n\
 Configure various style-related variables, such as colors"),
@@ -255,32 +247,43 @@  If enabled, output to the terminal is styled."),
 			   set_style_enabled, show_style_enabled,
 			   &style_set_list, &style_show_list);
 
-  file_name_style.add_setshow_commands ("filename", no_class,
-					_("\
+#define STYLE_ADD_SETSHOW_COMMANDS(style, name, prefix_doc)	  \
+  style.add_setshow_commands (name, no_class, prefix_doc,		\
+			      &style_set_list,				\
+			      [] (const char *args, int from_tty)	\
+			      {						\
+				help_list				\
+				  (style.set_list (),			\
+				   "set style " name " ",		\
+				   all_commands,			\
+				   gdb_stdout);				\
+			      },					\
+			      &style_show_list,				\
+			      [] (const char *args, int from_tty)	\
+			      {						\
+				cmd_show_list				\
+				  (file_name_style.show_list (),	\
+				   from_tty,				\
+				   "");					\
+			      })
+
+  STYLE_ADD_SETSHOW_COMMANDS (file_name_style, "filename",
+			      _("\
 Filename display styling\n\
-Configure filename colors and display intensity."),
-					"style filename",
-					&style_set_list,
-					&style_show_list);
-  function_name_style.add_setshow_commands ("function", no_class,
-					    _("\
+Configure filename colors and display intensity."));
+
+  STYLE_ADD_SETSHOW_COMMANDS (function_name_style, "function",
+			      _("\
 Function name display styling\n\
-Configure function name colors and display intensity"),
-					    "style function",
-					    &style_set_list,
-					    &style_show_list);
-  variable_name_style.add_setshow_commands ("variable", no_class,
+Configure function name colors and display intensity"));
+
+  STYLE_ADD_SETSHOW_COMMANDS (variable_name_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,
+Configure variable name colors and display intensity"));
+
+  STYLE_ADD_SETSHOW_COMMANDS (address_style, "address",
 				      _("\
 Address display styling\n\
-Configure address colors and display intensity"),
-				      "style address",
-				      &style_set_list,
-				      &style_show_list);
+Configure address colors and display intensity"));
 }
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index e4af5a4929..e95b7b26dd 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -38,10 +38,17 @@  public:
   void add_setshow_commands (const char *name,
 			     enum command_class theclass,
 			     const char *prefix_doc,
-			     const char *prefixname,
 			     struct cmd_list_element **set_list,
-			     struct cmd_list_element **show_list);
+			     void (*do_set) (const char *args, int from_tty),
+			     struct cmd_list_element **show_list,
+			     void (*do_show) (const char *args, int from_tty));
 
+  /* Return the 'set style NAME' command list, that can be used
+     to build a lambda DO_SET to call add_setshow_commands.  */
+  struct cmd_list_element *set_list () { return m_set_list; };
+
+  /* Same as SET_LIST but for the show command list.  */
+  struct cmd_list_element *show_list () { return m_show_list; };
 private:
 
   /* The foreground.  */
@@ -59,10 +66,6 @@  private:
   struct cmd_list_element *m_set_list = nullptr;
   struct cmd_list_element *m_show_list = nullptr;
 
-  /* Callback to set a value.  */
-  static void do_set (const char *args, int from_tty);
-  /* Callback to show a value.  */
-  static void do_show (const char *args, int from_tty);
   /* Callback to show the foreground.  */
   static void do_show_foreground (struct ui_file *file, int from_tty,
 				  struct cmd_list_element *cmd,