On 2/1/20 4:54 PM, Segher Boessenkool wrote:
> Hi!
>
> On Sat, Feb 01, 2020 at 05:02:18AM +0000, Bernd Edlinger wrote:
>>>> Definitely, if the situation with tmux is like xfce4-terminal (reportedly
>>>> the 0.8 version switched to a fixed VTE, which makes the URLs invisible,
>>>> but the URL feature request is pending sine 2017, with no activity
>>>> whatsoever),
>>>> then detecting that and disabling the URLs until they finally implement
>>>> the URLs is straight forward. I can add that to my patch if you confirm,
>>>> the right detection logic:
>>>
>>> The situation is that it is a terminal multiplexer; you can connect any
>>> terminal to it, and swap those out, etc. You might have one that has
>>> support for the url thing at one time, but when you look at that session
>>> from a different machine (from your phone, say), not anymore (or the
>>> other way around). You can also connect multiple actual terminals at
>>> the same time.
>>>
>>> In short, even *if* you could detect whether the terminal supports this
>>> url thing (and you cannot), you cannot detect whether it will support it
>>> two seconds from now, and even if the url thing you already displayed
>>> will misbehave *in the future*!
>>
>> Ah, okay, this is a totally wrong assumption then.
>>
>> So if I understand you right, I should add a check for
>> tmux in should_colorize, where the TERM=dumb thing is,
>> and add TERM=screen, and TMUX=anything, to switch of
>> auto-color off, which will take down URLs at the same time?
>
> Well, colourisation doesn't mess up the display so much, in practice; it
> just makes it completely unreadable (for me anyway), so I have GCC_COLORS=
> just like I need stuff for very many other programs to disable colours.
> Something similar can of course work to turn off the url thing, but since
> that make the display unusable on not very few systems, and the utility
> of this is much lower as well (people just love those colours, for some
> strange reason; urls is more meh), yes please, do not do the url thing
> with TERM=dumb or TERM=ansi or TERM=screen and maybe some similar; but I
> think many people will like their colours enabled.
>
>
re-sending this as I accidentally did not "reply-to-all"...
Okay in that case I would move the TUMX detection code to the auto_url
function, that is easy. I would of course like to be as specific to
a single terminal as possible here, since it is possible that
tumx will implement URLs some day, at least there seems to be
a patch available here:
https://github.com/tmux/tmux/issues/911#issuecomment-554312483
so once there is a released tumx version, and that works reliably,
which I may still doubt, we should reconsider this auto detection.
So the situation may well arise that we would like to auto-enable tumx
and auto-disable screen if I see that right. But certainly we should
only disable a potentially useful feature when there is hard evidence
that it is *only* causing real problems to real people.
And what is the meaning of TERM=ansi does it imply it does not
understand these OSC-8 escape codes ?
So I moved the check for TUMX to the url code path only now, as
Segher suggested.
I am yet undecided what to make out of TERM=ansi, but it is hopefully
only a theoretical issue.
I attached the new version again.
Is it OK for trunk?
Bernd.
From 68203865b77ec254324746316fc438fa65fbc2b3 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 tumx, 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 | 82 ++++++++++++++++++++++++++++++++++++++++++++++----
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, 264 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..7c2cb9e 100644
--- a/gcc/diagnostic-color.c
+++ b/gcc/diagnostic-color.c
@@ -239,20 +239,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..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
