| Message ID | 20191219202419.31099-1-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add --with-diagnostics-urls configuration option and GCC_URLS env var | expand |
On Thu, Dec 19, 2019 at 03:24:19PM -0500, David Malcolm wrote: > Currently -fdiagnostics-urls defaults to "auto" and this works > (e.g. for recent gnome-terminal, and is gracefully discarded by > konsole5), but there have been reports of incompatibilities of the > feature with various other terminals. > > This patch adds a configuration option to change this default, based > on Jakub's analogous change to -fdiagnostics-color (r217540). One > possible configure-time option is --with-diagnostics-urls=auto-if-env, > which adds support for a GCC_URLS environment variable to control > the default (again by analogy with the --with-diagnostics-color > configure-time support for changing -fdiagnostics-color). I think it would be better to have GCC_URLS env var to be be more consistent with GCC_COLORS, so yes, have the never,auto,auto-if-env,always configure possibilities like for colors and for auto-if-env don't print URLS if the env var is not present, but on top of that, parse also the value of the env var like parse_gcc_colors does, and have a way to disable urls even with -fdiagnostics-urls=always (like GCC_COLORS= disables colors), and perhaps use the env var to select between the ST and BEL terminator as apparently some terminals like one or the other more. So perhaps accept GCC_URLS= and GCC_URLS=no as disabling, GCC_URLS=bel as the (default) case for BEL termination and GCC_URLS=st for the ST termination? E.g. have a way even for a compiler configured that it will work with new terminals most of them time and thus --with-diagnostics-urls=always that if somebody sshs to some machine from some old host, GCC_URLS=no (or none?) can be still used to disable it, without the need to tweak build system to inject there some flags. > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu; I > tested the --with-diagnostics-urls=auto-if-env and GCC_URLS behavior > manually. > > Jakub: does this look sane to you? (given that this is heavily based > on your 2014 patches for --with-diagnostics-color). Jakub
I think this is good idea since it is consistent with env var GCC_COLORS -- the color support will be disabled even with -fdiagnostics-color=always, if GCC_COLORS is set to empty. And I has experienced more serious problem when I ssh(1) to the remote machine from a even older terminal, GNOME Terminal 2.30.2. The terminal bells made noticeable delays; The whole terminal window would hang for several seconds when gcc(1) emits too many warnings in at a time... 2019-12-20 4:48 GMT+08:00, Jakub Jelinek <jakub@redhat.com>: > On Thu, Dec 19, 2019 at 03:24:19PM -0500, David Malcolm wrote: >> Currently -fdiagnostics-urls defaults to "auto" and this works >> (e.g. for recent gnome-terminal, and is gracefully discarded by >> konsole5), but there have been reports of incompatibilities of the >> feature with various other terminals. >> >> This patch adds a configuration option to change this default, based >> on Jakub's analogous change to -fdiagnostics-color (r217540). One >> possible configure-time option is --with-diagnostics-urls=auto-if-env, >> which adds support for a GCC_URLS environment variable to control >> the default (again by analogy with the --with-diagnostics-color >> configure-time support for changing -fdiagnostics-color). > > I think it would be better to have GCC_URLS env var to be be more > consistent > with GCC_COLORS, so yes, have the never,auto,auto-if-env,always > configure possibilities like for colors and for auto-if-env don't print > URLS if the env var is not present, but on top of that, parse also the > value > of the env var like parse_gcc_colors does, and have a way to disable urls > even with -fdiagnostics-urls=always (like GCC_COLORS= disables colors), > and perhaps use the env var to select between the ST and BEL terminator > as apparently some terminals like one or the other more. > So perhaps accept GCC_URLS= and GCC_URLS=no as disabling, GCC_URLS=bel > as the (default) case for BEL termination and GCC_URLS=st for the ST > termination? > E.g. have a way even for a compiler configured that it will work with new > terminals most of them time and thus --with-diagnostics-urls=always that > if somebody sshs to some machine from some old host, GCC_URLS=no (or none?) > can be still used to disable it, without the need to tweak build system to > inject there some flags. > >> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu; I >> tested the --with-diagnostics-urls=auto-if-env and GCC_URLS behavior >> manually. >> >> Jakub: does this look sane to you? (given that this is heavily based >> on your 2014 patches for --with-diagnostics-color). > > Jakub > >
diff --git a/gcc/configure.ac b/gcc/configure.ac index 954c1496fb1..8f3cee0917b 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -6706,6 +6706,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 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.c b/gcc/diagnostic.c index 95cfb6e76ac..708d4649e90 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -257,8 +257,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 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")) + 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 19d649ccc83..83f35a131fb 100644 --- a/gcc/doc/install.texi +++ b/gcc/doc/install.texi @@ -2098,6 +2098,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} +is present in the environment, 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 6942881dbcc..aee4596120d 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -3921,14 +3921,20 @@ arguments in the C++ frontend. @item -fdiagnostics-urls[=@var{WHEN}] @opindex fdiagnostics-urls @cindex urls +@vindex GCC_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. + +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_URLS} environment variable isn't present in the environment, +and @samp{auto} otherwise. @item -fno-diagnostics-show-option @opindex fno-diagnostics-show-option