* [PATCH 1/2] pretty-print: support URL escape sequences (PR 87488)
@ 2019-10-10 17:06 David Malcolm
2019-10-10 17:19 ` [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] " David Malcolm
2019-12-06 23:41 ` [PATCH 1/2] pretty-print: support URL escape sequences " Jakub Jelinek
0 siblings, 2 replies; 11+ messages in thread
From: David Malcolm @ 2019-10-10 17:06 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda
describes an emerging standard for embedding URLs in escape sequences
for marking up text output. This is supported e.g. by recent releases
of GNOME Terminal.
This patch adds support to our pretty-printing framework for emitting
URLs.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
Committed to trunk as r276841.
A followup patch uses this to add URLs to the pertinent documentation
for the output of -fdiagnostics-show-option.
gcc/ChangeLog:
PR 87488
* common.opt (fdiagnostics-urls=): New option.
(diagnostic-url.h): Add SourceInclude.
(diagnostic_url_rule): New enum.
* diagnostic-color.c: Include "diagnostic-url.h".
(diagnostic_urls_enabled_p): New function.
* diagnostic-url.h: New file.
* diagnostic.c: Include "diagnostic-url.h".
(diagnostic_urls_init): New function.
* diagnostic.h (diagnostic_urls_init): New decl.
* doc/invoke.texi (Diagnostic Message Formatting Options): Add
-fdiagnostics-urls to the list.
(-fdiagnostics-urls): New option.
* gcc.c (driver_handle_option): Handle OPT_fdiagnostics_urls_.
(driver::global_initializations): Call diagnostic_urls_init.
* opts-global.c (init_options_once): Likewise.
* opts.c (common_handle_option): Handle OPT_fdiagnostics_urls_.
* pretty-print.c (pretty_printer::pretty_printer): Initialize
show_urls.
(pp_begin_url): New function.
(pp_end_url): New function.
(selftest::test_urls): New selftest.
(selftest::pretty_print_c_tests): Call it.
* pretty-print.h (pretty_printer::show_urls): New field.
(pp_begin_url): New decl.
(pp_end_url): New decl.
gcc/testsuite/ChangeLog:
PR 87488
* lib/prune.exp (TEST_ALWAYS_FLAGS): Add -fdiagnostics-urls=never.
---
gcc/common.opt | 20 ++++++++++++++
gcc/diagnostic-color.c | 20 ++++++++++++++
gcc/diagnostic-url.h | 36 +++++++++++++++++++++++++
gcc/diagnostic.c | 13 +++++++++
gcc/diagnostic.h | 1 +
gcc/doc/invoke.texi | 13 +++++++++
gcc/gcc.c | 5 ++++
gcc/opts-global.c | 1 +
gcc/opts.c | 4 +++
gcc/pretty-print.c | 65 ++++++++++++++++++++++++++++++++++++++++++++-
gcc/pretty-print.h | 6 +++++
gcc/testsuite/lib/prune.exp | 2 +-
12 files changed, 184 insertions(+), 2 deletions(-)
create mode 100644 gcc/diagnostic-url.h
diff --git a/gcc/common.opt b/gcc/common.opt
index 1b9e0f3..124e8cf 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1281,6 +1281,26 @@ Enum(diagnostic_color_rule) String(always) Value(DIAGNOSTICS_COLOR_YES)
EnumValue
Enum(diagnostic_color_rule) String(auto) Value(DIAGNOSTICS_COLOR_AUTO)
+fdiagnostics-urls=
+Driver Common Joined RejectNegative Var(flag_diagnostics_show_urls) Enum(diagnostic_url_rule) Init(DIAGNOSTICS_URL_AUTO)
+-fdiagnostics-urls=[never|always|auto] Embed URLs in diagnostics.
+
+; Required for these enum values.
+SourceInclude
+diagnostic-url.h
+
+Enum
+Name(diagnostic_url_rule) Type(int)
+
+EnumValue
+Enum(diagnostic_url_rule) String(never) Value(DIAGNOSTICS_URL_NO)
+
+EnumValue
+Enum(diagnostic_url_rule) String(always) Value(DIAGNOSTICS_URL_YES)
+
+EnumValue
+Enum(diagnostic_url_rule) String(auto) Value(DIAGNOSTICS_URL_AUTO)
+
fdiagnostics-format=
Common Joined RejectNegative Enum(diagnostics_output_format)
-fdiagnostics-format=[text|json] Select output format.
diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c
index 69e759f..abc919f 100644
--- a/gcc/diagnostic-color.c
+++ b/gcc/diagnostic-color.c
@@ -19,6 +19,7 @@
#include "config.h"
#include "system.h"
#include "diagnostic-color.h"
+#include "diagnostic-url.h"
#ifdef __MINGW32__
# include <windows.h>
@@ -236,3 +237,22 @@ colorize_init (diagnostic_color_rule_t rule)
gcc_unreachable ();
}
}
+
+/* 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)
+{
+ switch (rule)
+ {
+ case DIAGNOSTICS_URL_NO:
+ return false;
+ case DIAGNOSTICS_URL_YES:
+ return true;
+ case DIAGNOSTICS_URL_AUTO:
+ return should_colorize ();
+ default:
+ gcc_unreachable ();
+ }
+}
diff --git a/gcc/diagnostic-url.h b/gcc/diagnostic-url.h
new file mode 100644
index 0000000..ce0de45
--- /dev/null
+++ b/gcc/diagnostic-url.h
@@ -0,0 +1,36 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+ Contributed by David Malcolm <dmalcolm@redhat.com>.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3. If not see
+<http://www.gnu.org/licenses/>. */
+
+#ifndef GCC_DIAGNOSTIC_URL_H
+#define GCC_DIAGNOSTIC_URL_H
+
+/* Whether to add URLs to diagnostics:
+ - DIAGNOSTICS_URL_NO: never
+ - DIAGNOSTICS_URL_YES: always
+ - DIAGNOSTICS_URL_AUTO: depending on the output stream. */
+typedef enum
+{
+ DIAGNOSTICS_URL_NO = 0,
+ DIAGNOSTICS_URL_YES = 1,
+ DIAGNOSTICS_URL_AUTO = 2
+} diagnostic_url_rule_t;
+
+extern bool diagnostic_urls_enabled_p (diagnostic_url_rule_t);
+
+#endif /* ! GCC_DIAGNOSTIC_URL_H */
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 1b3306c..467cc39 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3. If not see
#include "backtrace.h"
#include "diagnostic.h"
#include "diagnostic-color.h"
+#include "diagnostic-url.h"
#include "edit-context.h"
#include "selftest.h"
#include "selftest-diagnostic.h"
@@ -246,6 +247,18 @@ diagnostic_color_init (diagnostic_context *context, int value /*= -1 */)
= colorize_init ((diagnostic_color_rule_t) value);
}
+/* Initialize URL support within CONTEXT based on VALUE, handling "auto". */
+
+void
+diagnostic_urls_init (diagnostic_context *context, int value /*= -1 */)
+{
+ if (value < 0)
+ value = DIAGNOSTICS_COLOR_DEFAULT;
+
+ context->printer->show_urls
+ = diagnostic_urls_enabled_p ((diagnostic_url_rule_t) value);
+}
+
/* Do any cleaning up required after the last diagnostic is emitted. */
void
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 530acb4..f0ea8e8 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -331,6 +331,7 @@ diagnostic_override_option_index (diagnostic_info *info, int optidx)
/* Diagnostic related functions. */
extern void diagnostic_initialize (diagnostic_context *, int);
extern void diagnostic_color_init (diagnostic_context *, int value = -1);
+extern void diagnostic_urls_init (diagnostic_context *, int value = -1);
extern void diagnostic_finish (diagnostic_context *);
extern void diagnostic_report_current_module (diagnostic_context *, location_t);
extern void diagnostic_show_locus (diagnostic_context *,
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9247603..bdbcd95 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -273,6 +273,7 @@ Objective-C and Objective-C++ Dialects}.
@gccoptlist{-fmessage-length=@var{n} @gol
-fdiagnostics-show-location=@r{[}once@r{|}every-line@r{]} @gol
-fdiagnostics-color=@r{[}auto@r{|}never@r{|}always@r{]} @gol
+-fdiagnostics-urls=@r{[}auto@r{|}never@r{|}always@r{]} @gol
-fdiagnostics-format=@r{[}text@r{|}json@r{]} @gol
-fno-diagnostics-show-option -fno-diagnostics-show-caret @gol
-fno-diagnostics-show-labels -fno-diagnostics-show-line-numbers @gol
@@ -3908,6 +3909,18 @@ SGR substring for highlighting mismatching types within template
arguments in the C++ frontend.
@end table
+@item -fdiagnostics-urls[=@var{WHEN}]
+@opindex fdiagnostics-urls
+@cindex urls
+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.
+
@item -fno-diagnostics-show-option
@opindex fno-diagnostics-show-option
@opindex fdiagnostics-show-option
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 1216cdd..c45a1df 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -4041,6 +4041,10 @@ driver_handle_option (struct gcc_options *opts,
diagnostic_color_init (dc, value);
break;
+ case OPT_fdiagnostics_urls_:
+ diagnostic_urls_init (dc, value);
+ break;
+
case OPT_fdiagnostics_format_:
diagnostic_output_format_init (dc,
(enum diagnostics_output_format)value);
@@ -7443,6 +7447,7 @@ driver::global_initializations ()
diagnostic_initialize (global_dc, 0);
diagnostic_color_init (global_dc);
+ diagnostic_urls_init (global_dc);
#ifdef GCC_DRIVER_HOST_INITIALIZATION
/* Perform host dependent initialization when needed. */
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index 7c5bd16..b51c2fb 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -255,6 +255,7 @@ init_options_once (void)
construct their pretty-printers means that all previous settings
are overriden. */
diagnostic_color_init (global_dc);
+ diagnostic_urls_init (global_dc);
}
/* Decode command-line options to an array, like
diff --git a/gcc/opts.c b/gcc/opts.c
index 2df0351..f98f3f4 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2492,6 +2492,10 @@ common_handle_option (struct gcc_options *opts,
diagnostic_color_init (dc, value);
break;
+ case OPT_fdiagnostics_urls_:
+ diagnostic_urls_init (dc, value);
+ break;
+
case OPT_fdiagnostics_format_:
diagnostic_output_format_init (dc,
(enum diagnostics_output_format)value);
diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 2b6c585..c57a3db 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -1579,7 +1579,8 @@ pretty_printer::pretty_printer (int maximum_length)
emitted_prefix (),
need_newline (),
translate_identifiers (true),
- show_color ()
+ show_color (),
+ show_urls (false)
{
pp_line_cutoff (this) = maximum_length;
/* By default, we emit prefixes once per message. */
@@ -2028,6 +2029,41 @@ identifier_to_locale (const char *ident)
}
}
+/* Support for encoding URLs.
+ See egmontkob/Hyperlinks_in_Terminal_Emulators.md
+ ( https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda ).
+
+ > A hyperlink is opened upon encountering an OSC 8 escape sequence with
+ > the target URI. The syntax is
+ >
+ > OSC 8 ; params ; URI ST
+ >
+ > A hyperlink is closed with the same escape sequence, omitting the
+ > parameters and the URI but keeping the separators:
+ >
+ > OSC 8 ; ; ST
+ >
+ > OSC (operating system command) is typically ESC ]. */
+
+/* If URL-printing is enabled, write an "open URL" escape sequence to PP
+ for the given URL. */
+
+void
+pp_begin_url (pretty_printer *pp, const char *url)
+{
+ if (pp->show_urls)
+ pp_printf (pp, "\33]8;;%s\33\\", url);
+}
+
+/* If URL-printing is enabled, write a "close URL" escape sequence to PP. */
+
+void
+pp_end_url (pretty_printer *pp)
+{
+ if (pp->show_urls)
+ pp_string (pp, "\33]8;;\33\\");
+}
+
#if CHECKING_P
namespace selftest {
@@ -2312,6 +2348,32 @@ test_prefixes_and_wrapping ()
}
+/* Verify that URL-printing works as expected. */
+
+void
+test_urls ()
+{
+ {
+ pretty_printer pp;
+ pp.show_urls = false;
+ pp_begin_url (&pp, "http://example.com");
+ pp_string (&pp, "This is a link");
+ pp_end_url (&pp);
+ ASSERT_STREQ ("This is a link",
+ pp_formatted_text (&pp));
+ }
+
+ {
+ pretty_printer pp;
+ pp.show_urls = true;
+ 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));
+ }
+}
+
/* Run all of the selftests within this file. */
void
@@ -2320,6 +2382,7 @@ pretty_print_c_tests ()
test_basic_printing ();
test_pp_format ();
test_prefixes_and_wrapping ();
+ test_urls ();
}
} // namespace selftest
diff --git a/gcc/pretty-print.h b/gcc/pretty-print.h
index 2d03b3f..c73fc30 100644
--- a/gcc/pretty-print.h
+++ b/gcc/pretty-print.h
@@ -273,6 +273,9 @@ public:
/* Nonzero means that text should be colorized. */
bool show_color;
+
+ /* Nonzero means that URLs should be emitted. */
+ bool show_urls;
};
static inline const char *
@@ -393,6 +396,9 @@ extern void pp_maybe_space (pretty_printer *);
extern void pp_begin_quote (pretty_printer *, bool);
extern void pp_end_quote (pretty_printer *, bool);
+extern void pp_begin_url (pretty_printer *pp, const char *url);
+extern void pp_end_url (pretty_printer *pp);
+
/* Switch into verbatim mode and return the old mode. */
static inline pp_wrapping_mode_t
pp_set_verbatim_wrapping_ (pretty_printer *pp)
diff --git a/gcc/testsuite/lib/prune.exp b/gcc/testsuite/lib/prune.exp
index 2885e87..176019f 100644
--- a/gcc/testsuite/lib/prune.exp
+++ b/gcc/testsuite/lib/prune.exp
@@ -21,7 +21,7 @@ load_lib multiline.exp
if ![info exists TEST_ALWAYS_FLAGS] {
set TEST_ALWAYS_FLAGS ""
}
-set TEST_ALWAYS_FLAGS "-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never $TEST_ALWAYS_FLAGS"
+set TEST_ALWAYS_FLAGS "-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -fdiagnostics-urls=never $TEST_ALWAYS_FLAGS"
proc prune_gcc_output { text } {
global srcdir
--
1.8.5.3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] (PR 87488)
2019-10-10 17:06 [PATCH 1/2] pretty-print: support URL escape sequences (PR 87488) David Malcolm
@ 2019-10-10 17:19 ` David Malcolm
2019-10-10 17:49 ` Manuel López-Ibáñez
2019-10-14 16:35 ` Michael Matz
2019-12-06 23:41 ` [PATCH 1/2] pretty-print: support URL escape sequences " Jakub Jelinek
1 sibling, 2 replies; 11+ messages in thread
From: David Malcolm @ 2019-10-10 17:19 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch uses the support for pretty-printing escaped URLs added in
the previous patch (PR 87488) so that in a sufficiently capable terminal
the [-Wname-of-option] emitted at the end of each diagnostic becomes a
hyperlink to the documentation for that option on the GCC website.
I've tested it with Fedora 30's GNOME Terminal (3.32.2 with VTE 0.56.3);
the option text is printed with a dotted underline, hovering shows the
URL, and on right-clicking it offers menu items to visit/copy the URL.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
Committed to trunk as r276843.
gcc/ChangeLog:
PR 87488
* Makefile.in (CFLAGS-opts.o): Pass in DOCUMENTATION_ROOT_URL via
-D.
* configure.ac (--with-documentation-root-url): New option.
* configure: Regenerate.
* diagnostic-format-json.cc (json_end_diagnostic): If there is an
option URL, add it as a new string field of the diagnostic option.
* diagnostic.c (diagnostic_initialize): Initialize get_option_url.
(print_option_information): If get_option_url is non-NULL, call
it, and if the result is non-NULL, potentially emit an escape
sequence to markup the option text with the resulting URL.
* diagnostic.h (diagnostic_context::get_option_url): New callback.
* doc/invoke.texi (-fdiagnostics-format=): Add "option_url" to
example of JSON output.
* opts-diagnostic.h (get_option_url): New decl.
* opts.c (get_option_url): New function.
* toplev.c (general_init): Initialize the get_option_url callback.
gcc/testsuite/ChangeLog:
PR 87488
* c-c++-common/diagnostic-format-json-2.c: Expect an "option_url"
field.
* c-c++-common/diagnostic-format-json-3.c: Likewise.
* gfortran.dg/diagnostic-format-json-2.F90: Likewise.
* gfortran.dg/diagnostic-format-json-3.F90: Likewise.
* jit.dg/test-error-array-bounds.c (create_code): Ensure that
error messages don't contain escaped URLs.
---
gcc/Makefile.in | 2 ++
gcc/configure.ac | 14 ++++++++++++++
gcc/diagnostic-format-json.cc | 11 +++++++++++
gcc/diagnostic.c | 12 ++++++++++++
gcc/diagnostic.h | 6 ++++++
gcc/doc/invoke.texi | 1 +
gcc/opts-diagnostic.h | 3 +++
gcc/opts.c | 21 +++++++++++++++++++++
.../c-c++-common/diagnostic-format-json-2.c | 1 +
.../c-c++-common/diagnostic-format-json-3.c | 1 +
.../c-c++-common/diagnostic-format-json-4.c | 10 ++++++++--
.../gfortran.dg/diagnostic-format-json-2.F90 | 1 +
.../gfortran.dg/diagnostic-format-json-3.F90 | 1 +
gcc/testsuite/jit.dg/test-error-array-bounds.c | 5 +++--
gcc/toplev.c | 1 +
15 files changed, 86 insertions(+), 4 deletions(-)
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 59adfaa..c82858f 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2142,6 +2142,8 @@ lto-wrapper$(exeext): $(LTO_WRAPPER_OBJS) libcommon-target.a $(LIBDEPS)
$(LTO_WRAPPER_OBJS) libcommon-target.a $(LIBS)
mv -f T$@ $@
+CFLAGS-opts.o += -DDOCUMENTATION_ROOT_URL=\"@DOCUMENTATION_ROOT_URL@\"
+
# Files used by all variants of C or by the stand-alone pre-processor.
CFLAGS-c-family/c-opts.o += @TARGET_SYSTEM_ROOT_DEFINE@
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 54a6715..62f4b26 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -960,6 +960,20 @@ AC_SUBST(CONFIGURE_SPECS)
ACX_PKGVERSION([GCC])
ACX_BUGURL([https://gcc.gnu.org/bugs/])
+# Allow overriding the default URL for documentation
+AC_ARG_WITH(documentation-root-url,
+ AS_HELP_STRING([--with-documentation-root-url=URL],
+ [Root for documentation URLs]),
+ [case "$withval" in
+ yes) AC_MSG_ERROR([documentation root URL not specified]) ;;
+ no) AC_MSG_ERROR([documentation root URL not specified]) ;;
+ *) DOCUMENTATION_ROOT_URL="$withval"
+ ;;
+ esac],
+ DOCUMENTATION_ROOT_URL="https://gcc.gnu.org/onlinedocs/gcc/"
+)
+AC_SUBST(DOCUMENTATION_ROOT_URL)
+
# Sanity check enable_languages in case someone does not run the toplevel
# configure # script.
AC_ARG_ENABLE(languages,
diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc
index 53c3b63..eb99952 100644
--- a/gcc/diagnostic-format-json.cc
+++ b/gcc/diagnostic-format-json.cc
@@ -154,6 +154,17 @@ json_end_diagnostic (diagnostic_context *context, diagnostic_info *diagnostic,
free (option_text);
}
+ if (context->get_option_url)
+ {
+ char *option_url = context->get_option_url (context,
+ diagnostic->option_index);
+ if (option_url)
+ {
+ diag_obj->set ("option_url", new json::string (option_url));
+ free (option_url);
+ }
+ }
+
/* If we've already emitted a diagnostic within this auto_diagnostic_group,
then add diag_obj to its "children" array. */
if (cur_group)
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 467cc39..a29bcf1 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -200,6 +200,7 @@ diagnostic_initialize (diagnostic_context *context, int n_opts)
context->option_enabled = NULL;
context->option_state = NULL;
context->option_name = NULL;
+ context->get_option_url = NULL;
context->last_location = UNKNOWN_LOCATION;
context->last_module = 0;
context->x_data = NULL;
@@ -912,11 +913,22 @@ print_option_information (diagnostic_context *context,
if (option_text)
{
+ char *option_url = NULL;
+ if (context->get_option_url)
+ option_url = context->get_option_url (context,
+ diagnostic->option_index);
pretty_printer *pp = context->printer;
pp_string (pp, " [");
pp_string (pp, colorize_start (pp_show_color (pp),
diagnostic_kind_color[diagnostic->kind]));
+ if (option_url)
+ pp_begin_url (pp, option_url);
pp_string (pp, option_text);
+ if (option_url)
+ {
+ pp_end_url (pp);
+ free (option_url);
+ }
pp_string (pp, colorize_stop (pp_show_color (pp)));
pp_character (pp, ']');
free (option_text);
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index f0ea8e8..91e4c50 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -194,6 +194,12 @@ struct diagnostic_context
May be passed 0 as well as the index of a particular option. */
char *(*option_name) (diagnostic_context *, int, diagnostic_t, diagnostic_t);
+ /* Client hook to return a URL describing the option that controls
+ a diagnostic. Returns malloced memory. May return NULL if no URL
+ is available. May be passed 0 as well as the index of a
+ particular option. */
+ char *(*get_option_url) (diagnostic_context *, int);
+
/* Auxiliary data for client. */
void *x_data;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index bdbcd95..085e8b0 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4116,6 +4116,7 @@ might be printed in JSON form (after formatting) like this:
],
"message": "this \u2018if\u2019 clause does not guard...",
"option": "-Wmisleading-indentation",
+ "option_url": "https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wmisleading-indentation",
"children": [
@{
"kind": "note",
diff --git a/gcc/opts-diagnostic.h b/gcc/opts-diagnostic.h
index 08110af..3f58ae9 100644
--- a/gcc/opts-diagnostic.h
+++ b/gcc/opts-diagnostic.h
@@ -22,4 +22,7 @@ along with GCC; see the file COPYING3. If not see
extern char *option_name (diagnostic_context *context, int option_index,
diagnostic_t orig_diag_kind, diagnostic_t diag_kind);
+
+extern char *get_option_url (diagnostic_context *context, int option_index);
+
#endif
diff --git a/gcc/opts.c b/gcc/opts.c
index f98f3f4..5ef6bf7 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -3236,3 +3236,24 @@ option_name (diagnostic_context *context, int option_index,
else
return NULL;
}
+
+/* Return malloced memory for a URL describing the option OPTION_INDEX
+ which enabled a diagnostic (context CONTEXT). */
+
+char *
+get_option_url (diagnostic_context *, int option_index)
+{
+ if (option_index)
+ /* DOCUMENTATION_ROOT_URL should be supplied via -D by the Makefile
+ (see --with-documentation-root-url).
+
+ Expect an anchor of the form "index-Wfoo" e.g.
+ <a name="index-Wformat"></a>, and thus an id within
+ the URL of "#index-Wformat". */
+ return concat (DOCUMENTATION_ROOT_URL,
+ "Warning-Options.html",
+ "#index", cl_options[option_index].opt_text,
+ NULL);
+ else
+ return NULL;
+}
diff --git a/gcc/testsuite/c-c++-common/diagnostic-format-json-2.c b/gcc/testsuite/c-c++-common/diagnostic-format-json-2.c
index 239c75e..557ccf8 100644
--- a/gcc/testsuite/c-c++-common/diagnostic-format-json-2.c
+++ b/gcc/testsuite/c-c++-common/diagnostic-format-json-2.c
@@ -10,6 +10,7 @@
/* { dg-regexp "\"kind\": \"warning\"" } */
/* { dg-regexp "\"message\": \"#warning message\"" } */
/* { dg-regexp "\"option\": \"-Wcpp\"" } */
+/* { dg-regexp "\"option_url\": \"https:\[^\n\r\"\]*#index-Wcpp\"" } */
/* { dg-regexp "\"caret\": \{" } */
/* { dg-regexp "\"file\": \"\[^\n\r\"\]*diagnostic-format-json-2.c\"" } */
diff --git a/gcc/testsuite/c-c++-common/diagnostic-format-json-3.c b/gcc/testsuite/c-c++-common/diagnostic-format-json-3.c
index 1c54eca..378205c 100644
--- a/gcc/testsuite/c-c++-common/diagnostic-format-json-3.c
+++ b/gcc/testsuite/c-c++-common/diagnostic-format-json-3.c
@@ -10,6 +10,7 @@
/* { dg-regexp "\"kind\": \"error\"" } */
/* { dg-regexp "\"message\": \"#warning message\"" } */
/* { dg-regexp "\"option\": \"-Werror=cpp\"" } */
+/* { dg-regexp "\"option_url\": \"https:\[^\n\r\"\]*#index-Wcpp\"" } */
/* { dg-regexp "\"caret\": \{" } */
/* { dg-regexp "\"file\": \"\[^\n\r\"\]*diagnostic-format-json-3.c\"" } */
diff --git a/gcc/testsuite/c-c++-common/diagnostic-format-json-4.c b/gcc/testsuite/c-c++-common/diagnostic-format-json-4.c
index 1c3b034..2738be6 100644
--- a/gcc/testsuite/c-c++-common/diagnostic-format-json-4.c
+++ b/gcc/testsuite/c-c++-common/diagnostic-format-json-4.c
@@ -30,13 +30,12 @@ int test (void)
/* { dg-regexp "\"line\": 8" } */
/* { dg-regexp "\"column\": 10" } */
-/* { dg-regexp "\"locations\": \[\[\{\}, \]*\]" } */
-
/* The outer diagnostic. */
/* { dg-regexp "\"kind\": \"warning\"" } */
/* { dg-regexp "\"message\": \"this 'if' clause does not guard...\"" } */
/* { dg-regexp "\"option\": \"-Wmisleading-indentation\"" } */
+/* { dg-regexp "\"option_url\": \"https:\[^\n\r\"\]*#index-Wmisleading-indentation\"" } */
/* { dg-regexp "\"caret\": \{" } */
/* { dg-regexp "\"file\": \"\[^\n\r\"\]*diagnostic-format-json-4.c\"" } */
@@ -48,6 +47,13 @@ int test (void)
/* { dg-regexp "\"line\": 6" } */
/* { dg-regexp "\"column\": 4" } */
+/* More from the nested diagnostic (we can't guarantee what order the
+ "file" keys are consumed). */
+
+/* { dg-regexp "\"locations\": \[\[\{\}, \]*\]" } */
+
+/* More from the outer diagnostic. */
+
/* { dg-regexp "\"locations\": \[\[\{\}, \]*\]" } */
/* { dg-regexp "\"children\": \[\[\{\}, \]*\]" } */
diff --git a/gcc/testsuite/gfortran.dg/diagnostic-format-json-2.F90 b/gcc/testsuite/gfortran.dg/diagnostic-format-json-2.F90
index a6d27d9..bebcf68 100644
--- a/gcc/testsuite/gfortran.dg/diagnostic-format-json-2.F90
+++ b/gcc/testsuite/gfortran.dg/diagnostic-format-json-2.F90
@@ -10,6 +10,7 @@
! { dg-regexp "\"kind\": \"warning\"" }
! { dg-regexp "\"message\": \"#warning message\"" }
! { dg-regexp "\"option\": \"-Wcpp\"" }
+! { dg-regexp "\"option_url\": \"\[^\n\r\"\]*#index-Wcpp\"" }
! { dg-regexp "\"caret\": \{" }
! { dg-regexp "\"file\": \"\[^\n\r\"\]*diagnostic-format-json-2.F90\"" }
diff --git a/gcc/testsuite/gfortran.dg/diagnostic-format-json-3.F90 b/gcc/testsuite/gfortran.dg/diagnostic-format-json-3.F90
index 702a937..7ab78eb 100644
--- a/gcc/testsuite/gfortran.dg/diagnostic-format-json-3.F90
+++ b/gcc/testsuite/gfortran.dg/diagnostic-format-json-3.F90
@@ -10,6 +10,7 @@
! { dg-regexp "\"kind\": \"error\"" }
! { dg-regexp "\"message\": \"#warning message\"" }
! { dg-regexp "\"option\": \"-Werror=cpp\"" }
+! { dg-regexp "\"option_url\": \"\[^\n\r\"\]*#index-Wcpp\"" }
! { dg-regexp "\"caret\": \{" }
! { dg-regexp "\"file\": \"\[^\n\r\"\]*diagnostic-format-json-3.F90\"" }
diff --git a/gcc/testsuite/jit.dg/test-error-array-bounds.c b/gcc/testsuite/jit.dg/test-error-array-bounds.c
index 889c517..cd9361f 100644
--- a/gcc/testsuite/jit.dg/test-error-array-bounds.c
+++ b/gcc/testsuite/jit.dg/test-error-array-bounds.c
@@ -20,9 +20,10 @@ create_code (gcc_jit_context *ctxt, void *user_data)
gcc_jit_context_add_command_line_option (ctxt, "-Warray-bounds");
gcc_jit_context_add_command_line_option (ctxt, "-ftree-vrp");
- /* Ensure that the error message doesn't contain colorization codes,
- even if run at a TTY. */
+ /* Ensure that the error message doesn't contain colorization codes
+ or escaped URLs, even if run at a TTY. */
gcc_jit_context_add_command_line_option (ctxt, "-fdiagnostics-color=never");
+ gcc_jit_context_add_command_line_option (ctxt, "-fdiagnostics-urls=never");
gcc_jit_type *char_type =
gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_CHAR);
diff --git a/gcc/toplev.c b/gcc/toplev.c
index d741a66..df4ca01 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1110,6 +1110,7 @@ general_init (const char *argv0, bool init_signals)
global_dc->option_enabled = option_enabled;
global_dc->option_state = &global_options;
global_dc->option_name = option_name;
+ global_dc->get_option_url = get_option_url;
if (init_signals)
{
--
1.8.5.3
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] (PR 87488)
2019-10-10 17:19 ` [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] " David Malcolm
@ 2019-10-10 17:49 ` Manuel López-Ibáñez
2019-10-11 3:54 ` Eric Gallager
2019-10-14 16:35 ` Michael Matz
1 sibling, 1 reply; 11+ messages in thread
From: Manuel López-Ibáñez @ 2019-10-10 17:49 UTC (permalink / raw)
To: David Malcolm, GCC Patches
Hi David,
While I agree that this is quite cool to have, the following:
> + /* DOCUMENTATION_ROOT_URL should be supplied via -D by the Makefile
> + (see --with-documentation-root-url).
> +
> + Expect an anchor of the form "index-Wfoo" e.g.
> + <a name="index-Wformat"></a>, and thus an id within
> + the URL of "#index-Wformat". */
> + return concat (DOCUMENTATION_ROOT_URL,
> + "Warning-Options.html",
> + "#index", cl_options[option_index].opt_text,
> + NULL);
will not work for many -W options:
https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html#C_002b_002b-Dialect-Options
(scroll to the bottom)
https://gcc.gnu.org/onlinedocs/gcc/Objective-C-and-Objective-C_002b_002b-Dialect-Options.html#Objective-C-and-Objective-C_002b_002b-Dialect-Options
(scroll to the bottom)
https://gcc.gnu.org/onlinedocs/gcc-9.2.0/cpp/Invocation.html#index-Wcomment
https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gfortran/Error-and-Warning-Options.html#Error-and-Warning-Options
I would argue that some options are documented in the wrong place (I think all
C/C++ -W options should just go to Warning-Options.html), but I also believe
the HTML page should be language dependent.
Cheers,
Manuel.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] (PR 87488)
2019-10-10 17:49 ` Manuel López-Ibáñez
@ 2019-10-11 3:54 ` Eric Gallager
0 siblings, 0 replies; 11+ messages in thread
From: Eric Gallager @ 2019-10-11 3:54 UTC (permalink / raw)
To: Manuel López-Ibáñez; +Cc: David Malcolm, GCC Patches
On 10/10/19, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> Hi David,
>
> While I agree that this is quite cool to have, the following:
>
>> + /* DOCUMENTATION_ROOT_URL should be supplied via -D by the Makefile
>> + (see --with-documentation-root-url).
>> +
>> + Expect an anchor of the form "index-Wfoo" e.g.
>> + <a name="index-Wformat"></a>, and thus an id within
>> + the URL of "#index-Wformat". */
>> + return concat (DOCUMENTATION_ROOT_URL,
>> + "Warning-Options.html",
>> + "#index", cl_options[option_index].opt_text,
>> + NULL);
>
> will not work for many -W options:
>
> https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html#C_002b_002b-Dialect-Options
> (scroll to the bottom)
>
> https://gcc.gnu.org/onlinedocs/gcc/Objective-C-and-Objective-C_002b_002b-Dialect-Options.html#Objective-C-and-Objective-C_002b_002b-Dialect-Options
> (scroll to the bottom)
>
> https://gcc.gnu.org/onlinedocs/gcc-9.2.0/cpp/Invocation.html#index-Wcomment
>
> https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gfortran/Error-and-Warning-Options.html#Error-and-Warning-Options
>
> I would argue that some options are documented in the wrong place (I think all
> C/C++ -W options should just go to Warning-Options.html),
That's bug 71283, for reference:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71283
> but I also believe the HTML page should be language dependent.
>
> Cheers,
>
> Manuel.
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] (PR 87488)
2019-10-10 17:19 ` [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] " David Malcolm
2019-10-10 17:49 ` Manuel López-Ibáñez
@ 2019-10-14 16:35 ` Michael Matz
1 sibling, 0 replies; 11+ messages in thread
From: Michael Matz @ 2019-10-14 16:35 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
Hi,
On Thu, 10 Oct 2019, David Malcolm wrote:
> This patch uses the support for pretty-printing escaped URLs added in
> the previous patch (PR 87488) so that in a sufficiently capable terminal
> the [-Wname-of-option] emitted at the end of each diagnostic becomes a
> hyperlink to the documentation for that option on the GCC website.
>
> I've tested it with Fedora 30's GNOME Terminal (3.32.2 with VTE 0.56.3);
> the option text is printed with a dotted underline, hovering shows the
> URL, and on right-clicking it offers menu items to visit/copy the URL.
I don't think you can enable this by default as long as even the gist page
only claims that 8 of 22 terminal emulators support this (and among those
not supporting it are xterm, and all default terminal emulators of desktop
environment other than GNOME). screen and tmux don't support it either,
and the konsole feature request for this has a discussion about problems
with the design from a security p.o.v. so there will probably be emulators
that never implement this.
That there's no way of detecting the capability, and, worse, that the
normal default behaviour of terminals is to ignore unknown OCS sequences
(which means that you don't see the URL but only the replacement text)
also means information loss.
Tieing the hyperlinks default capability to the colorize capability is bad
as well, because colorization is an old concept, and better yet, supported
by terminfo and termcap capabilities (hence can be translated meaningfully
by terminal multiplexers), unlike the hyperlinking.
So, for the above reasons IMHO the default can only be false.
Ciao,
Michael.
>
> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
>
> Committed to trunk as r276843.
>
> gcc/ChangeLog:
> PR 87488
> * Makefile.in (CFLAGS-opts.o): Pass in DOCUMENTATION_ROOT_URL via
> -D.
> * configure.ac (--with-documentation-root-url): New option.
> * configure: Regenerate.
> * diagnostic-format-json.cc (json_end_diagnostic): If there is an
> option URL, add it as a new string field of the diagnostic option.
> * diagnostic.c (diagnostic_initialize): Initialize get_option_url.
> (print_option_information): If get_option_url is non-NULL, call
> it, and if the result is non-NULL, potentially emit an escape
> sequence to markup the option text with the resulting URL.
> * diagnostic.h (diagnostic_context::get_option_url): New callback.
> * doc/invoke.texi (-fdiagnostics-format=): Add "option_url" to
> example of JSON output.
> * opts-diagnostic.h (get_option_url): New decl.
> * opts.c (get_option_url): New function.
> * toplev.c (general_init): Initialize the get_option_url callback.
>
> gcc/testsuite/ChangeLog:
> PR 87488
> * c-c++-common/diagnostic-format-json-2.c: Expect an "option_url"
> field.
> * c-c++-common/diagnostic-format-json-3.c: Likewise.
> * gfortran.dg/diagnostic-format-json-2.F90: Likewise.
> * gfortran.dg/diagnostic-format-json-3.F90: Likewise.
> * jit.dg/test-error-array-bounds.c (create_code): Ensure that
> error messages don't contain escaped URLs.
> ---
> gcc/Makefile.in | 2 ++
> gcc/configure.ac | 14 ++++++++++++++
> gcc/diagnostic-format-json.cc | 11 +++++++++++
> gcc/diagnostic.c | 12 ++++++++++++
> gcc/diagnostic.h | 6 ++++++
> gcc/doc/invoke.texi | 1 +
> gcc/opts-diagnostic.h | 3 +++
> gcc/opts.c | 21 +++++++++++++++++++++
> .../c-c++-common/diagnostic-format-json-2.c | 1 +
> .../c-c++-common/diagnostic-format-json-3.c | 1 +
> .../c-c++-common/diagnostic-format-json-4.c | 10 ++++++++--
> .../gfortran.dg/diagnostic-format-json-2.F90 | 1 +
> .../gfortran.dg/diagnostic-format-json-3.F90 | 1 +
> gcc/testsuite/jit.dg/test-error-array-bounds.c | 5 +++--
> gcc/toplev.c | 1 +
> 15 files changed, 86 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 59adfaa..c82858f 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -2142,6 +2142,8 @@ lto-wrapper$(exeext): $(LTO_WRAPPER_OBJS) libcommon-target.a $(LIBDEPS)
> $(LTO_WRAPPER_OBJS) libcommon-target.a $(LIBS)
> mv -f T$@ $@
>
> +CFLAGS-opts.o += -DDOCUMENTATION_ROOT_URL=\"@DOCUMENTATION_ROOT_URL@\"
> +
> # Files used by all variants of C or by the stand-alone pre-processor.
>
> CFLAGS-c-family/c-opts.o += @TARGET_SYSTEM_ROOT_DEFINE@
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 54a6715..62f4b26 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -960,6 +960,20 @@ AC_SUBST(CONFIGURE_SPECS)
> ACX_PKGVERSION([GCC])
> ACX_BUGURL([https://gcc.gnu.org/bugs/])
>
> +# Allow overriding the default URL for documentation
> +AC_ARG_WITH(documentation-root-url,
> + AS_HELP_STRING([--with-documentation-root-url=URL],
> + [Root for documentation URLs]),
> + [case "$withval" in
> + yes) AC_MSG_ERROR([documentation root URL not specified]) ;;
> + no) AC_MSG_ERROR([documentation root URL not specified]) ;;
> + *) DOCUMENTATION_ROOT_URL="$withval"
> + ;;
> + esac],
> + DOCUMENTATION_ROOT_URL="https://gcc.gnu.org/onlinedocs/gcc/"
> +)
> +AC_SUBST(DOCUMENTATION_ROOT_URL)
> +
> # Sanity check enable_languages in case someone does not run the toplevel
> # configure # script.
> AC_ARG_ENABLE(languages,
> diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc
> index 53c3b63..eb99952 100644
> --- a/gcc/diagnostic-format-json.cc
> +++ b/gcc/diagnostic-format-json.cc
> @@ -154,6 +154,17 @@ json_end_diagnostic (diagnostic_context *context, diagnostic_info *diagnostic,
> free (option_text);
> }
>
> + if (context->get_option_url)
> + {
> + char *option_url = context->get_option_url (context,
> + diagnostic->option_index);
> + if (option_url)
> + {
> + diag_obj->set ("option_url", new json::string (option_url));
> + free (option_url);
> + }
> + }
> +
> /* If we've already emitted a diagnostic within this auto_diagnostic_group,
> then add diag_obj to its "children" array. */
> if (cur_group)
> diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
> index 467cc39..a29bcf1 100644
> --- a/gcc/diagnostic.c
> +++ b/gcc/diagnostic.c
> @@ -200,6 +200,7 @@ diagnostic_initialize (diagnostic_context *context, int n_opts)
> context->option_enabled = NULL;
> context->option_state = NULL;
> context->option_name = NULL;
> + context->get_option_url = NULL;
> context->last_location = UNKNOWN_LOCATION;
> context->last_module = 0;
> context->x_data = NULL;
> @@ -912,11 +913,22 @@ print_option_information (diagnostic_context *context,
>
> if (option_text)
> {
> + char *option_url = NULL;
> + if (context->get_option_url)
> + option_url = context->get_option_url (context,
> + diagnostic->option_index);
> pretty_printer *pp = context->printer;
> pp_string (pp, " [");
> pp_string (pp, colorize_start (pp_show_color (pp),
> diagnostic_kind_color[diagnostic->kind]));
> + if (option_url)
> + pp_begin_url (pp, option_url);
> pp_string (pp, option_text);
> + if (option_url)
> + {
> + pp_end_url (pp);
> + free (option_url);
> + }
> pp_string (pp, colorize_stop (pp_show_color (pp)));
> pp_character (pp, ']');
> free (option_text);
> diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
> index f0ea8e8..91e4c50 100644
> --- a/gcc/diagnostic.h
> +++ b/gcc/diagnostic.h
> @@ -194,6 +194,12 @@ struct diagnostic_context
> May be passed 0 as well as the index of a particular option. */
> char *(*option_name) (diagnostic_context *, int, diagnostic_t, diagnostic_t);
>
> + /* Client hook to return a URL describing the option that controls
> + a diagnostic. Returns malloced memory. May return NULL if no URL
> + is available. May be passed 0 as well as the index of a
> + particular option. */
> + char *(*get_option_url) (diagnostic_context *, int);
> +
> /* Auxiliary data for client. */
> void *x_data;
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index bdbcd95..085e8b0 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -4116,6 +4116,7 @@ might be printed in JSON form (after formatting) like this:
> ],
> "message": "this \u2018if\u2019 clause does not guard...",
> "option": "-Wmisleading-indentation",
> + "option_url": "https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wmisleading-indentation",
> "children": [
> @{
> "kind": "note",
> diff --git a/gcc/opts-diagnostic.h b/gcc/opts-diagnostic.h
> index 08110af..3f58ae9 100644
> --- a/gcc/opts-diagnostic.h
> +++ b/gcc/opts-diagnostic.h
> @@ -22,4 +22,7 @@ along with GCC; see the file COPYING3. If not see
>
> extern char *option_name (diagnostic_context *context, int option_index,
> diagnostic_t orig_diag_kind, diagnostic_t diag_kind);
> +
> +extern char *get_option_url (diagnostic_context *context, int option_index);
> +
> #endif
> diff --git a/gcc/opts.c b/gcc/opts.c
> index f98f3f4..5ef6bf7 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -3236,3 +3236,24 @@ option_name (diagnostic_context *context, int option_index,
> else
> return NULL;
> }
> +
> +/* Return malloced memory for a URL describing the option OPTION_INDEX
> + which enabled a diagnostic (context CONTEXT). */
> +
> +char *
> +get_option_url (diagnostic_context *, int option_index)
> +{
> + if (option_index)
> + /* DOCUMENTATION_ROOT_URL should be supplied via -D by the Makefile
> + (see --with-documentation-root-url).
> +
> + Expect an anchor of the form "index-Wfoo" e.g.
> + <a name="index-Wformat"></a>, and thus an id within
> + the URL of "#index-Wformat". */
> + return concat (DOCUMENTATION_ROOT_URL,
> + "Warning-Options.html",
> + "#index", cl_options[option_index].opt_text,
> + NULL);
> + else
> + return NULL;
> +}
> diff --git a/gcc/testsuite/c-c++-common/diagnostic-format-json-2.c b/gcc/testsuite/c-c++-common/diagnostic-format-json-2.c
> index 239c75e..557ccf8 100644
> --- a/gcc/testsuite/c-c++-common/diagnostic-format-json-2.c
> +++ b/gcc/testsuite/c-c++-common/diagnostic-format-json-2.c
> @@ -10,6 +10,7 @@
> /* { dg-regexp "\"kind\": \"warning\"" } */
> /* { dg-regexp "\"message\": \"#warning message\"" } */
> /* { dg-regexp "\"option\": \"-Wcpp\"" } */
> +/* { dg-regexp "\"option_url\": \"https:\[^\n\r\"\]*#index-Wcpp\"" } */
>
> /* { dg-regexp "\"caret\": \{" } */
> /* { dg-regexp "\"file\": \"\[^\n\r\"\]*diagnostic-format-json-2.c\"" } */
> diff --git a/gcc/testsuite/c-c++-common/diagnostic-format-json-3.c b/gcc/testsuite/c-c++-common/diagnostic-format-json-3.c
> index 1c54eca..378205c 100644
> --- a/gcc/testsuite/c-c++-common/diagnostic-format-json-3.c
> +++ b/gcc/testsuite/c-c++-common/diagnostic-format-json-3.c
> @@ -10,6 +10,7 @@
> /* { dg-regexp "\"kind\": \"error\"" } */
> /* { dg-regexp "\"message\": \"#warning message\"" } */
> /* { dg-regexp "\"option\": \"-Werror=cpp\"" } */
> +/* { dg-regexp "\"option_url\": \"https:\[^\n\r\"\]*#index-Wcpp\"" } */
>
> /* { dg-regexp "\"caret\": \{" } */
> /* { dg-regexp "\"file\": \"\[^\n\r\"\]*diagnostic-format-json-3.c\"" } */
> diff --git a/gcc/testsuite/c-c++-common/diagnostic-format-json-4.c b/gcc/testsuite/c-c++-common/diagnostic-format-json-4.c
> index 1c3b034..2738be6 100644
> --- a/gcc/testsuite/c-c++-common/diagnostic-format-json-4.c
> +++ b/gcc/testsuite/c-c++-common/diagnostic-format-json-4.c
> @@ -30,13 +30,12 @@ int test (void)
> /* { dg-regexp "\"line\": 8" } */
> /* { dg-regexp "\"column\": 10" } */
>
> -/* { dg-regexp "\"locations\": \[\[\{\}, \]*\]" } */
> -
> /* The outer diagnostic. */
>
> /* { dg-regexp "\"kind\": \"warning\"" } */
> /* { dg-regexp "\"message\": \"this 'if' clause does not guard...\"" } */
> /* { dg-regexp "\"option\": \"-Wmisleading-indentation\"" } */
> +/* { dg-regexp "\"option_url\": \"https:\[^\n\r\"\]*#index-Wmisleading-indentation\"" } */
>
> /* { dg-regexp "\"caret\": \{" } */
> /* { dg-regexp "\"file\": \"\[^\n\r\"\]*diagnostic-format-json-4.c\"" } */
> @@ -48,6 +47,13 @@ int test (void)
> /* { dg-regexp "\"line\": 6" } */
> /* { dg-regexp "\"column\": 4" } */
>
> +/* More from the nested diagnostic (we can't guarantee what order the
> + "file" keys are consumed). */
> +
> +/* { dg-regexp "\"locations\": \[\[\{\}, \]*\]" } */
> +
> +/* More from the outer diagnostic. */
> +
> /* { dg-regexp "\"locations\": \[\[\{\}, \]*\]" } */
>
> /* { dg-regexp "\"children\": \[\[\{\}, \]*\]" } */
> diff --git a/gcc/testsuite/gfortran.dg/diagnostic-format-json-2.F90 b/gcc/testsuite/gfortran.dg/diagnostic-format-json-2.F90
> index a6d27d9..bebcf68 100644
> --- a/gcc/testsuite/gfortran.dg/diagnostic-format-json-2.F90
> +++ b/gcc/testsuite/gfortran.dg/diagnostic-format-json-2.F90
> @@ -10,6 +10,7 @@
> ! { dg-regexp "\"kind\": \"warning\"" }
> ! { dg-regexp "\"message\": \"#warning message\"" }
> ! { dg-regexp "\"option\": \"-Wcpp\"" }
> +! { dg-regexp "\"option_url\": \"\[^\n\r\"\]*#index-Wcpp\"" }
>
> ! { dg-regexp "\"caret\": \{" }
> ! { dg-regexp "\"file\": \"\[^\n\r\"\]*diagnostic-format-json-2.F90\"" }
> diff --git a/gcc/testsuite/gfortran.dg/diagnostic-format-json-3.F90 b/gcc/testsuite/gfortran.dg/diagnostic-format-json-3.F90
> index 702a937..7ab78eb 100644
> --- a/gcc/testsuite/gfortran.dg/diagnostic-format-json-3.F90
> +++ b/gcc/testsuite/gfortran.dg/diagnostic-format-json-3.F90
> @@ -10,6 +10,7 @@
> ! { dg-regexp "\"kind\": \"error\"" }
> ! { dg-regexp "\"message\": \"#warning message\"" }
> ! { dg-regexp "\"option\": \"-Werror=cpp\"" }
> +! { dg-regexp "\"option_url\": \"\[^\n\r\"\]*#index-Wcpp\"" }
>
> ! { dg-regexp "\"caret\": \{" }
> ! { dg-regexp "\"file\": \"\[^\n\r\"\]*diagnostic-format-json-3.F90\"" }
> diff --git a/gcc/testsuite/jit.dg/test-error-array-bounds.c b/gcc/testsuite/jit.dg/test-error-array-bounds.c
> index 889c517..cd9361f 100644
> --- a/gcc/testsuite/jit.dg/test-error-array-bounds.c
> +++ b/gcc/testsuite/jit.dg/test-error-array-bounds.c
> @@ -20,9 +20,10 @@ create_code (gcc_jit_context *ctxt, void *user_data)
> gcc_jit_context_add_command_line_option (ctxt, "-Warray-bounds");
> gcc_jit_context_add_command_line_option (ctxt, "-ftree-vrp");
>
> - /* Ensure that the error message doesn't contain colorization codes,
> - even if run at a TTY. */
> + /* Ensure that the error message doesn't contain colorization codes
> + or escaped URLs, even if run at a TTY. */
> gcc_jit_context_add_command_line_option (ctxt, "-fdiagnostics-color=never");
> + gcc_jit_context_add_command_line_option (ctxt, "-fdiagnostics-urls=never");
>
> gcc_jit_type *char_type =
> gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_CHAR);
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index d741a66..df4ca01 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1110,6 +1110,7 @@ general_init (const char *argv0, bool init_signals)
> global_dc->option_enabled = option_enabled;
> global_dc->option_state = &global_options;
> global_dc->option_name = option_name;
> + global_dc->get_option_url = get_option_url;
>
> if (init_signals)
> {
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] pretty-print: support URL escape sequences (PR 87488)
2019-10-10 17:06 [PATCH 1/2] pretty-print: support URL escape sequences (PR 87488) David Malcolm
2019-10-10 17:19 ` [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] " David Malcolm
@ 2019-12-06 23:41 ` Jakub Jelinek
2019-12-07 1:31 ` David Malcolm
1 sibling, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2019-12-06 23:41 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
On Thu, Oct 10, 2019 at 01:06:13PM -0400, David Malcolm wrote:
> https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda
> describes an emerging standard for embedding URLs in escape sequences
> for marking up text output. This is supported e.g. by recent releases
> of GNOME Terminal.
Unfortunately, as reported by several people, the
OSC 8 ;; URL ST Text OSC 8 ;; ST
sequence renders badly on recentish konsole5 terminal emulator, which is
something a lot of people use.
While the above page suggests the use of ST rather than BEL, in practice
that at least currently does better job.
Tested with
echo -e '\e]8;;http://example.com\aThis is a link\e]8;;\a\n\e]8;;http://example.com\e\\This is a link\e]8;;\e\\\n'
on various terminals:
gnome-terminal-3.30.2 both lines the same, URLs work
konsole5-18.12.3 the first line normal text, the second one wrapped in
between \s, i.e. \This is a link\ , URLs don't work
xterm-334 both lines the same, normal text, URLs don't work
rxvt-2.7.10 ditto
xterm-314 ditto
konsole5-15.12.1 ditto
linux kernel 5.0.13 console ditto
linux kernel 4.4.14 console ditto
gnome-terminal-3.16.2 prints garbage around and both the URL and the text
are visible, but the line with ST has more garbage
than line with BEL
BEL instead of ST is also what ls -l --hyperlink prints.
Ok for trunk?
2019-12-07 Jakub Jelinek <jakub@redhat.com>
* pretty-print.c (pp_begin_url, pp_end_url, test_urls): Use BEL
instead of ST sequence to terminate OSC 8 strings.
--- gcc/pretty-print.c.jj 2019-10-11 09:29:15.103953133 +0200
+++ gcc/pretty-print.c 2019-12-07 00:17:00.860500837 +0100
@@ -2043,7 +2043,10 @@ identifier_to_locale (const char *ident)
>
> OSC 8 ; ; ST
>
- > OSC (operating system command) is typically ESC ]. */
+ > OSC (operating system command) is typically ESC ].
+
+ Use BEL instead of ST, as that is currently rendered better in some
+ terminal emulators that don't support OSC 8, like konsole5. */
/* If URL-printing is enabled, write an "open URL" escape sequence to PP
for the given URL. */
@@ -2052,7 +2055,7 @@ void
pp_begin_url (pretty_printer *pp, const char *url)
{
if (pp->show_urls)
- pp_printf (pp, "\33]8;;%s\33\\", url);
+ pp_printf (pp, "\33]8;;%s\a", url);
}
/* If URL-printing is enabled, write a "close URL" escape sequence to PP. */
@@ -2061,7 +2064,7 @@ void
pp_end_url (pretty_printer *pp)
{
if (pp->show_urls)
- pp_string (pp, "\33]8;;\33\\");
+ pp_string (pp, "\33]8;;\a");
}
#if CHECKING_P
@@ -2369,7 +2372,7 @@ test_urls ()
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\\",
+ ASSERT_STREQ ("\33]8;;http://example.com\aThis is a link\33]8;;\a",
pp_formatted_text (&pp));
}
}
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] pretty-print: support URL escape sequences (PR 87488)
2019-12-06 23:41 ` [PATCH 1/2] pretty-print: support URL escape sequences " Jakub Jelinek
@ 2019-12-07 1:31 ` David Malcolm
0 siblings, 0 replies; 11+ messages in thread
From: David Malcolm @ 2019-12-07 1:31 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Sat, 2019-12-07 at 00:41 +0100, Jakub Jelinek wrote:
> On Thu, Oct 10, 2019 at 01:06:13PM -0400, David Malcolm wrote:
> > https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda
> > describes an emerging standard for embedding URLs in escape
> > sequences
> > for marking up text output. This is supported e.g. by recent
> > releases
> > of GNOME Terminal.
>
> Unfortunately, as reported by several people, the
> OSC 8 ;; URL ST Text OSC 8 ;; ST
> sequence renders badly on recentish konsole5 terminal emulator, which
> is
> something a lot of people use.
> While the above page suggests the use of ST rather than BEL, in
> practice
> that at least currently does better job.
> Tested with
> echo -e '\e]8;;http://example.com\aThis is a link\e]8;;\a\n\e]8;;
> http://example.com\e\\This is a link\e]8;;\e\\\n'
> on various terminals:
> gnome-terminal-3.30.2 both lines the same, URLs work
> konsole5-18.12.3 the first line normal text, the second one wrapped
> in
> between \s, i.e. \This is a link\ , URLs don't work
> xterm-334 both lines the same, normal text, URLs don't work
> rxvt-2.7.10 ditto
> xterm-314 ditto
> konsole5-15.12.1 ditto
> linux kernel 5.0.13 console ditto
> linux kernel 4.4.14 console ditto
> gnome-terminal-3.16.2 prints garbage around and both the URL and the
> text
> are visible, but the line with ST has more
> garbage
> than line with BEL
>
> BEL instead of ST is also what ls -l --hyperlink prints.
systemd also uses BEL rather than ST.
> Ok for trunk?
OK; please reference PR 87488 in the ChangeLog.
> 2019-12-07 Jakub Jelinek <jakub@redhat.com>
>
> * pretty-print.c (pp_begin_url, pp_end_url, test_urls): Use BEL
> instead of ST sequence to terminate OSC 8 strings.
>
> --- gcc/pretty-print.c.jj 2019-10-11 09:29:15.103953133 +0200
> +++ gcc/pretty-print.c 2019-12-07 00:17:00.860500837 +0100
> @@ -2043,7 +2043,10 @@ identifier_to_locale (const char *ident)
> >
> > OSC 8 ; ; ST
> >
> - > OSC (operating system command) is typically ESC ]. */
> + > OSC (operating system command) is typically ESC ].
> +
> + Use BEL instead of ST, as that is currently rendered better in
> some
> + terminal emulators that don't support OSC 8, like konsole5. */
>
> /* If URL-printing is enabled, write an "open URL" escape sequence
> to PP
> for the given URL. */
> @@ -2052,7 +2055,7 @@ void
> pp_begin_url (pretty_printer *pp, const char *url)
> {
> if (pp->show_urls)
> - pp_printf (pp, "\33]8;;%s\33\\", url);
> + pp_printf (pp, "\33]8;;%s\a", url);
> }
>
> /* If URL-printing is enabled, write a "close URL" escape sequence
> to PP. */
> @@ -2061,7 +2064,7 @@ void
> pp_end_url (pretty_printer *pp)
> {
> if (pp->show_urls)
> - pp_string (pp, "\33]8;;\33\\");
> + pp_string (pp, "\33]8;;\a");
> }
>
> #if CHECKING_P
> @@ -2369,7 +2372,7 @@ test_urls ()
> 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\\",
> + ASSERT_STREQ ("\33]8;;http://example.com\aThis is a
> link\33]8;;\a",
> pp_formatted_text (&pp));
> }
> }
>
>
> Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] (PR 87488)
@ 2019-10-12 7:28 Bernd Edlinger
2019-10-12 14:51 ` David Malcolm
0 siblings, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2019-10-12 7:28 UTC (permalink / raw)
To: David Malcolm, gcc-patches
Hi David,
I just wanted to say that this does not work right on ubuntu 14.04 at least:
g++ -Wshadow=local -Wno-shadow=compatible-local -S Wshadow-1.c
Wshadow-1.c: In function 'void foo(int*, int*)':
Wshadow-1.c:8:9: warning: declaration of 'int d' shadows a parameter [^[]8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow=local-Wshadow=local^[]8;;]
8 | int d = 0; /* { dg-warning "Wshadow=local" } */
| ^
Wshadow-1.c:4:23: note: shadowed declaration is here
4 | void foo(int *c, int *d) /* { dg-bogus "Wshadow" } */
| ~~~~~^
Wshadow-1.c: In function 'void bar(int*, int*)':
Wshadow-1.c:15:15: warning: declaration of 'c' shadows a global declaration [^[]8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow-Wshadow^[]8;;]
15 | void bar(int *c, int *d) /* { dg-warning "Wshadow" } */
| ~~~~~^
Wshadow-1.c:3:5: note: shadowed declaration is here
3 | int c;
| ^
Wshadow-1.c:19:9: warning: declaration of 'int d' shadows a parameter [^[]8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow-Wshadow^[]8;;]
19 | int d = 0; /* { dg-warning "Wshadow" } */
| ^
Wshadow-1.c:15:23: note: shadowed declaration is here
15 | void bar(int *c, int *d) /* { dg-warning "Wshadow" } */
| ~~~~~^
Wshadow-1.c:22:10: warning: declaration of 'e' shadows a previous local [^[]8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow-Wshadow^[]8;;]
22 | int *e = 0; /* { dg-warning "Wshadow" } */
| ^
Wshadow-1.c:17:8: note: shadowed declaration is here
17 | int *e = d;
| ^
Coloring works, even the hyperlinks seem to work, when clicked at, although they jump just
to the top of the page, and one has to scroll down to the warning manually.
But the warning name is very hard to spot in all that glibberish long text :-(
Do you see a way to find out if the escape sequences are supported,
or how to disable this function per configure option?
Thanks
Bernd.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] (PR 87488)
2019-10-12 7:28 [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] " Bernd Edlinger
@ 2019-10-12 14:51 ` David Malcolm
2019-10-12 15:43 ` Bernd Edlinger
2019-10-12 18:10 ` Segher Boessenkool
0 siblings, 2 replies; 11+ messages in thread
From: David Malcolm @ 2019-10-12 14:51 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches
On Sat, 2019-10-12 at 07:00 +0000, Bernd Edlinger wrote:
> Hi David,
>
> I just wanted to say that this does not work right on ubuntu 14.04 at
> least:
>
> g++ -Wshadow=local -Wno-shadow=compatible-local -S Wshadow-1.c
> Wshadow-1.c: In function 'void foo(int*, int*)':
> Wshadow-1.c:8:9: warning: declaration of 'int d' shadows a parameter
> [^[]8;;
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow=local-Wshadow=local^[]8
> ;;]
> 8 | int d = 0; /* { dg-warning "Wshadow=local" }
> */
> | ^
> Wshadow-1.c:4:23: note: shadowed declaration is here
> 4 | void foo(int *c, int *d) /* { dg-bogus "Wshadow" } */
> | ~~~~~^
> Wshadow-1.c: In function 'void bar(int*, int*)':
> Wshadow-1.c:15:15: warning: declaration of 'c' shadows a global
> declaration [^[]8;;
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow-Wshadow^[]8
> ;;]
> 15 | void bar(int *c, int *d) /* { dg-warning "Wshadow" } */
> | ~~~~~^
> Wshadow-1.c:3:5: note: shadowed declaration is here
> 3 | int c;
> | ^
> Wshadow-1.c:19:9: warning: declaration of 'int d' shadows a parameter
> [^[]8;;
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow-Wshadow^[]8
> ;;]
> 19 | int d = 0; /* { dg-warning "Wshadow" } */
> | ^
> Wshadow-1.c:15:23: note: shadowed declaration is here
> 15 | void bar(int *c, int *d) /* { dg-warning "Wshadow" } */
> | ~~~~~^
> Wshadow-1.c:22:10: warning: declaration of 'e' shadows a previous
> local [^[]8;;
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow-Wshadow^[]8
> ;;]
> 22 | int *e = 0; /* { dg-warning "Wshadow" } */
> | ^
> Wshadow-1.c:17:8: note: shadowed declaration is here
> 17 | int *e = d;
> | ^
>
> Coloring works, even the hyperlinks seem to work, when clicked at,
> although they jump just
> to the top of the page, and one has to scroll down to the warning
> manually.
> But the warning name is very hard to spot in all that glibberish long
> text :-(
Thanks for the report.
Bother.
There are (at least) two issues here.
(a) It sounds like your terminal doesn't handle the escape sequence my
patch is emitting. The expected outcome is that the terminal embeds
this URL:
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow=local
within:
[-Wshadow=local]
i.e. equivalent to:
[<a href="
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow=local"</a>
;;]
It seems like instead it's displaying this:
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow=local-Wshadow=local%1B]8
When you say "even the hyperlinks seem to work" it seems like that URL
fragment is being handled by the terminal's regular "look for URLs in
the text" logic.
Does your terminal have a "Copy URL" option (perhaps on a right-click
menu)? If so, what URL is copied?
Which terminal (and version) are you seeing this with? (if gnome
terminal, which vte version?)
What happens if you try this at the terminal:
printf '\e]8;;http://example.com\e\\This is a link\e]8;;\e\\\n'
(hopefully that doesn't get mangled by my mailer; this is the example
from:
https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda
)
I don't yet know if there's a way to query the terminal to see if the
escape is supported. My hope was that they would be silently
discarded.
> Do you see a way to find out if the escape sequences are supported,
> or how to disable this function per configure option?
Do you mean a GCC configure-time option to change the default? We have
that for colors; we could add one for URLs. (Though I'd prefer to know
more about the scope of the problem first)
The other problem:
(b) The URL itself is wrong; for this option it ought to be:
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow
(i.e. without the "=local"). As noted by Manu it sounds like the
implementation needs to be smarter about generating URLs.
Thanks again for the report.
Is this working/broken for other people?
Thanks
Dave
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] (PR 87488)
2019-10-12 14:51 ` David Malcolm
@ 2019-10-12 15:43 ` Bernd Edlinger
2019-10-12 18:10 ` Segher Boessenkool
1 sibling, 0 replies; 11+ messages in thread
From: Bernd Edlinger @ 2019-10-12 15:43 UTC (permalink / raw)
To: David Malcolm, gcc-patches
On 10/12/19 4:21 PM, David Malcolm wrote:
> On Sat, 2019-10-12 at 07:00 +0000, Bernd Edlinger wrote:
>> Hi David,
>>
>> I just wanted to say that this does not work right on ubuntu 14.04 at
>> least:
>>
>> g++ -Wshadow=local -Wno-shadow=compatible-local -S Wshadow-1.c
>> Wshadow-1.c: In function 'void foo(int*, int*)':
>> Wshadow-1.c:8:9: warning: declaration of 'int d' shadows a parameter
>> [^[]8;;
>> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow=local-Wshadow=local^[]8
>> ;;]
>> 8 | int d = 0; /* { dg-warning "Wshadow=local" }
>> */
>> | ^
>> Wshadow-1.c:4:23: note: shadowed declaration is here
>> 4 | void foo(int *c, int *d) /* { dg-bogus "Wshadow" } */
>> | ~~~~~^
>> Wshadow-1.c: In function 'void bar(int*, int*)':
>> Wshadow-1.c:15:15: warning: declaration of 'c' shadows a global
>> declaration [^[]8;;
>> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow-Wshadow^[]8
>> ;;]
>> 15 | void bar(int *c, int *d) /* { dg-warning "Wshadow" } */
>> | ~~~~~^
>> Wshadow-1.c:3:5: note: shadowed declaration is here
>> 3 | int c;
>> | ^
>> Wshadow-1.c:19:9: warning: declaration of 'int d' shadows a parameter
>> [^[]8;;
>> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow-Wshadow^[]8
>> ;;]
>> 19 | int d = 0; /* { dg-warning "Wshadow" } */
>> | ^
>> Wshadow-1.c:15:23: note: shadowed declaration is here
>> 15 | void bar(int *c, int *d) /* { dg-warning "Wshadow" } */
>> | ~~~~~^
>> Wshadow-1.c:22:10: warning: declaration of 'e' shadows a previous
>> local [^[]8;;
>> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow-Wshadow^[]8
>> ;;]
>> 22 | int *e = 0; /* { dg-warning "Wshadow" } */
>> | ^
>> Wshadow-1.c:17:8: note: shadowed declaration is here
>> 17 | int *e = d;
>> | ^
>>
>> Coloring works, even the hyperlinks seem to work, when clicked at,
>> although they jump just
>> to the top of the page, and one has to scroll down to the warning
>> manually.
>> But the warning name is very hard to spot in all that glibberish long
>> text :-(
>
> Thanks for the report.
>
> Bother.
>
> There are (at least) two issues here.
>
> (a) It sounds like your terminal doesn't handle the escape sequence my
> patch is emitting. The expected outcome is that the terminal embeds
> this URL:
>
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow=local
>
> within:
>
> [-Wshadow=local]
>
> i.e. equivalent to:
>
> [<a href="
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow=local"</a>
> ;;]
>
> It seems like instead it's displaying this:
>
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow=local-Wshadow=local%1B]8
>
> When you say "even the hyperlinks seem to work" it seems like that URL
> fragment is being handled by the terminal's regular "look for URLs in
> the text" logic.
>
> Does your terminal have a "Copy URL" option (perhaps on a right-click
> menu)? If so, what URL is copied?
>
Yes, the URL is
"https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow-Wshadow"
> Which terminal (and version) are you seeing this with? (if gnome
> terminal, which vte version?)
>
I think the process that hosts the "Terminal" is
/usr/bin/xfce4-terminal
The a Help/About menu says:
xfce4-terminal 0.6.3
> What happens if you try this at the terminal:
>
> printf '\e]8;;http://example.com\e\\This is a link\e]8;;\e\\\n'
>
I see this:
^[]8;;http://example.comThis is a link^[]8;;
the ] is actually a little square with little 0101 in it.
again the URL is copied. This time it is "http://example.comThis"
> (hopefully that doesn't get mangled by my mailer; this is the example
> from:
> https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda
> )
>
Both show the same info.
> I don't yet know if there's a way to query the terminal to see if the
> escape is supported. My hope was that they would be silently
> discarded.
>
I wonder why this is not done with a ncurses function?
In case it helps this is the environment I have:
$ export
declare -x CLUTTER_IM_MODULE="xim"
declare -x COLORTERM="xfce4-terminal"
declare -x DBUS_SESSION_BUS_ADDRESS="unix:abstract=/tmp/dbus-Wh3VZkTGvf"
declare -x DEFAULTS_PATH="/usr/share/gconf/ubuntustudio.default.path"
declare -x DESKTOP_SESSION="ubuntustudio"
declare -x DISPLAY=":0.0"
declare -x GDMSESSION="ubuntustudio"
declare -x GDM_LANG="de_DE"
declare -x GLADE_CATALOG_PATH=":"
declare -x GLADE_MODULE_PATH=":"
declare -x GLADE_PIXMAP_PATH=":"
declare -x GNOME_KEYRING_CONTROL="/run/user/1000/keyring-V870cQ"
declare -x GNOME_KEYRING_PID="1770"
declare -x GPG_AGENT_INFO="/run/user/1000/keyring-V870cQ/gpg:0:1"
declare -x GTK_IM_MODULE="xim"
declare -x HOME="/home/ed"
declare -x IM_CONFIG_PHASE="1"
declare -x INSTANCE=""
declare -x JOB="dbus"
declare -x LANG="de_DE.UTF-8"
declare -x LANGUAGE="de_DE"
declare -x LESSCLOSE="/usr/bin/lesspipe %s %s"
declare -x LESSOPEN="| /usr/bin/lesspipe %s"
declare -x LOGNAME="ed"
declare -x LS_COLORS="rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arj=01;31:*.taz=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lz=01;31:*.xz=01;31:*.bz2=01;31:*.bz=01;31:*.tbz=01;31:*.tbz2=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.war=01;31:*.ear=01;31:*.sar=01;31:*.rar=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.webm=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.axv=01;35:*.anx=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=00;36:*.au=00;36:*.flac=00;36:*.mid=00;36:*.midi=00;36:*.mka=00;36:*.mp3=00;36:*.mpc=00;36:*.ogg=00;36:*.ra=00;36:*.wav=00;36:*.axa=00;36:*.oga=00;36:*.spx=00;36:*.xspf=00;36:"
declare -x MANDATORY_PATH="/usr/share/gconf/ubuntustudio.mandatory.path"
declare -x OLDPWD
declare -x PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games"
declare -x PWD="/home/ed"
declare -x QT4_IM_MODULE="xim"
declare -x SELINUX_INIT="YES"
declare -x SESSION="ubuntustudio"
declare -x SESSIONTYPE=""
declare -x SESSION_MANAGER="local/w-ed:@/tmp/.ICE-unix/1891,unix/w-ed:/tmp/.ICE-unix/1891"
declare -x SHELL="/bin/bash"
declare -x SHLVL="1"
declare -x SSH_AGENT_PID="1911"
declare -x SSH_AUTH_SOCK="/tmp/ssh-M2NF9s1ZmbAq/agent.1910"
declare -x TERM="xterm"
declare -x TEXTDOMAIN="im-config"
declare -x TEXTDOMAINDIR="/usr/share/locale/"
declare -x UPSTART_EVENTS="started xsession"
declare -x UPSTART_INSTANCE=""
declare -x UPSTART_JOB="startxfce4"
declare -x UPSTART_SESSION="unix:abstract=/com/ubuntu/upstart-session/1000/1773"
declare -x USER="ed"
declare -x WINDOWID="56623795"
declare -x XAUTHORITY="/home/ed/.Xauthority"
declare -x XDG_CONFIG_DIRS="/etc/xdg/xdg-ubuntustudio:/usr/share/upstart/xdg:/etc/xdg:/etc/xdg"
declare -x XDG_CURRENT_DESKTOP="XFCE"
declare -x XDG_DATA_DIRS="/usr/share/ubuntustudio:/usr/share/xfce4:/usr/local/share/:/usr/share/:/usr/share"
declare -x XDG_GREETER_DATA_DIR="/var/lib/lightdm-data/ed"
declare -x XDG_MENU_PREFIX="xfce-"
declare -x XDG_RUNTIME_DIR="/run/user/1000"
declare -x XDG_SEAT="seat0"
declare -x XDG_SEAT_PATH="/org/freedesktop/DisplayManager/Seat0"
declare -x XDG_SESSION_ID="c2"
declare -x XDG_SESSION_PATH="/org/freedesktop/DisplayManager/Session0"
declare -x XDG_VTNR="7"
declare -x XMODIFIERS="@im=none"
>
>> Do you see a way to find out if the escape sequences are supported,
>> or how to disable this function per configure option?
>
> Do you mean a GCC configure-time option to change the default? We have
> that for colors; we could add one for URLs. (Though I'd prefer to know
> more about the scope of the problem first)
>
Yes, me too.
>
> The other problem:
>
> (b) The URL itself is wrong; for this option it ought to be:
>
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow
>
> (i.e. without the "=local"). As noted by Manu it sounds like the
> implementation needs to be smarter about generating URLs.
>
I think the "=" in =local may be a problem, the html page
would need
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow_003dlocal
However when it works the browser jumps just 2 lines too far down,
so you don't see the headline anymore, and you don't know what warning is described
here.
> Thanks again for the report.
>
>
> Is this working/broken for other people?
>
> Thanks
> Dave
>
Thanks
Bernd.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] (PR 87488)
2019-10-12 14:51 ` David Malcolm
2019-10-12 15:43 ` Bernd Edlinger
@ 2019-10-12 18:10 ` Segher Boessenkool
1 sibling, 0 replies; 11+ messages in thread
From: Segher Boessenkool @ 2019-10-12 18:10 UTC (permalink / raw)
To: David Malcolm; +Cc: Bernd Edlinger, gcc-patches
On Sat, Oct 12, 2019 at 10:21:56AM -0400, David Malcolm wrote:
> It seems like instead it's displaying this:
>
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wshadow=local-Wshadow=local%1B]8
That's what it does for me (well, with weird escapes at both ends). And
if run inside a tmux it just shows what you wanted to show, but there is
no way to click it: the URL is not displayed at all.
> What happens if you try this at the terminal:
>
> printf '\e]8;;http://example.com\e\\This is a link\e]8;;\e\\\n'
In gnome term it shows a bunch of funny escapes. In xterm it just
displays "This is a link", without any way to click it, like in the
tmux scenario.
The links are wrong btw, they should be version-specific?
> I don't yet know if there's a way to query the terminal to see if the
> escape is supported. My hope was that they would be silently
> discarded.
Just display the URL if you want this? That works, and has always worked,
and many terminals have convenient ways to use that (have had that since
forever, too).
Can we have this *off* by default, please? The warnings are getting
way too verbose. Often one warning does not fit on one screen already!
> > Do you see a way to find out if the escape sequences are supported,
> > or how to disable this function per configure option?
>
> Do you mean a GCC configure-time option to change the default? We have
> that for colors;
Do we? I've had GCC_COLORS= since forever; is this finally not needed
anymore? Neat :-) Now just ASAN_OPTIONS=color=never TSAN_OPTIONS=color=never
by default and there is a bit of sanity again.
> Is this working/broken for other people?
I've only tried your example so far, and that does not work :-(
Segher
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-12-07 1:31 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-10 17:06 [PATCH 1/2] pretty-print: support URL escape sequences (PR 87488) David Malcolm
2019-10-10 17:19 ` [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] " David Malcolm
2019-10-10 17:49 ` Manuel López-Ibáñez
2019-10-11 3:54 ` Eric Gallager
2019-10-14 16:35 ` Michael Matz
2019-12-06 23:41 ` [PATCH 1/2] pretty-print: support URL escape sequences " Jakub Jelinek
2019-12-07 1:31 ` David Malcolm
2019-10-12 7:28 [PATCH 2/2] Documentation hyperlinks for [-Wname-of-option] " Bernd Edlinger
2019-10-12 14:51 ` David Malcolm
2019-10-12 15:43 ` Bernd Edlinger
2019-10-12 18:10 ` Segher Boessenkool
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).