[3/4] gdb: add 'maint set/show gnu-source-highlight enabled' command

Message ID 08bd8ccb9af26fead9b0ed9d2fb39bb003cb9e23.1641565040.git.aburgess@redhat.com
State New
Headers show
Series
  • Source highlight non utf-8 characters using Python
Related show

Commit Message

Simon Marchi via Gdb-patches Jan. 7, 2022, 2:23 p.m.
In a later commit I want to address an issue with the Python pygments
based code styling solution.  As this approach is only used when the
GNU Source Highlight library is not available, testing bugs in this
area can be annoying, as it requires GDB to be rebuilt with use of GNU
Source Highlight disabled.

This commit adds a pair of new maintenance commands:

  maintenance set gnu-source-highlight enabled on|off
  maintenance show gnu-source-highlight enabled

these commands can be used to disable use of the GNU Source Highlight
library, allowing me, in a later commit, to easily test bugs that
would otherwise be masked by GNU Source Highlight being used.

I made this a maintenance command, rather than a general purpose
command, as it didn't seem like this was something a general user
would need to adjust.  We can always convert the maintenance command
to a general command later if needed.

There's no test for this here, but this feature will be used in a
later commit.
---
 gdb/NEWS            |  8 +++++
 gdb/doc/gdb.texinfo | 15 ++++++++++
 gdb/source-cache.c  | 72 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 94 insertions(+), 1 deletion(-)

-- 
2.25.4

Comments

Simon Marchi via Gdb-patches Jan. 7, 2022, 2:53 p.m. | #1
> Date: Fri,  7 Jan 2022 14:23:13 +0000

> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>

> Cc: Andrew Burgess <aburgess@redhat.com>

> 

> +maint set gnu-source-highlight enabled on|off

> +maint show gnu-source-highlight enabled

> +  When on GDB will use the GNU Source Highlight library for adding


"Whether GDB should use the GNU Source Highlight ..."

> +  styling to source code.  When off the library will not be used, even

                                      ^
Comma missing there.

> +  when available.  When GNU Source Highlight isn't used, or can't add

> +  styling to a particular source file, then the Python Pygments

> +  library will be used instead.

> +@kindex maint set gnu-source-highlight enabled

> +@kindex maint show gnu-source-highlight enabled

> +@item maint set gnu-source-highlight enabled @r{[}on|off@r{]}

> +@itemx maint show gnu-source-highlight enabled

> +Control whether @value{GDBN} should use the GNU Source Highlight

> +library for applying styling to source code (@pxref{Output Styling}).

> +This will be @samp{on} by default if the GNU Source Highlight library

> +is available.  If the GNU Source Highlight library is not available,

> +then this will be @samp{off} by default, and attempting to change this

> +value to @samp{on} will give an error.

> +

> +If the GNU Source Highlight library is not being used, then

> +@value{GDBN} will use the Python Pygments package for source code

> +styling, if it is available.


Here, too, I would suggest saying that this is useful for debugging
Pygments styling when Source Highlight is available.

> +  add_setshow_boolean_cmd ("enabled", class_maintenance,

> +			   &use_gnu_source_highlight, _("\

> +Set whether the GNU Source Highlight library can be used."), _("\

                                                ^^^^^^^^^^^
I think "should be used" is better.

Thanks.
Tom Tromey Jan. 10, 2022, 3:58 p.m. | #2
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:


Andrew> In a later commit I want to address an issue with the Python pygments
Andrew> based code styling solution.  As this approach is only used when the
Andrew> GNU Source Highlight library is not available, testing bugs in this
Andrew> area can be annoying, as it requires GDB to be rebuilt with use of GNU
Andrew> Source Highlight disabled.

Andrew> This commit adds a pair of new maintenance commands:

Andrew>   maintenance set gnu-source-highlight enabled on|off
Andrew>   maintenance show gnu-source-highlight enabled

Looks good, thank you.

Tom
Simon Marchi via Gdb-patches Jan. 11, 2022, 1:07 p.m. | #3
* Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> [2022-01-07 16:53:29 +0200]:

> > Date: Fri,  7 Jan 2022 14:23:13 +0000

> > From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>

> > Cc: Andrew Burgess <aburgess@redhat.com>

> > 

> > +maint set gnu-source-highlight enabled on|off

> > +maint show gnu-source-highlight enabled

> > +  When on GDB will use the GNU Source Highlight library for adding

> 

> "Whether GDB should use the GNU Source Highlight ..."

> 

> > +  styling to source code.  When off the library will not be used, even

>                                       ^

> Comma missing there.

> 

> > +  when available.  When GNU Source Highlight isn't used, or can't add

> > +  styling to a particular source file, then the Python Pygments

> > +  library will be used instead.

> > +@kindex maint set gnu-source-highlight enabled

> > +@kindex maint show gnu-source-highlight enabled

> > +@item maint set gnu-source-highlight enabled @r{[}on|off@r{]}

> > +@itemx maint show gnu-source-highlight enabled

> > +Control whether @value{GDBN} should use the GNU Source Highlight

> > +library for applying styling to source code (@pxref{Output Styling}).

> > +This will be @samp{on} by default if the GNU Source Highlight library

> > +is available.  If the GNU Source Highlight library is not available,

> > +then this will be @samp{off} by default, and attempting to change this

> > +value to @samp{on} will give an error.

> > +

> > +If the GNU Source Highlight library is not being used, then

> > +@value{GDBN} will use the Python Pygments package for source code

> > +styling, if it is available.

> 

> Here, too, I would suggest saying that this is useful for debugging

> Pygments styling when Source Highlight is available.


Thanks for the feedback.  I've made the fixes you suggested.  For the
manual entry the full text is now this:

  @kindex maint set gnu-source-highlight enabled
  @kindex maint show gnu-source-highlight enabled
  @item maint set gnu-source-highlight enabled @r{[}on|off@r{]}
  @itemx maint show gnu-source-highlight enabled
  Control whether @value{GDBN} should use the GNU Source Highlight
  library for applying styling to source code (@pxref{Output Styling}).
  This will be @samp{on} by default if the GNU Source Highlight library
  is available.  If the GNU Source Highlight library is not available,
  then this will be @samp{off} by default, and attempting to change this
  value to @samp{on} will give an error.
  
  If the GNU Source Highlight library is not being used, then
  @value{GDBN} will use the Python Pygments package for source code
  styling, if it is available.
  
  This option is useful for debugging @value{GDBN}'s use of the Pygments
  library when @value{GDBN} is linked against the GNU Source Highlight
  library.

Is this better?

The full patch, including this change is below.

Thanks,
Andrew

---

commit 8346698512836d313cfe91581f2ba43be359090a
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Fri Nov 26 15:13:43 2021 +0000

    gdb: add 'maint set/show gnu-source-highlight enabled' command
    
    In a later commit I want to address an issue with the Python pygments
    based code styling solution.  As this approach is only used when the
    GNU Source Highlight library is not available, testing bugs in this
    area can be annoying, as it requires GDB to be rebuilt with use of GNU
    Source Highlight disabled.
    
    This commit adds a pair of new maintenance commands:
    
      maintenance set gnu-source-highlight enabled on|off
      maintenance show gnu-source-highlight enabled
    
    these commands can be used to disable use of the GNU Source Highlight
    library, allowing me, in a later commit, to easily test bugs that
    would otherwise be masked by GNU Source Highlight being used.
    
    I made this a maintenance command, rather than a general purpose
    command, as it didn't seem like this was something a general user
    would need to adjust.  We can always convert the maintenance command
    to a general command later if needed.
    
    There's no test for this here, but this feature will be used in a
    later commit.

diff --git a/gdb/NEWS b/gdb/NEWS
index 105fcf74ff4..9e8e173c0d8 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -67,6 +67,14 @@ show debug threads
 maint flush source-cache
   Flush the contents of the source code cache.
 
+maint set gnu-source-highlight enabled on|off
+maint show gnu-source-highlight enabled
+  Whether GDB should use the GNU Source Highlight library for adding
+  styling to source code.  When off, the library will not be used, even
+  when available.  When GNU Source Highlight isn't used, or can't add
+  styling to a particular source file, then the Python Pygments
+  library will be used instead.
+
 * Changed commands
 
 maint packet
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 222a01023e0..4ed1302302e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39797,6 +39797,25 @@
 described in @ref{set libthread-db-search-path}.  For more information
 about the tests, see @ref{maint check libthread-db}.
 
+@kindex maint set gnu-source-highlight enabled
+@kindex maint show gnu-source-highlight enabled
+@item maint set gnu-source-highlight enabled @r{[}on|off@r{]}
+@itemx maint show gnu-source-highlight enabled
+Control whether @value{GDBN} should use the GNU Source Highlight
+library for applying styling to source code (@pxref{Output Styling}).
+This will be @samp{on} by default if the GNU Source Highlight library
+is available.  If the GNU Source Highlight library is not available,
+then this will be @samp{off} by default, and attempting to change this
+value to @samp{on} will give an error.
+
+If the GNU Source Highlight library is not being used, then
+@value{GDBN} will use the Python Pygments package for source code
+styling, if it is available.
+
+This option is useful for debugging @value{GDBN}'s use of the Pygments
+library when @value{GDBN} is linked against the GNU Source Highlight
+library.
+
 @kindex maint space
 @cindex memory used by commands
 @item maint space @var{value}
diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index 41fcda267b3..608738679e8 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -47,6 +47,46 @@
 
 source_cache g_source_cache;
 
+/* When this is true we will use the GNU Source Highlight to add styling to
+   source code (assuming the library is available).  This is initialized to
+   true (if appropriate) in _initialize_source_cache below.  */
+
+static bool use_gnu_source_highlight;
+
+/* The "maint show gnu-source-highlight enabled" command. */
+
+static void
+show_use_gnu_source_highlight_enabled  (struct ui_file *file, int from_tty,
+					struct cmd_list_element *c,
+					const char *value)
+{
+  fprintf_filtered (file,
+		    _("Use of GNU Source Highlight library is \"%s\".\n"),
+		    value);
+}
+
+/* The "maint set gnu-source-highlight enabled" command.  */
+
+static void
+set_use_gnu_source_highlight_enabled (const char *ignore_args,
+				      int from_tty,
+				      struct cmd_list_element *c)
+{
+#ifndef HAVE_SOURCE_HIGHLIGHT
+  /* If the library is not available and the user tried to enable use of
+     the library, then disable use of the library, and give an error.  */
+  if (use_gnu_source_highlight)
+    {
+      use_gnu_source_highlight = false;
+      error (_("the GNU Source Highlight library is not available"));
+    }
+#else
+  /* We (might) have just changed how we style source code, discard any
+     previously cached contents.  */
+  forget_cached_source_info ();
+#endif
+}
+
 /* See source-cache.h.  */
 
 std::string
@@ -193,7 +233,7 @@ source_cache::ensure (struct symtab *s)
 #ifdef HAVE_SOURCE_HIGHLIGHT
       bool already_styled = false;
       const char *lang_name = get_language_name (SYMTAB_LANGUAGE (s));
-      if (lang_name != nullptr)
+      if (lang_name != nullptr && use_gnu_source_highlight)
 	{
 	  /* The global source highlight object, or null if one was
 	     never constructed.  This is stored here rather than in
@@ -364,6 +404,36 @@ _initialize_source_cache ()
 	   _("Force gdb to flush its source code cache."),
 	   &maintenanceflushlist);
 
+  /* All the 'maint set|show gnu-source-highlight' sub-commands.  */
+  static struct cmd_list_element *maint_set_gnu_source_highlight_cmdlist;
+  static struct cmd_list_element *maint_show_gnu_source_highlight_cmdlist;
+
+  /* Adds 'maint set|show gnu-source-highlight'.  */
+  add_setshow_prefix_cmd ("gnu-source-highlight", class_maintenance,
+			  _("Set gnu-source-highlight specific variables."),
+			  _("Show gnu-source-highlight specific variables."),
+			  &maint_set_gnu_source_highlight_cmdlist,
+			  &maint_show_gnu_source_highlight_cmdlist,
+			  &maintenance_set_cmdlist,
+			  &maintenance_show_cmdlist);
+
+  /* Adds 'maint set|show gnu-source-highlight enabled'.  */
+  add_setshow_boolean_cmd ("enabled", class_maintenance,
+			   &use_gnu_source_highlight, _("\
+Set whether the GNU Source Highlight library should be used."), _("\
+Show whether the GNU Source Highlight library is being used."),_("\
+When enabled, GDB will use the GNU Source Highlight library to apply\n\
+styling to source code lines that are shown."),
+			   set_use_gnu_source_highlight_enabled,
+			   show_use_gnu_source_highlight_enabled,
+			   &maint_set_gnu_source_highlight_cmdlist,
+			   &maint_show_gnu_source_highlight_cmdlist);
+
+  /* Enable use of GNU Source Highlight library, if we have it.  */
+#ifdef HAVE_SOURCE_HIGHLIGHT
+  use_gnu_source_highlight = true;
+#endif
+
 #if GDB_SELF_TEST
   selftests::register_test ("source-cache", selftests::extract_lines_test);
 #endif
Simon Marchi via Gdb-patches Jan. 11, 2022, 1:34 p.m. | #4
> Date: Tue, 11 Jan 2022 13:07:44 +0000

> From: Andrew Burgess <aburgess@redhat.com>

> Cc: gdb-patches@sourceware.org

> 

>   @kindex maint set gnu-source-highlight enabled

>   @kindex maint show gnu-source-highlight enabled

>   @item maint set gnu-source-highlight enabled @r{[}on|off@r{]}

>   @itemx maint show gnu-source-highlight enabled

>   Control whether @value{GDBN} should use the GNU Source Highlight

>   library for applying styling to source code (@pxref{Output Styling}).

>   This will be @samp{on} by default if the GNU Source Highlight library

>   is available.  If the GNU Source Highlight library is not available,

>   then this will be @samp{off} by default, and attempting to change this

>   value to @samp{on} will give an error.

>   

>   If the GNU Source Highlight library is not being used, then

>   @value{GDBN} will use the Python Pygments package for source code

>   styling, if it is available.

>   

>   This option is useful for debugging @value{GDBN}'s use of the Pygments

>   library when @value{GDBN} is linked against the GNU Source Highlight

>   library.

> 

> Is this better?


Yes, thanks.
Simon Marchi via Gdb-patches Jan. 12, 2022, 11:37 a.m. | #5
* Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> [2022-01-11 15:34:12 +0200]:

> > Date: Tue, 11 Jan 2022 13:07:44 +0000

> > From: Andrew Burgess <aburgess@redhat.com>

> > Cc: gdb-patches@sourceware.org

> > 

> >   @kindex maint set gnu-source-highlight enabled

> >   @kindex maint show gnu-source-highlight enabled

> >   @item maint set gnu-source-highlight enabled @r{[}on|off@r{]}

> >   @itemx maint show gnu-source-highlight enabled

> >   Control whether @value{GDBN} should use the GNU Source Highlight

> >   library for applying styling to source code (@pxref{Output Styling}).

> >   This will be @samp{on} by default if the GNU Source Highlight library

> >   is available.  If the GNU Source Highlight library is not available,

> >   then this will be @samp{off} by default, and attempting to change this

> >   value to @samp{on} will give an error.

> >   

> >   If the GNU Source Highlight library is not being used, then

> >   @value{GDBN} will use the Python Pygments package for source code

> >   styling, if it is available.

> >   

> >   This option is useful for debugging @value{GDBN}'s use of the Pygments

> >   library when @value{GDBN} is linked against the GNU Source Highlight

> >   library.

> > 

> > Is this better?

> 

> Yes, thanks.


Thanks, I pushed this patch.

Andrew

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 105fcf74ff4..d968b0cde44 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -67,6 +67,14 @@  show debug threads
 maint flush source-cache
   Flush the contents of the source code cache.
 
+maint set gnu-source-highlight enabled on|off
+maint show gnu-source-highlight enabled
+  When on GDB will use the GNU Source Highlight library for adding
+  styling to source code.  When off the library will not be used, even
+  when available.  When GNU Source Highlight isn't used, or can't add
+  styling to a particular source file, then the Python Pygments
+  library will be used instead.
+
 * Changed commands
 
 maint packet
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e17149afa2a..4b7430fb60a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39793,6 +39793,21 @@ 
 described in @ref{set libthread-db-search-path}.  For more information
 about the tests, see @ref{maint check libthread-db}.
 
+@kindex maint set gnu-source-highlight enabled
+@kindex maint show gnu-source-highlight enabled
+@item maint set gnu-source-highlight enabled @r{[}on|off@r{]}
+@itemx maint show gnu-source-highlight enabled
+Control whether @value{GDBN} should use the GNU Source Highlight
+library for applying styling to source code (@pxref{Output Styling}).
+This will be @samp{on} by default if the GNU Source Highlight library
+is available.  If the GNU Source Highlight library is not available,
+then this will be @samp{off} by default, and attempting to change this
+value to @samp{on} will give an error.
+
+If the GNU Source Highlight library is not being used, then
+@value{GDBN} will use the Python Pygments package for source code
+styling, if it is available.
+
 @kindex maint space
 @cindex memory used by commands
 @item maint space @var{value}
diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index 9a5ac40497a..d64ddd3cc61 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -47,6 +47,46 @@ 
 
 source_cache g_source_cache;
 
+/* When this is true we will use the GNU Source Highlight to add styling to
+   source code (assuming the library is available).  This is initialized to
+   true (if appropriate) in _initialize_source_cache below.  */
+
+static bool use_gnu_source_highlight;
+
+/* The "maint show gnu-source-highlight enabled" command. */
+
+static void
+show_use_gnu_source_highlight_enabled  (struct ui_file *file, int from_tty,
+					struct cmd_list_element *c,
+					const char *value)
+{
+  fprintf_filtered (file,
+		    _("Use of GNU Source Highlight library is \"%s\".\n"),
+		    value);
+}
+
+/* The "maint set gnu-source-highlight enabled" command.  */
+
+static void
+set_use_gnu_source_highlight_enabled (const char *ignore_args,
+				      int from_tty,
+				      struct cmd_list_element *c)
+{
+#ifndef HAVE_SOURCE_HIGHLIGHT
+  /* If the library is not available and the user tried to enable use of
+     the library, then disable use of the library, and give an error.  */
+  if (use_gnu_source_highlight)
+    {
+      use_gnu_source_highlight = false;
+      error (_("the GNU Source Highlight library is not available"));
+    }
+#else
+  /* We (might) have just changed how we style source code, discard any
+     previously cached contents.  */
+  forget_cached_source_info ();
+#endif
+}
+
 /* See source-cache.h.  */
 
 std::string
@@ -193,7 +233,7 @@  source_cache::ensure (struct symtab *s)
 #ifdef HAVE_SOURCE_HIGHLIGHT
       bool already_styled = false;
       const char *lang_name = get_language_name (SYMTAB_LANGUAGE (s));
-      if (lang_name != nullptr)
+      if (lang_name != nullptr && use_gnu_source_highlight)
 	{
 	  /* The global source highlight object, or null if one was
 	     never constructed.  This is stored here rather than in
@@ -365,6 +405,36 @@  _initialize_source_cache ()
 	   _("Force gdb to flush its source code cache."),
 	   &maintenanceflushlist);
 
+  /* All the 'maint set|show gnu-source-highlight' sub-commands.  */
+  static struct cmd_list_element *maint_set_gnu_source_highlight_cmdlist;
+  static struct cmd_list_element *maint_show_gnu_source_highlight_cmdlist;
+
+  /* Adds 'maint set|show gnu-source-highlight'.  */
+  add_setshow_prefix_cmd ("gnu-source-highlight", class_maintenance,
+			  _("Set gnu-source-highlight specific variables."),
+			  _("Show gnu-source-highlight specific variables."),
+			  &maint_set_gnu_source_highlight_cmdlist,
+			  &maint_show_gnu_source_highlight_cmdlist,
+			  &maintenance_set_cmdlist,
+			  &maintenance_show_cmdlist);
+
+  /* Adds 'maint set|show gnu-source-highlight enabled'.  */
+  add_setshow_boolean_cmd ("enabled", class_maintenance,
+			   &use_gnu_source_highlight, _("\
+Set whether the GNU Source Highlight library can be used."), _("\
+Show whether the GNU Source Highlight library is being used."),_("\
+When enabled, GDB will use the GNU Source Highlight library to apply\n\
+styling to source code lines that are shown."),
+			   set_use_gnu_source_highlight_enabled,
+			   show_use_gnu_source_highlight_enabled,
+			   &maint_set_gnu_source_highlight_cmdlist,
+			   &maint_show_gnu_source_highlight_cmdlist);
+
+  /* Enable use of GNU Source Highlight library, if we have it.  */
+#ifdef HAVE_SOURCE_HIGHLIGHT
+  use_gnu_source_highlight = true;
+#endif
+
 #if GDB_SELF_TEST
   selftests::register_test ("source-cache", selftests::extract_lines_test);
 #endif