On 2/2/20 1:28 AM, Sandra Loosemore wrote:
> On 2/1/20 8:43 AM, Bernd Edlinger wrote:
> 
>> 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.
>> +
> 
> It would be good to rewrite both of the above paragraphs to get rid of the 
> future tense and put them in the active voice, like
> 
> @samp{auto-if-env} makes ... the default if ...
> 

done.

> Also @code{GCC_COLORS} ought to be @env{GCC_COLORS}, no?
> 

right.

>> 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".
> 
> We should avoid literal quotes in the manual instead of proper markup. Maybe 
> @samp{dumb}?  Or is this trying to express something other than a literal 
> string?
> 

okay, in this case, maybe it gets clearer when I write:

"standard error is a terminal, and when not executing in an emacs shell."

I moved the "dumb" to a comment in the C-function doing that detection
now, and consider it an implementation detail for the emacs detection.


>> +
>> +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
> 
> s/will be/is/, unless this is something that hasn't been implemented yet.  :-S
> 

okay, thanks.

>> +dependent on those environment variables:
>> +If @env{GCC_URLS} set to empty or "no", do not embed URLs in diagnostics.
> 
> s/set/is set/ ??
> 

yes.

> And please avoid literal quotes in the text here too.
> 

done.

>> +If set to "st", URLs use ST escape sequences.
>> +If set to "bel", the default, URLs use BEL escape sequences.
> 
> I have no idea what ST and BEL escape sequences are....
> 

those are the acronyms of certain escape sequences, I added an explanation.

>> +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
> 
> What is "an installed basis"?
> 

I meant that old versions are still installed on a lot of places,
and we do not want to break them.
I re-worded that paragraph, to be hopefully clearer now.

>> +escapes are likely to mis-behave, i.e. print garbage on the screen.
> 
> s/mis-behave/misbehave/
> 

done.

>> +That list is currently xfce4-terminal and tmux and mingw, this can be
>> +overridden with the @option{-fdiagnostics-urls=always}.
> 
> Are those things literal strings for something?  How do you use that option 
> to override the list of terminals?
> 

No, xfce4-terminal and tmux are names of software packages.
mingw, is a minimal interface library, that allows GCC to run under Windows.
Windows has a command shell, that is printing garbage on windows 7,
when URL escapes are used and ignores the URLs on windows 10, but
there is also a windows terminal program that prints garbage in all versions.
Therefore this feature cannot be used on windows, but it is known to cause
screen corruptions at least on some Windows versions, that are still in use.

> Frankly, this whole feature and the options that control it seem horribly 
> over-complex to me.  :-S
> 

Thanks for your review,
I attached a changes.diff with just the changes in response to your review,
and a complete git format-patch file, containing the latest patch.


Is it OK for trunk?

Thanks
Bernd.
diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c
index 7c2cb9e..e115144 100644
--- a/gcc/diagnostic-color.c
+++ b/gcc/diagnostic-color.c
@@ -216,6 +216,7 @@ should_colorize (void)
 	  && GetConsoleMode (h, &m);
 #else
   char const *t = getenv ("TERM");
+  /* emacs M-x shell sets TERM="dumb".  */
   return t && strcmp (t, "dumb") != 0 && isatty (STDERR_FILENO);
 #endif
 }
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 1d3fec5..8ddebbb 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2095,8 +2095,8 @@ GLIBC 2.11 or above, otherwise disabled.
 Tells GCC to use @var{choice} as the default for @option{-fdiagnostics-color=}
 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}
+where @samp{auto} is the default.  @samp{auto-if-env} makes
+@option{-fdiagnostics-color=auto} the default if @env{GCC_COLORS}
 is present and non-empty in the environment of the compiler, and
 @option{-fdiagnostics-color=never} otherwise.
 
@@ -2104,8 +2104,8 @@ is present and non-empty in the environment of the compiler, and
 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}
+where @samp{auto} is the default.  @samp{auto-if-env} makes
+@option{-fdiagnostics-urls=auto} 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.
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 59306bc..cd3e062 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3919,7 +3919,8 @@ or @samp{auto}.  The default depends on how the compiler has been configured,
 it can be any of the above @var{WHEN} options or also @samp{never}
 if @env{GCC_COLORS} environment variable isn't present in the environment,
 and @samp{auto} otherwise.
-@samp{auto} means to use color only when the standard error is a terminal.
+@samp{auto} makes GCC use color only when the standard error is a terminal,
+and when not executing in an emacs shell.
 The forms @option{-fdiagnostics-color} and @option{-fno-diagnostics-color} are
 aliases for @option{-fdiagnostics-color=always} and
 @option{-fdiagnostics-color=never}, respectively.
@@ -4040,8 +4041,9 @@ option controlling a diagnostic, embed a URL for documentation of that
 option.
 
 @var{WHEN} is @samp{never}, @samp{always}, or @samp{auto}.
-@samp{auto} means to use URL escape sequences only when the standard error
-is a terminal, and @env{TERM} is not set to "dumb".
+@samp{auto} makes GCC use URL escape sequences only when the standard error
+is a terminal, and when not executing in an emacs shell or any graphical
+terminal which is known to be incompatible with this feature, see below.
 
 The default depends on how the compiler has been configured.
 It can be any of the above @var{WHEN} options.
@@ -4054,19 +4056,22 @@ 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
+However, even with @option{-fdiagnostics-urls=always} the behavior is
 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.
+If @env{GCC_URLS} is set to empty or @samp{no}, do not embed URLs in
+diagnostics.  If set to @samp{st}, URLs use ST escape sequences.
+If set to @samp{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.
+Note: ST is an ANSI escape sequence, string terminator @samp{ESC \},
+BEL is an ASCII character, CTRL-G that usually sounds like a beep.
 
 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}.
+not implement the URL feature, and have bugs or at least had bugs in
+some versions that are still in use, where the URL escapes are likely
+to misbehave, i.e. print garbage on the screen.
+That list is currently xfce4-terminal and tmux and mingw.
+This check can be skipped with the @option{-fdiagnostics-urls=always}.
 
 @item -fno-diagnostics-show-option
 @opindex fno-diagnostics-show-option
From 1fb036372063eb06ffbd2e15766397aaab0464b6 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 (parse_gcc_urls): New helper function.
	(auto_enable_urls): Disable URLs on tmux, 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   | 15 +++++++--
 gcc/doc/invoke.texi    | 38 +++++++++++++++++++++--
 gcc/pretty-print.c     | 44 ++++++++++++++++++++++----
 gcc/pretty-print.h     |  5 +--
 10 files changed, 273 insertions(+), 26 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..e115144 100644
--- a/gcc/diagnostic-color.c
+++ b/gcc/diagnostic-color.c
@@ -216,6 +216,7 @@ should_colorize (void)
 	  && GetConsoleMode (h, &m);
 #else
   char const *t = getenv ("TERM");
+  /* emacs M-x shell sets TERM="dumb".  */
   return t && strcmp (t, "dumb") != 0 && isatty (STDERR_FILENO);
 #endif
 }
@@ -239,20 +240,90 @@ 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;
+
+  /* Do not enable URLs when TMUX is detected, since that is
+     known to not work reliably.  */
+  p = getenv ("TERM");
+  if (p && !strcmp (p, "screen") && getenv ("TMUX"))
+    return false;
+
+  /* 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.  */
+  p = getenv ("COLORTERM");
+  if (p && !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..8ddebbb 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2095,11 +2095,20 @@ GLIBC 2.11 or above, otherwise disabled.
 Tells GCC to use @var{choice} as the default for @option{-fdiagnostics-color=}
 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
+where @samp{auto} is the default.  @samp{auto-if-env} makes
+@option{-fdiagnostics-color=auto} the default if @env{GCC_COLORS}
+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} makes
+@option{-fdiagnostics-urls=auto} 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..4915bd5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3919,7 +3919,8 @@ or @samp{auto}.  The default depends on how the compiler has been configured,
 it can be any of the above @var{WHEN} options or also @samp{never}
 if @env{GCC_COLORS} environment variable isn't present in the environment,
 and @samp{auto} otherwise.
-@samp{auto} means to use color only when the standard error is a terminal.
+@samp{auto} makes GCC use color only when the standard error is a terminal,
+and when not executing in an emacs shell.
 The forms @option{-fdiagnostics-color} and @option{-fno-diagnostics-color} are
 aliases for @option{-fdiagnostics-color=always} and
 @option{-fdiagnostics-color=never}, respectively.
@@ -4032,14 +4033,45 @@ 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} makes GCC use URL escape sequences only when the standard error
+is a terminal, and when not executing in an emacs shell or any graphical
+terminal which is known to be incompatible with this feature, see below.
+
+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 is
+dependent on those environment variables:
+If @env{GCC_URLS} is set to empty or @samp{no}, do not embed URLs in
+diagnostics.  If set to @samp{st}, URLs use ST escape sequences.
+If set to @samp{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.
+Note: ST is an ANSI escape sequence, string terminator @samp{ESC \},
+BEL is an ASCII character, CTRL-G that usually sounds like a beep.
+
+At this time GCC tries to detect also a few terminals that are known to
+not implement the URL feature, and have bugs or at least had bugs in
+some versions that are still in use, where the URL escapes are likely
+to misbehave, i.e. print garbage on the screen.
+That list is currently xfce4-terminal and tmux and mingw.
+This check can be skipped 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