public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Subject: [PATCH 2/4] [gdb/cli] Factor out try_source_highlight
Date: Thu, 12 Oct 2023 16:47:05 +0200	[thread overview]
Message-ID: <20231012144707.13931-2-tdevries@suse.de> (raw)
In-Reply-To: <20231012144707.13931-1-tdevries@suse.de>

Function source_cache::ensure contains some code using the GNU
source-highlight library.

The code is a sizable chunk of the function, and contains conditional
compilation in a slightly convoluted way:
...
       if (!already_styled)
 #endif /* HAVE_SOURCE_HIGHLIGHT */
       {
...

Fix this by factoring out the code into new function try_source_highlight,
such that:
- source_cache::ensure is easier to read, and
- the conditional compilation is at the level of the function body.

Tested on x86_64-linux.
---
 gdb/source-cache.c | 93 +++++++++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 39 deletions(-)

diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index 7ef54411c69..186a4f73256 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -191,6 +191,58 @@ get_language_name (enum language lang)
 
 #endif /* HAVE_SOURCE_HIGHLIGHT */
 
+/* Try to highlight CONTENTS from file FULLNAME in language LANG_NAME using
+   the GNU source-higlight library.  Return true if highlighting
+   succeeded.  */
+
+static bool
+try_source_highlight (std::string &contents, const char *lang_name,
+		      const std::string &fullname)
+{
+#ifdef HAVE_SOURCE_HIGHLIGHT
+  if (lang_name == nullptr || !use_gnu_source_highlight)
+    return false;
+
+  /* The global source highlight object, or null if one was
+     never constructed.  This is stored here rather than in
+     the class so that we don't need to include anything or do
+     conditional compilation in source-cache.h.  */
+  static srchilite::SourceHighlight *highlighter;
+
+  bool styled = false;
+  try
+    {
+      if (highlighter == nullptr)
+	{
+	  highlighter = new srchilite::SourceHighlight ("esc.outlang");
+	  highlighter->setStyleFile ("esc.style");
+	}
+
+      std::istringstream input (contents);
+      std::ostringstream output;
+      highlighter->highlight (input, output, lang_name, fullname);
+#if __cplusplus >= 202002L
+      contents = std::move (output).str();
+#else
+      contents = output.str ();
+#endif
+      styled = true;
+    }
+  catch (...)
+    {
+      /* Source Highlight will throw an exception if
+	 highlighting fails.  One possible reason it can fail
+	 is if the language is unknown -- which matters to gdb
+	 because Rust support wasn't added until after 3.1.8.
+	 Ignore exceptions here. */
+    }
+
+  return styled;
+#else
+  return false;
+#endif /* HAVE_SOURCE_HIGHLIGHT */
+}
+
 /* See source-cache.h.  */
 
 bool
@@ -230,48 +282,11 @@ source_cache::ensure (struct symtab *s)
 
   if (source_styling && gdb_stdout->can_emit_style_escape ())
     {
-#ifdef HAVE_SOURCE_HIGHLIGHT
-      bool already_styled = false;
       const char *lang_name = get_language_name (s->language ());
-      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
-	     the class so that we don't need to include anything or do
-	     conditional compilation in source-cache.h.  */
-	  static srchilite::SourceHighlight *highlighter;
-
-	  try
-	    {
-	      if (highlighter == nullptr)
-		{
-		  highlighter = new srchilite::SourceHighlight ("esc.outlang");
-		  highlighter->setStyleFile ("esc.style");
-		}
-
-	      std::istringstream input (contents);
-	      std::ostringstream output;
-	      highlighter->highlight (input, output, lang_name, fullname);
-#if __cplusplus >= 202002L
-	      contents = std::move (output).str();
-#else
-	      contents = output.str ();
-#endif
-	      already_styled = true;
-	    }
-	  catch (...)
-	    {
-	      /* Source Highlight will throw an exception if
-		 highlighting fails.  One possible reason it can fail
-		 is if the language is unknown -- which matters to gdb
-		 because Rust support wasn't added until after 3.1.8.
-		 Ignore exceptions here and fall back to
-		 un-highlighted text. */
-	    }
-	}
+      bool already_styled
+	= try_source_highlight (contents, lang_name, fullname);
 
       if (!already_styled)
-#endif /* HAVE_SOURCE_HIGHLIGHT */
 	{
 	  gdb::optional<std::string> ext_contents;
 	  ext_contents = ext_lang_colorize (fullname, contents);
-- 
2.35.3


  reply	other threads:[~2023-10-12 14:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-12 14:47 [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries
2023-10-12 14:47 ` Tom de Vries [this message]
2023-10-12 14:47 ` [PATCH 3/4] [gdb/cli] Keep track of styling failures in source_cache Tom de Vries
2023-10-12 14:47 ` [PATCH 4/4] [gdb/cli] Allow source highlighting to be interrupted Tom de Vries
2023-10-13 12:21 ` [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231012144707.13931-2-tdevries@suse.de \
    --to=tdevries@suse.de \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).