On 2/1/20 2:41 PM, David Malcolm wrote:
> On Sat, 2020-02-01 at 07:30 +0000, Bernd Edlinger wrote:
>>
>> On 1/31/20 11:06 PM, David Malcolm wrote:
>>> On Fri, 2020-01-31 at 16:59 +0000, Bernd Edlinger wrote:
>>>> Hi,
>>>>
>>>> this is patch is heavily based on David's original patch here:
>>>> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01409.html
>>>>
>>>> and addresses Jakub's review comments here:
>>>> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01412.html
>>>>
>>>> So I hope you don't mind, when I pick up this patch since there
>>>> was not much activity recently on this issue, so I assumed you
>>>> would appreciate some help.
>>>
>>> Thanks Bernd; sorry, I got distracted by analyzer bug-fixing.  It's
>>> appreciated.
>>>
>>>> I will try to improve the patch a bit, and hope you are gonna
>>>> like
>>>> it. I agree that this feature is fine, and should be enabled by
>>>> default, and just if it is positively clear that it won't work,
>>>> disabled in the auto mode.
>>>>
>>>> Also as requested by Jakub this tries to be more compatible to
>>>> GCC_COLORS define, and adds the capability to switch between ST
>>>> and BEL string termination and also have a way to disable urls
>>>> even with -fdiagnostics-urls=always (like GCC_COLORS= disables
>>>> colors).
>>>>
>>>> In addition to that I propose to use GCC_URLS and if that
>>>> is not defined use TERM_URLS to control that feature for
>>>> all applications willing to support that.
> 
> [..]
> 
> Thanks for the updated patch; here are some notes:
> 
>> diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c
>> index d554795..f406139 100644
>> --- a/gcc/diagnostic-color.c
>> +++ b/gcc/diagnostic-color.c
>> @@ -216,6 +216,10 @@ should_colorize (void)
>>        && GetConsoleMode (h, &m);
>>  #else
>>    char const *t = getenv ("TERM");
>> +  /* Do not enable colors when TMUX is detected, since that is
>> +     known to not work reliably.  */
>> +  if (t && strcmp (t, "screen") && getenv ("TMUX"))
>> +    return false;
>>    return t && strcmp (t, "dumb") != 0 && isatty (STDERR_FILENO);
>>  #endif
>>  }
> 
> Does our existing colorization code not work with TMUX, or is it just the
> new URLs that are broken?  Segher?
> 

I seem to remember him saying that he always has to configure with
--with-diagnostics-color=never, and the URLs are on top of that.
But there was no configure option for that, which, given his explanation,
made immediately sense to me.

In the case of the xfce terminal, the color thing was always working fine,
but beginning with last october, the warnings look just terrible.

If that assumption turns out to be wrong, we can easily move that check from
the auto_color to the auto_url code, or add more terminals which are
of that kind.


>> @@ -239,20 +243,86 @@ colorize_init (diagnostic_color_rule_t rule)
>>      }
>>  }
>>  
>> +/* Return URL_FORMAT_XXX which tells how we should emit urls
>> +   when in always mode.
>> +   We use GCC_URLS and if that is not defined TERM_URLS.
>> +   If neither is defined the feature is enabled by default.  */
>> +
>> +static diagnostic_url_format
>> +parse_gcc_urls()
> 
> Perhaps rename this to parse_env_vars_for_urls or somesuch, given that
> it's not just GCC_URLS being parsed?
> 

Okay, no problem.

>> +/* Return true if we should use urls when in auto mode, false otherwise.  */
>> +
>> +static bool
>> +auto_enable_urls ()
>> +{
>> +#ifdef __MINGW32__
>> +  return false;
>> +#else
>> +  const char *p;
>> +
>> +  /* First check the terminal is capable to print color escapes,
>                                  ^^^^^^^^^^^^^^^^^^^
> grammar nit: "is capable of printing"
> 

Thanks.

>> +     if not URLs won't work either.  */
>> +  if (!should_colorize ())
>> +    return false;
>> +
>> +  p = getenv ("COLORTERM");
>> +  if (p == NULL)
>> +    return true;
> 
> Is this part of the rejection of terminals that can't print color
> escapes, or is this some other heuristic based on the discussion?
> 

Yes, it has nothing to do with color, the point that was made on the
Hyperlink discussion gist page, was that the correct value would have been
COLORTERM="truecolor", instead of "xfce4-terminal" so that seems to be
a bug.  If they ever fix that, the workaround here will automatically
be disabled, but that is just fine, since they do already use a fixed
VTE version that is not disturbed by the URL escapes, whether or not
the URL feature is ever implemented by the XFCE maintainers.

> 
>> +  /* xfce4-terminal is known to not implement URLs at this time.
>> +     Recently new installations will safely ignore the URL escape
>> +     sequences, but a large number of legacy installations print
>> +     garbage when URLs are printed.  Therefore we loose nothing by
>                                                      ^^^^^
> "loose" -> "lose"
> 

Fixed, thanks.
Note grep finds lots of places where the term "loose" is used all over
the gcc tree.

> If you have version numbers handy, it might be good to mention those
> in this comment, since references to "at this time" and "new" can
> get dated.
> 

Done.

>> +     disabling this feature for that specific terminal type.  */
>> +  if (!strcmp (p, "xfce4-terminal"))
>> +    return false;
>> +
>> +  return true;
>> +#endif
>> +}
>> +
>>  /* Determine if URLs should be enabled, based on RULE.
>>     This reuses the logic for colorization.  */
> 
> Maybe this:
> 
> /* Determine if URLs should be enabled, based on RULE,
>    and, if so, which format to use.
>    This reuses the logic for colorization.  */
> 

Done.

>> -bool
>> -diagnostic_urls_enabled_p (diagnostic_url_rule_t rule)
>> +diagnostic_url_format
>> +diagnostic_urls_enabled (diagnostic_url_rule_t rule)
> 
> [...]
> 
>> diff --git a/gcc/diagnostic-url.h b/gcc/diagnostic-url.h
>> index 6be0569..22253fd 100644
>> --- a/gcc/diagnostic-url.h
>> +++ b/gcc/diagnostic-url.h
>> @@ -31,6 +31,20 @@ typedef enum
>>    DIAGNOSTICS_URL_AUTO     = 2
>>  } diagnostic_url_rule_t;
>>  
>> -extern bool diagnostic_urls_enabled_p (diagnostic_url_rule_t);
>> +/* Tells how URLs are to be emitted:
>> +   - URL_FORMAT_NONE means no URLs shall be emitted.
>> +   - URL_FORMAT_ST means use ST string termination.
>> +   - URL_FROMAT_BEL means use BEL string termination,
>> +     which is the default.  */
>> +enum diagnostic_url_format
>> +{
>> +  URL_FORMAT_NONE,
>> +  URL_FORMAT_ST,
>> +  URL_FORMAT_BEL
>> +};
> 
> There's a typo in the above, but I think it's better to document
> the values inline, so how about:
> 
> /* Tells whether URLs should be emitted, and, if so, how to
>    terminate strings within the escape sequence.   */
> 
> enum diagnostic_url_format
> {
>   /* No URLs shall be emitted.  */
>   URL_FORMAT_NONE,
> 
>   /* Use ST string termination.  */
>   URL_FORMAT_ST,
> 
>   /* Use BEL string termination.  */
>   URL_FORMAT_BEL
> };
> 

Good point.

>> +#define URL_FORMAT_DEFAULT URL_FORMAT_BEL
> 
> Can this be a const rather than a #define?
> 

Yes, works.

> [...]
> 
>> diff --git a/gcc/pretty-print.h b/gcc/pretty-print.h
>> index 001468c9..a0d273b 100644
>> --- a/gcc/pretty-print.h
>> +++ b/gcc/pretty-print.h
> 
> [...]
> 
>> @@ -279,7 +280,7 @@ public:
>>    bool show_color;
>>  
>>    /* Nonzero means that URLs should be emitted.  */
>> -  bool show_urls;
>> +  diagnostic_url_format show_urls;
>>  };
> 
> How about renaming this field to "url_format"?  Maybe:
> 
>     /* Whether URLs should be emitted, and which terminator to use.  */
>     diagnostic_url_format url_format;
> 

No problem, done.

> Hope this is constructive
> Dave
>

Definitely, after all, it is your patch as well.

Attached you'll find the updated version of the patch, for your reference.


Thanks
Bernd.
 
From ba024ff15c3133c0cf327042fe680da1838bc70f Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlin...@hotmail.de>
Date: Wed, 29 Jan 2020 15:31:10 +0100
Subject: [PATCH] PR 87488: Add --with-diagnostics-urls configuration option

2020-01-30  David Malcolm  <dmalc...@redhat.com>
	    Bernd Edlinger  <bernd.edlin...@hotmail.de>

	PR 87488
	PR other/93168
	* config.in (DIAGNOSTICS_URLS_DEFAULT): New define.
	* configure.ac (--with-diagnostics-urls): New configuration
	option, based on --with-diagnostics-color.
	(DIAGNOSTICS_URLS_DEFAULT): New define.
	* config.h: Regenerate.
	* configure: Regenerate.
	* diagnostic.c (diagnostic_urls_init): Handle -1 for
	DIAGNOSTICS_URLS_DEFAULT from configure-time
	--with-diagnostics-urls=auto-if-env by querying for a GCC_URLS
	and TERM_URLS environment variable.
	* diagnostic-url.h (diagnostic_url_format): New enum type.
	(diagnostic_urls_enabled_p): rename to...
	(parse_env_vars_for_urls): ... this, and change return type.
	* diagnostic-color.c (should_colorize): Detect tmux.
	(parse_gcc_urls): New helper function.
	(auto_enable_urls): Disable URLs on xfce4-terminal, mingw.
	(parse_env_vars_for_urls): Adjust.
	* pretty-print.h (pretty_printer::show_urls): rename to...
	(pretty_printer::url_format): ... this, and change to enum.
	* pretty-print.c (pretty_printer::pretty_printer,
	pp_begin_url, pp_end_url, test_urls): Adjust.
	* doc/install.texi (--with-diagnostics-urls): Document the new
	configuration option.
	* doc/invoke.texi (-fdiagnostics-urls): Add GCC_URLS and TERM_URLS
	vindex reference.  Update description of defaults based on the above.
---
 gcc/config.in          |  6 ++++
 gcc/configure          | 41 +++++++++++++++++++++++--
 gcc/configure.ac       | 28 +++++++++++++++++
 gcc/diagnostic-color.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++----
 gcc/diagnostic-url.h   | 18 ++++++++++-
 gcc/diagnostic.c       | 21 +++++++++++--
 gcc/doc/install.texi   | 11 ++++++-
 gcc/doc/invoke.texi    | 31 +++++++++++++++++--
 gcc/pretty-print.c     | 44 ++++++++++++++++++++++----
 gcc/pretty-print.h     |  5 +--
 10 files changed, 265 insertions(+), 23 deletions(-)

diff --git a/gcc/config.in b/gcc/config.in
index 4829286..01fb18d 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -76,6 +76,12 @@
 #endif
 
 
+/* The default for -fdiagnostics-urls option */
+#ifndef USED_FOR_TARGET
+#undef DIAGNOSTICS_URLS_DEFAULT
+#endif
+
+
 /* Define 0/1 if static analyzer feature is enabled. */
 #ifndef USED_FOR_TARGET
 #undef ENABLE_ANALYZER
diff --git a/gcc/configure b/gcc/configure
index 5fa565a..f55cdb8 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -1015,6 +1015,7 @@ enable_host_shared
 enable_libquadmath_support
 with_linker_hash_style
 with_diagnostics_color
+with_diagnostics_urls
 enable_default_pie
 '
       ac_precious_vars='build_alias
@@ -1836,6 +1837,11 @@ Optional Packages:
                           auto-if-env stands for -fdiagnostics-color=auto if
                           GCC_COLOR environment variable is present and
                           -fdiagnostics-color=never otherwise
+  --with-diagnostics-urls={never,auto,auto-if-env,always}
+                          specify the default of -fdiagnostics-urls option
+                          auto-if-env stands for -fdiagnostics-urls=auto if
+                          GCC_URLS or TERM_URLS environment variable is
+                          present and -fdiagnostics-urls=never otherwise
 
 Some influential environment variables:
   CC          C compiler command
@@ -18974,7 +18980,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 18977 "configure"
+#line 18983 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -19080,7 +19086,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 19083 "configure"
+#line 19089 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -30575,6 +30581,37 @@ cat >>confdefs.h <<_ACEOF
 _ACEOF
 
 
+# Specify what should be the default of -fdiagnostics-urls option.
+
+# Check whether --with-diagnostics-urls was given.
+if test "${with_diagnostics_urls+set}" = set; then :
+  withval=$with_diagnostics_urls; case x"$withval" in
+   xnever)
+     DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_NO
+     ;;
+   xauto)
+     DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_AUTO
+     ;;
+   xauto-if-env)
+     DIAGNOSTICS_URLS_DEFAULT=-1
+     ;;
+   xalways)
+     DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_YES
+     ;;
+   *)
+     as_fn_error $? "$withval is an invalid option to --with-diagnostics-urls" "$LINENO" 5
+     ;;
+ esac
+else
+  DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_AUTO
+fi
+
+
+cat >>confdefs.h <<_ACEOF
+#define DIAGNOSTICS_URLS_DEFAULT $DIAGNOSTICS_URLS_DEFAULT
+_ACEOF
+
+
 # Generate gcc-driver-name.h containing GCC_DRIVER_NAME for the benefit
 # of jit/jit-playback.c.
 gcc_driver_version=`eval "${get_gcc_base_ver} $srcdir/BASE-VER"`
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 671b9a6..0e6e475 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -6741,6 +6741,34 @@ AC_ARG_WITH([diagnostics-color],
 AC_DEFINE_UNQUOTED(DIAGNOSTICS_COLOR_DEFAULT, $DIAGNOSTICS_COLOR_DEFAULT,
 		   [The default for -fdiagnostics-color option])
 
+# Specify what should be the default of -fdiagnostics-urls option.
+AC_ARG_WITH([diagnostics-urls],
+[AC_HELP_STRING([--with-diagnostics-urls={never,auto,auto-if-env,always}],
+                [specify the default of -fdiagnostics-urls option
+                 auto-if-env stands for -fdiagnostics-urls=auto if
+                 GCC_URLS or TERM_URLS environment variable is present and
+                 -fdiagnostics-urls=never otherwise])],
+[case x"$withval" in
+   xnever)
+     DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_NO
+     ;;
+   xauto)
+     DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_AUTO
+     ;;
+   xauto-if-env)
+     DIAGNOSTICS_URLS_DEFAULT=-1
+     ;;
+   xalways)
+     DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_YES
+     ;;
+   *)
+     AC_MSG_ERROR([$withval is an invalid option to --with-diagnostics-urls])
+     ;;
+ esac],
+[DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_AUTO])
+AC_DEFINE_UNQUOTED(DIAGNOSTICS_URLS_DEFAULT, $DIAGNOSTICS_URLS_DEFAULT,
+		   [The default for -fdiagnostics-urls option])
+
 # Generate gcc-driver-name.h containing GCC_DRIVER_NAME for the benefit
 # of jit/jit-playback.c.
 gcc_driver_version=`eval "${get_gcc_base_ver} $srcdir/BASE-VER"`
diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c
index d554795..b14de6a 100644
--- a/gcc/diagnostic-color.c
+++ b/gcc/diagnostic-color.c
@@ -216,6 +216,10 @@ should_colorize (void)
 	  && GetConsoleMode (h, &m);
 #else
   char const *t = getenv ("TERM");
+  /* Do not enable colors when TMUX is detected, since that is
+     known to not work reliably.  */
+  if (t && strcmp (t, "screen") && getenv ("TMUX"))
+    return false;
   return t && strcmp (t, "dumb") != 0 && isatty (STDERR_FILENO);
 #endif
 }
@@ -239,20 +243,87 @@ colorize_init (diagnostic_color_rule_t rule)
     }
 }
 
-/* Determine if URLs should be enabled, based on RULE.
+/* Return URL_FORMAT_XXX which tells how we should emit urls
+   when in always mode.
+   We use GCC_URLS and if that is not defined TERM_URLS.
+   If neither is defined the feature is enabled by default.  */
+
+static diagnostic_url_format
+parse_env_vars_for_urls ()
+{
+  const char *p;
+
+  p = getenv ("GCC_URLS"); /* Plural! */
+  if (p == NULL)
+    p = getenv ("TERM_URLS");
+
+  if (p == NULL)
+    return URL_FORMAT_DEFAULT;
+
+  if (*p == '\0')
+    return URL_FORMAT_NONE;
+
+  if (!strcmp (p, "no"))
+    return URL_FORMAT_NONE;
+
+  if (!strcmp (p, "st"))
+    return URL_FORMAT_ST;
+
+  if (!strcmp (p, "bel"))
+    return URL_FORMAT_BEL;
+
+  return URL_FORMAT_DEFAULT;
+}
+
+/* Return true if we should use urls when in auto mode, false otherwise.  */
+
+static bool
+auto_enable_urls ()
+{
+#ifdef __MINGW32__
+  return false;
+#else
+  const char *p;
+
+  /* First check the terminal is capable of printing color escapes,
+     if not URLs won't work either.  */
+  if (!should_colorize ())
+    return false;
+
+  p = getenv ("COLORTERM");
+  if (p == NULL)
+    return true;
+
+  /* xfce4-terminal is known to not implement URLs at this time.
+     Recently new installations (0.8) will safely ignore the URL escape
+     sequences, but a large number of legacy installations (0.6.3) print
+     garbage when URLs are printed.  Therefore we lose nothing by
+     disabling this feature for that specific terminal type.  */
+  if (!strcmp (p, "xfce4-terminal"))
+    return false;
+
+  return true;
+#endif
+}
+
+/* Determine if URLs should be enabled, based on RULE,
+   and, if so, which format to use.
    This reuses the logic for colorization.  */
 
-bool
-diagnostic_urls_enabled_p (diagnostic_url_rule_t rule)
+diagnostic_url_format
+parse_env_vars_for_urls (diagnostic_url_rule_t rule)
 {
   switch (rule)
     {
     case DIAGNOSTICS_URL_NO:
-      return false;
+      return URL_FORMAT_NONE;
     case DIAGNOSTICS_URL_YES:
-      return true;
+      return parse_env_vars_for_urls ();
     case DIAGNOSTICS_URL_AUTO:
-      return should_colorize ();
+      if (auto_enable_urls ())
+	return parse_env_vars_for_urls ();
+      else
+	return URL_FORMAT_NONE;
     default:
       gcc_unreachable ();
     }
diff --git a/gcc/diagnostic-url.h b/gcc/diagnostic-url.h
index 6be0569..df3fdc3 100644
--- a/gcc/diagnostic-url.h
+++ b/gcc/diagnostic-url.h
@@ -31,6 +31,22 @@ typedef enum
   DIAGNOSTICS_URL_AUTO     = 2
 } diagnostic_url_rule_t;
 
-extern bool diagnostic_urls_enabled_p (diagnostic_url_rule_t);
+/* Tells whether URLs should be emitted, and, if so, how to
+   terminate strings within the escape sequence.   */
+enum diagnostic_url_format
+{
+  /* No URLs shall be emitted.  */
+  URL_FORMAT_NONE,
+
+  /* Use ST string termination.  */
+  URL_FORMAT_ST,
+
+  /* Use BEL string termination.  */
+  URL_FORMAT_BEL
+};
+
+const diagnostic_url_format URL_FORMAT_DEFAULT = URL_FORMAT_BEL;
+
+extern diagnostic_url_format parse_env_vars_for_urls (diagnostic_url_rule_t);
 
 #endif /* ! GCC_DIAGNOSTIC_URL_H */
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 3386f07..24243ee 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -260,11 +260,26 @@ diagnostic_color_init (diagnostic_context *context, int value /*= -1 */)
 void
 diagnostic_urls_init (diagnostic_context *context, int value /*= -1 */)
 {
+  /* value == -1 is the default value.  */
   if (value < 0)
-    value = DIAGNOSTICS_COLOR_DEFAULT;
+    {
+      /* If DIAGNOSTICS_URLS_DEFAULT is -1, default to
+	 -fdiagnostics-urls=auto if GCC_URLS or TERM_URLS is in the
+	 environment, otherwise default to -fdiagnostics-urls=never,
+	 for other values default to that
+	 -fdiagnostics-urls={never,auto,always}.  */
+      if (DIAGNOSTICS_URLS_DEFAULT == -1)
+	{
+	  if (!getenv ("GCC_URLS") && !getenv ("TERM_URLS"))
+	    return;
+	  value = DIAGNOSTICS_URL_AUTO;
+	}
+      else
+	value = DIAGNOSTICS_URLS_DEFAULT;
+    }
 
-  context->printer->show_urls
-    = diagnostic_urls_enabled_p ((diagnostic_url_rule_t) value);
+  context->printer->url_format
+    = parse_env_vars_for_urls ((diagnostic_url_rule_t) value);
 }
 
 /* Do any cleaning up required after the last diagnostic is emitted.  */
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 6ffafac..1d3fec5 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2097,9 +2097,18 @@ option (if not used explicitly on the command line).  @var{choice}
 can be one of @samp{never}, @samp{auto}, @samp{always}, and @samp{auto-if-env}
 where @samp{auto} is the default.  @samp{auto-if-env} means that
 @option{-fdiagnostics-color=auto} will be the default if @code{GCC_COLORS}
-is present and non-empty in the environment, and
+is present and non-empty in the environment of the compiler, and
 @option{-fdiagnostics-color=never} otherwise.
 
+@item --with-diagnostics-urls=@var{choice}
+Tells GCC to use @var{choice} as the default for @option{-fdiagnostics-urls=}
+option (if not used explicitly on the command line).  @var{choice}
+can be one of @samp{never}, @samp{auto}, @samp{always}, and @samp{auto-if-env}
+where @samp{auto} is the default.  @samp{auto-if-env} means that
+@option{-fdiagnostics-urls=auto} will be the default if @env{GCC_URLS}
+or @env{TERM_URLS} is present and non-empty in the environment of the
+compiler, and @option{-fdiagnostics-urls=never} otherwise.
+
 @item --enable-lto
 @itemx --disable-lto
 Enable support for link-time optimization (LTO).  This is enabled by
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index b8ba8a3..59306bc 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4032,14 +4032,41 @@ arguments in the C++ frontend.
 @item -fdiagnostics-urls[=@var{WHEN}]
 @opindex fdiagnostics-urls
 @cindex urls
+@vindex GCC_URLS @r{environment variable}
+@vindex TERM_URLS @r{environment variable}
 Use escape sequences to embed URLs in diagnostics.  For example, when
 @option{-fdiagnostics-show-option} emits text showing the command-line
 option controlling a diagnostic, embed a URL for documentation of that
 option.
 
 @var{WHEN} is @samp{never}, @samp{always}, or @samp{auto}.
-The default is @samp{auto}, which means to use URL escape sequences only
-when the standard error is a terminal.
+@samp{auto} means to use URL escape sequences only when the standard error
+is a terminal, and @env{TERM} is not set to "dumb".
+
+The default depends on how the compiler has been configured.
+It can be any of the above @var{WHEN} options.
+
+GCC can also be configured (via the
+@option{--with-diagnostics-urls=auto-if-env} configure-time option)
+so that the default is affected by environment variables.
+Under such a configuration, GCC defaults to using @samp{auto}
+if either @env{GCC_URLS} or @env{TERM_URLS} environment variables are
+present and non-empty in the environment of the compiler, or @samp{never}
+if neither are.
+
+However, even with @option{-fdiagnostics-urls=always} the behavior will be
+dependent on those environment variables:
+If @env{GCC_URLS} set to empty or "no", do not embed URLs in diagnostics.
+If set to "st", URLs use ST escape sequences.
+If set to "bel", the default, URLs use BEL escape sequences.
+Any other non-empty value enables the feature.
+If @env{GCC_URLS} is not set, use @env{TERM_URLS} as a fallback.
+
+At this time GCC tries to detect also a few terminals that are known to
+not implement the URL feature, and have an installed basis where the URL
+escapes are likely to mis-behave, i.e. print garbage on the screen.
+That list is currently xfce4-terminal and tmux and mingw, this can be
+overridden with the @option{-fdiagnostics-urls=always}.
 
 @item -fno-diagnostics-show-option
 @opindex fno-diagnostics-show-option
diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 817c105..dde138b 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -1647,7 +1647,7 @@ pretty_printer::pretty_printer (int maximum_length)
     need_newline (),
     translate_identifiers (true),
     show_color (),
-    show_urls (false)
+    url_format (URL_FORMAT_NONE)
 {
   pp_line_cutoff (this) = maximum_length;
   /* By default, we emit prefixes once per message.  */
@@ -1670,7 +1670,7 @@ pretty_printer::pretty_printer (const pretty_printer &other)
   need_newline (other.need_newline),
   translate_identifiers (other.translate_identifiers),
   show_color (other.show_color),
-  show_urls (other.show_urls)
+  url_format (other.url_format)
 {
   pp_line_cutoff (this) = maximum_length;
   /* By default, we emit prefixes once per message.  */
@@ -2171,8 +2171,19 @@ identifier_to_locale (const char *ident)
 void
 pp_begin_url (pretty_printer *pp, const char *url)
 {
-  if (pp->show_urls)
+  switch (pp->url_format)
+  {
+  case URL_FORMAT_NONE:
+    break;
+  case URL_FORMAT_ST:
+    pp_printf (pp, "\33]8;;%s\33\\", url);
+    break;
+  case URL_FORMAT_BEL:
     pp_printf (pp, "\33]8;;%s\a", url);
+    break;
+  default:
+    gcc_unreachable ();
+  }
 }
 
 /* If URL-printing is enabled, write a "close URL" escape sequence to PP.  */
@@ -2180,8 +2191,19 @@ pp_begin_url (pretty_printer *pp, const char *url)
 void
 pp_end_url (pretty_printer *pp)
 {
-  if (pp->show_urls)
+  switch (pp->url_format)
+  {
+  case URL_FORMAT_NONE:
+    break;
+  case URL_FORMAT_ST:
+    pp_string (pp, "\33]8;;\33\\");
+    break;
+  case URL_FORMAT_BEL:
     pp_string (pp, "\33]8;;\a");
+    break;
+  default:
+    gcc_unreachable ();
+  }
 }
 
 #if CHECKING_P
@@ -2490,7 +2512,7 @@ test_urls ()
 {
   {
     pretty_printer pp;
-    pp.show_urls = false;
+    pp.url_format = URL_FORMAT_NONE;
     pp_begin_url (&pp, "http://example.com";);
     pp_string (&pp, "This is a link");
     pp_end_url (&pp);
@@ -2500,7 +2522,17 @@ test_urls ()
 
   {
     pretty_printer pp;
-    pp.show_urls = true;
+    pp.url_format = URL_FORMAT_ST;
+    pp_begin_url (&pp, "http://example.com";);
+    pp_string (&pp, "This is a link");
+    pp_end_url (&pp);
+    ASSERT_STREQ ("\33]8;;http://example.com\33\\This is a link\33]8;;\33\\",
+		  pp_formatted_text (&pp));
+  }
+
+  {
+    pretty_printer pp;
+    pp.url_format = URL_FORMAT_BEL;
     pp_begin_url (&pp, "http://example.com";);
     pp_string (&pp, "This is a link");
     pp_end_url (&pp);
diff --git a/gcc/pretty-print.h b/gcc/pretty-print.h
index 001468c9..22892f1 100644
--- a/gcc/pretty-print.h
+++ b/gcc/pretty-print.h
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
 #define GCC_PRETTY_PRINT_H
 
 #include "obstack.h"
+#include "diagnostic-url.h"
 
 /* Maximum number of format string arguments.  */
 #define PP_NL_ARGMAX   30
@@ -278,8 +279,8 @@ public:
   /* Nonzero means that text should be colorized.  */
   bool show_color;
 
-  /* Nonzero means that URLs should be emitted.  */
-  bool show_urls;
+  /* Whether URLs should be emitted, and which terminator to use.  */
+  diagnostic_url_format url_format;
 };
 
 static inline const char *
-- 
1.9.1

Reply via email to