On 2/3/20 10:05 PM, Segher Boessenkool wrote: > On Mon, Feb 03, 2020 at 08:16:52PM +0000, Bernd Edlinger wrote: >> So gnome terminal is a problem, since it depend heavily on the software >> version, VTE library, and gnome-terminal. >> Sometimes URLs are functional, sometimes competely buggy. >> >> But, wait a moment, here is the deal: >> >> I can detect old gnome terminals, >> they have COLORTERM=gnome-terminal (and produce garbage) >> >> but new gnome terminal with true URL-support have >> >> COLORTERM=truecolor >> >> So how about adding that to the detection logic ? > > It works on at least one of my older setups, too (will have to check > the rest when I have time, unfortunately the weekend is just past). >
Cool. so here is the next version, which removes tmux, and adds detection of old gnome-terminal, and linux console sessions, while also attempting to work with ssh sessions, where we do we have a bit less reliable information, but I would think that is still an improvement. I'd let TERM_URLS and GCC_URLS override the last two exceptions, as TERM=xterm can also mean, really xterm, but while that one does not print garbage, it does not handle the URLs either. How do you like it this way? Is it OK for trunk? Thanks Bernd.
From 35a6a850680907995e13dcd8497f5190710af0dd 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 xfce4-terminal, gnome-terminal, the linux console, and 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 | 102 ++++++++++++++++++++++++++++++++++++++++++++++--- gcc/diagnostic-url.h | 18 ++++++++- gcc/diagnostic.c | 21 ++++++++-- gcc/doc/install.texi | 15 ++++++-- gcc/doc/invoke.texi | 39 +++++++++++++++++-- gcc/pretty-print.c | 44 ++++++++++++++++++--- gcc/pretty-print.h | 5 ++- 10 files changed, 293 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..f1b91e9 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,109 @@ 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 *t, *p; + + /* First check the terminal is capable of printing color escapes, + if not URLs won't work either. */ + if (!should_colorize ()) + return false; + + t = getenv ("TERM"); + p = getenv ("COLORTERM"); + + /* 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 (p && !strcmp (p, "xfce4-terminal")) + return false; + + /* Old versions of gnome-terminal where URL escapes cause screen + corruptions set COLORTERM="gnome-terminal", recent versions + with working URL support set this to "truecolor". */ + if (p && !strcmp (p, "gnome-terminal")) + return false; + + /* Since the following checks are less specific than the ones + above, let GCC_URLS and TERM_URLS override the decision. */ + if (getenv ("GCC_URLS") || getenv ("TERM_URLS")) + return true; + + /* In an ssh session the COLORTERM is not there, but TERM=xterm + can be used as an indication of a incompatible terminal while + TERM=xterm-256color appears to be a working terminal. */ + if (!p && t && !strcmp (t, "xterm")) + return false; + + /* When logging in a linux over serial line, we see TERM=linux + and no COLORTERM, it is unlikely that the URL escapes will + work in that environmen either. */ + if (!p && t && !strcmp (t, "linux")) + 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..4c9bfa2 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,46 @@ 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, certain known to be buggy +gnome-terminal versions, the linux console, 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