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.
>
> I think I like the overall idea.
>
Thanks!
>> Since I have seen much garbage from the URLs in the xfce4-terminal
>> 0.6.3
>> admittedly an old Ubuntu installation, but still in LTS status,
>> and no attempt from the xfc4-terminal to _ever_ implement URLs, I
>> would
>> like to detect the xfce4-terminal, and use that in auto mode
>> to switch off the URLs feature, since in best case the URLs will
>> just be ignored, but can and will often create garbage on the screen.
>>
>> Likewise on MinGW, since the windows 10 cmd prompt does at best
>> ignore the URLs, but windows terminal and windows 7 cmd prompt
>> print garbage when URLs are output.
>
> That sounds reasonable, but we should document those exclusions, I
> think.
>
done.
> I think the ChangeLog should also refer to "PR other/93168":
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93168
>
done.
>> diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c
>> index d554795..ad84d1f 100644
>> --- a/gcc/diagnostic-color.c
>> +++ b/gcc/diagnostic-color.c
>> @@ -239,6 +239,56 @@ colorize_init (diagnostic_color_rule_t rule)
>> }
>> }
>>
>> +bool diagnostic_urls_use_st = false;
>
> I don't like global variables (a pet peeve of mine); can you make this
> be a field of the pretty_printer instead?
>
> Even better: convert pretty_printer::show_urls from a bool to a new
> enum; something like:
>
> enum diagnostic_url_format
> {
> URL_FORMAT_NONE,
> URL_FORMAT_ST,
> URL_FORMAT_BEL
> };
>
> with suitable leading comments (probably a good place to summarize some
> of the compatibility info within the source).
>
> Then we could verify the differences between ST and BEL in the
> selftests, and also not have the selftests be affected by env vars.
>
Okay, I have to include diagnostic-url.h from pretty-print.h in that case,
since both files are included from the generated options.c in alphabetic order
/-).
I was first a bit reluctant to do so, since we usually don't have recursive
header files but probably that is okay then, alternatively I could use a
bitfield
in pretty-print.h and put defines for URL_FORMAT_xxx in diagnostic-url.h.
>
>> +/* Return true if we should use urls when in always mode, false otherwise.
>> + Additionally initialize diagnostic_urls_use_st to true if ST escapes
>> + should be used and false for BEL escapes. */
>
> Maybe have this return that 3-way enum instead?
>
okay, done.
>> +
>> +static bool
>> +parse_gcc_urls()
>> +{
>> + const char *p;
>> +
>> + p = getenv ("GCC_URLS"); /* Plural! */
>> + if (p == NULL)
>> + p = getenv ("TERM_URLS");
>> +
>> + if (p == NULL)
>> + return true;
>> + if (*p == '\0')
>> + return false;
>> +
>> + if (!strcmp (p, "no"))
>> + return false;
>> + if (!strcmp (p, "st"))
>> + diagnostic_urls_use_st = true;
>> + else if (!strcmp (p, "bel"))
>> + diagnostic_urls_use_st = false;
>> + return true;
>> +}
>> +
>> +/* 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;
>> +
>> + if (!should_colorize ())
>> + return false;
>> + p = getenv ("COLORTERM");
>> + if (p == NULL)
>> + return true;
>> + if (!strcmp (p, "xfce4-terminal"))
>> + return false;
>> + return true;
>> +#endif
>> +}
>
> I think this logic warrants a comment for each of the exclusions: why
> are we excluding them? I'd prefer to capture that in the source,
> rather than just in the mailing list.
>
done.
>> /* Determine if URLs should be enabled, based on RULE.
>> This reuses the logic for colorization. */
>>
>> @@ -250,9 +300,12 @@ diagnostic_urls_enabled_p (diagnostic_url_rule_t rule)
>> case DIAGNOSTICS_URL_NO:
>> return false;
>> case DIAGNOSTICS_URL_YES:
>> - return true;
>> + return parse_gcc_urls ();
>> case DIAGNOSTICS_URL_AUTO:
>> - return should_colorize ();
>> + if (auto_enable_urls ())
>> + return parse_gcc_urls ();
>> + else
>> + return false;
>> default:
>> gcc_unreachable ();
>> }
>> diff --git a/gcc/diagnostic-url.h b/gcc/diagnostic-url.h
>> index 6be0569..06b7784 100644
>> --- a/gcc/diagnostic-url.h
>> +++ b/gcc/diagnostic-url.h
>> @@ -32,5 +32,6 @@ typedef enum
>> } diagnostic_url_rule_t;
>>
>> extern bool diagnostic_urls_enabled_p (diagnostic_url_rule_t);
>> +extern bool diagnostic_urls_use_st;
>>
>> #endif /* ! GCC_DIAGNOSTIC_URL_H */
>> diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
>> index 3386f07..55e323f 100644
>> --- a/gcc/diagnostic.c
>> +++ b/gcc/diagnostic.c
>> @@ -260,8 +260,23 @@ 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);
>> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
>> index 6ffafac..f3f63be 100644
>> --- a/gcc/doc/install.texi
>> +++ b/gcc/doc/install.texi
>> @@ -2100,6 +2100,15 @@ where @samp{auto} is the default. @samp{auto-if-env}
>> means that
>> is present and non-empty in the environment, 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, and
>> +@option{-fdiagnostics-urls=never} otherwise.
>
> I realize that you're adapting what I wrote, but I realize now it's a
> little unclear: are those environment vars checked at configure time, or
> when GCC is run? I believe that it's the latter, but this doc could be
> misread as if it's the former.
>
okay, I added "at runtime"
> [...]
>
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 2cd8d7e..4ebb411 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -4032,14 +4032,25 @@ arguments in the C++ frontend.
>> @item -fdiagnostics-urls[=@var{WHEN}]
>> @opindex fdiagnostics-urls
>> @cindex urls
>> +@vindex GCC_URLS @r{environment variable}
>
> Should also have a vindex for TERM_URLS here.
>
done.
>> 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 or also @samp{never}
>> +if neither @env{GCC_URLS} nor @env{TERM_URLS} environment variable is
>> +present in the environment, and @samp{auto} otherwise.
>
> I found myself getting confused reading the above.
> I this would be clearer if we start a new sentence or para at the
> "or also" above.
>
> How about something like:
>
> 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 in the environment, or @samp{never} if neither are.
>
> Did I correctly characterize the behavior? Is that clearer?
>
Yes, indeed.
>> +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.
>
> This also could use a little rewording. The situation is quite complex in
> that we have configure-time defaults, command-line defaults, and
> environment variables interacting with each other.
>
I am stuck here, probably need help with re-wording...
CC: Sandra, for the documentation part.
> Am I right in thinking that:
>
> (a) with the configure-time default of
> --with-diagnostics-urls=auto
> that you get URLs if at a tty, but you can set GCC_URLS=no to turn them
> off, or can override that with the -fdiagnostics-urls=WHEN
>
> (b) with the configure-time default of
> --with-diagnostics-urls=auto-if-env
> that you don't get URLs if at tty unless you set GCC_URLS in your env.
>
> and that some known not-working are always excluded?
>
Yes, the known not-working are excluded by auto or auto-if-env
the always option skips that check, but can still be overridden by
GCC_URLS=no for instance. I took that idea from Jakub's comment
on the original patch, that is same how GCC_COLORS= overrides
the -fdiagnostics-color=always command line option.
> Hence (a) presumably makes a good default for a bleeding-edge
> distribution, and (b) may makes a good default for a conservative
> distribution which may generally have older terminals.
>
> We should also document the exclusions here (e.g. xfce-terminal).
>
Done. also added exclusion logic for tmux which would be nice if
Segher could test it.
So attached is what I have right now.
Bootstrapped and reg-tested again on x86_64-pc-linux-gnu with
xfce4-terminal as console.
Is it OK for trunk?
Thanks
Bernd.
From 8c22cedf20f279a26db4f3612a9aea333683a50a 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...
(diagnostic_urls_enabled): ... 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.
(diagnostic_urls_enabled): Adjust.
* pretty-print.h (pretty_printer::show_urls): Change to enum.
* pretty-print.c (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 | 80 ++++++++++++++++++++++++++++++++++++++++++++++----
gcc/diagnostic-url.h | 16 +++++++++-
gcc/diagnostic.c | 19 ++++++++++--
gcc/doc/install.texi | 11 ++++++-
gcc/doc/invoke.texi | 30 +++++++++++++++++--
gcc/pretty-print.c | 42 ++++++++++++++++++++++----
gcc/pretty-print.h | 3 +-
10 files changed, 257 insertions(+), 19 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..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
}
@@ -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()
+{
+ 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 to print 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 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
+ 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. */
-bool
-diagnostic_urls_enabled_p (diagnostic_url_rule_t rule)
+diagnostic_url_format
+diagnostic_urls_enabled (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_gcc_urls ();
case DIAGNOSTICS_URL_AUTO:
- return should_colorize ();
+ if (auto_enable_urls ())
+ return parse_gcc_urls ();
+ else
+ return URL_FORMAT_NONE;
default:
gcc_unreachable ();
}
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
+};
+
+#define URL_FORMAT_DEFAULT URL_FORMAT_BEL
+
+extern diagnostic_url_format diagnostic_urls_enabled (diagnostic_url_rule_t);
#endif /* ! GCC_DIAGNOSTIC_URL_H */
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 3386f07..f21fa65 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);
+ = diagnostic_urls_enabled ((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..59d7ecd 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 at runtime, 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 at runtime, 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..6df0efd 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4032,14 +4032,40 @@ 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 in the environment, or @samp{never} if neither are.
+
+But even with @option{-fdiagnostics-urls=always} the behavior will be
+dependent on those environmen 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..6b6cda4 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)
+ show_urls (URL_FORMAT_NONE)
{
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->show_urls)
+ {
+ 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->show_urls)
+ {
+ 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.show_urls = 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.show_urls = 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.show_urls = 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..a0d273b 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
@@ -279,7 +280,7 @@ public:
bool show_color;
/* Nonzero means that URLs should be emitted. */
- bool show_urls;
+ diagnostic_url_format show_urls;
};
static inline const char *
--
1.9.1
