[PATCH] [gdb/cli] Allow source-highlight to autodetect language

[Guinevere Larsen]

On 18/10/2023 18:56, Tom de Vries wrote:
> Currently when gdb asks the source-highlight library to highlight a file, it
> tells it what language file to use.
>
> For instance, if gdb learns from the debug info that the file is language_c,
> the language file "c.lang" is used.  This mapping is hardcoded in
> get_language_name.
>
> However, if gdb doesn't know what language file to use, it falls back to using
> python pygments, and in absence of that, unhighlighted source text.
>
> In the case of python pygments, it autodetects which language to use based on
> the file name.
>
> Add the same capability when using the source-highlight library.
>
> Tested on x86_64-linux.
>
> Verified that it works by:
> - making get_language_name return nullptr for language_c, and
> - checking that source-highlight still manages to highlight a hello world.
>
> PR cli/30966
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30966

Thanks for working on this! I tested this the way you mentioned and it 
works.

I only have one small nit for style.

> ---
>   gdb/source-cache.c | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/source-cache.c b/gdb/source-cache.c
> index 99be3334803..92acb100901 100644
> --- a/gdb/source-cache.c
> +++ b/gdb/source-cache.c
> @@ -37,6 +37,7 @@
>   #include <sstream>
>   #include <srchilite/sourcehighlight.h>
>   #include <srchilite/langmap.h>
> +#include <srchilite/settings.h>
>   #endif
>   
>   /* The number of source files we'll cache.  */
> @@ -205,8 +206,6 @@ try_source_highlight (std::string &contents ATTRIBUTE_UNUSED,
>       return false;
>   
>     const char *lang_name = get_language_name (lang);
> -  if (lang_name == nullptr)
> -    return false;
>   
>     /* The global source highlight object, or null if one was
>        never constructed.  This is stored here rather than in
> @@ -214,6 +213,9 @@ try_source_highlight (std::string &contents ATTRIBUTE_UNUSED,
>        conditional compilation in source-cache.h.  */
>     static srchilite::SourceHighlight *highlighter;
>   
> +  /* The global source highlight language map object.  */
> +  static srchilite::LangMap *langmap;
> +
>     bool styled = false;
>     try
>       {
> @@ -221,6 +223,18 @@ try_source_highlight (std::string &contents ATTRIBUTE_UNUSED,
>   	{
>   	  highlighter = new srchilite::SourceHighlight ("esc.outlang");
>   	  highlighter->setStyleFile ("esc.style");
> +
> +	  const std::string &datadir = srchilite::Settings::retrieveDataDir ();
> +	  langmap = new srchilite::LangMap (datadir, "lang.map");
> +	}
> +
> +      std::string detected_lang;
> +      if (lang_name == nullptr)
> +	{
> +	  detected_lang = langmap->getMappedFileNameFromFileName (fullname);
> +	  if (detected_lang.empty ())
> +	    return false;
> +	  lang_name = detected_lang.c_str ();

Instead of an early exit here, I think I would prefer if it was:

if ( !detected_lang.empty ())
   lang_name = detected_lang.c_str ();

mainly because if something else is changed in this function, this might 
cause bugs that would be very hard to track down.

Testing locally, I don't see any performance difference with 5000 
repeats, and I feel like the easier flow graph could be would be worth it.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>   	}
>   
>         std::istringstream input (contents);
>
> base-commit: 29736fc507c7a9c6e797b7f83e8df4be73d37767