Bug 87488 - hyperlink filenames in diagnostics
Summary: hyperlink filenames in diagnostics
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 9.0
: P3 enhancement
Target Milestone: ---
Assignee: David Malcolm
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2018-10-02 16:32 UTC by Manuel López-Ibáñez
Modified: 2019-10-10 21:11 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-10-09 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Manuel López-Ibáñez 2018-10-02 16:32:22 UTC
Modern terminals are starting to implement the interpretation of hyperlinks encoded in escape sequences (https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda). The idea is to produce a escape sequence similar to the one generated for colors that encodes a hyperlink that, in the simplest case, describes the path to a file.

Support for hyperlinks has been added to ls:

https://github.com/coreutils/coreutils/commit/799bac0d06cfabe9491498727308df8d1aca6d98

and to systemd:

https://github.com/systemd/systemd/commit/23b27b39d2a3a002ad827a2e8a9872a51495d797

It would be interesting if GCC hyperlinked filenames in GCC diagnostics such that given:

myfile.c:1:2: error: some error

it is possible to click in myfile.c and automatically open "file://path/to/myfile.c"

I suggest  -fdiagnostics-hyperlink=[always|never|auto] with a default of never for now until support becomes more widespread.
Comment 1 Manuel López-Ibáñez 2018-10-02 16:32:58 UTC
David may be interested in this.
Comment 2 Manuel López-Ibáñez 2018-10-02 16:36:15 UTC
Actually, according to https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda#detecting-availability-of-the-feature

"almost all terminal emulators ignore the OSC sequences they don't know about, so emitting OSC 8 will result in the anchor text not becoming a hyperlink, and the target URI getting lost."

That is, the default could be "auto" since terminals that do not recognize the sequence will simply discard it.
Comment 3 Jonathan Wakely 2018-10-02 16:52:09 UTC
https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda#file-uris-and-the-hostname says there should be a hostname part, so not just file:///path but file://hostname/path

If I compile on a remote machine I don't want URLs that include the remote hostname (which might not even be reachable from my local machine via that name) or even necessarily the path relative to the remote machine. I might want to be able to click on a filename in the output of the remote compilation but have it open the corresponding file locally. So it might be useful to have one or more -fdiagnostic-xxx options that allow me to set the hostname explicitly, and/or to map from one path to another. So when I click on myfile.c on the remote host I can get a URL to a local copy of that file (which I would edit and push to the remote machine before recompiling).
Comment 4 David Malcolm 2018-10-02 16:53:18 UTC
Interesting.  Is there a way to encode the line number in the URL?  (and the column number?)  Or does it just give the file?
Comment 5 Jonathan Wakely 2018-10-02 17:32:55 UTC
Vim already Does The Right Thing for a filename like file.c:20:23
Comment 6 Manuel López-Ibáñez 2018-10-02 19:51:20 UTC
I think the utility of this is similar to colors, that is when the output is a terminal, not when it is emacs or vim or any other non-tty.

In remote hosts, I think we should do whatever GNU coreutils (so far, ls) is doing, probably no hostname at all. Most GNU/Linux distributions will use xdg-open to open the file:// url and that seems to work without a hostname.
Comment 7 David Malcolm 2019-01-08 15:15:34 UTC
I'm unconvinced that doing it for filenames is a good idea (based on the objections in comment #3), but I think that there could be other good uses tagging URLs into the output.

For example, a static analysis plugin that emits e.g.:

  error: NULL pointer 'p' passed as argument 1 to 'foo' [CWE-690]

might want to tag the "CWE-690" in the message/metadata with the URL https://cwe.mitre.org/data/definitions/690.html

Similarly, any time we mention a GCC option, perhaps we could provide a URL to the documentation of that option on the GCC website.  (Sadly, most of our options don't have good URLs)
Comment 8 David Malcolm 2019-10-09 20:24:13 UTC
I'm working on an implementation of this, providing URLs for option documentation for the [-Woption-name] suffixes to diagnostics.
Comment 9 David Malcolm 2019-10-10 16:58:01 UTC
Author: dmalcolm
Date: Thu Oct 10 16:57:30 2019
New Revision: 276841

URL: https://gcc.gnu.org/viewcvs?rev=276841&root=gcc&view=rev
Log:
pretty-print: support URL escape sequences (PR 87488)

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.

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.


Added:
    trunk/gcc/diagnostic-url.h
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/common.opt
    trunk/gcc/diagnostic-color.c
    trunk/gcc/diagnostic.c
    trunk/gcc/diagnostic.h
    trunk/gcc/doc/invoke.texi
    trunk/gcc/gcc.c
    trunk/gcc/opts-global.c
    trunk/gcc/opts.c
    trunk/gcc/pretty-print.c
    trunk/gcc/pretty-print.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/lib/prune.exp
Comment 10 David Malcolm 2019-10-10 17:04:17 UTC
Author: dmalcolm
Date: Thu Oct 10 17:03:46 2019
New Revision: 276843

URL: https://gcc.gnu.org/viewcvs?rev=276843&root=gcc&view=rev
Log:
Documentation hyperlinks for [-Wname-of-option] (PR 87488)

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.

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.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/Makefile.in
    trunk/gcc/configure
    trunk/gcc/configure.ac
    trunk/gcc/diagnostic-format-json.cc
    trunk/gcc/diagnostic.c
    trunk/gcc/diagnostic.h
    trunk/gcc/doc/invoke.texi
    trunk/gcc/opts-diagnostic.h
    trunk/gcc/opts.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/diagnostic-format-json-2.c
    trunk/gcc/testsuite/c-c++-common/diagnostic-format-json-3.c
    trunk/gcc/testsuite/c-c++-common/diagnostic-format-json-4.c
    trunk/gcc/testsuite/gfortran.dg/diagnostic-format-json-2.F90
    trunk/gcc/testsuite/gfortran.dg/diagnostic-format-json-3.F90
    trunk/gcc/testsuite/jit.dg/test-error-array-bounds.c
    trunk/gcc/toplev.c
Comment 11 David Malcolm 2019-10-10 17:12:41 UTC
r276841 implements URL support within pretty_printer.

r276843 uses this to implement hyperlinks to our documentation for the [-Wname-of-option] text in our diagnostics.

I'm still not convinced that we should do this for filenames (based on the objections in comment #3), so I'm going to mark this bug as resolved.
Comment 12 Egmont Koblinger 2019-10-10 21:11:49 UTC
(In reply to Jonathan Wakely from comment #3) 

> [the spec] says there should be a hostname part, so not just
> file:///path but file://hostname/path

Yes. The exact reason for this design is that it can be detected when the filename belongs to a remote host and not mistakenly open its local counterpart.

E.g. gnome-terminal just tells you it doesn't know how to open that file. iTerm2, as far as I know, offers you to fetch it via scp (which only works if the host is accessible under its name in a single hop).

> If I compile on a remote machine I don't want URLs that include the remote
> hostname (which might not even be reachable from my local machine via that
> name) or even necessarily the path relative to the remote machine.

Do you have any particular concern other than "I don't want..."?

You access a remote host from your local one (possibly via multiple hops) and you're free to issue commands like "hostname" remotely. It's most likely printed in your prompt. I don't see how the remote host automatically telling it when you run gcc would be any sort of data leakage or privacy concern -- if that's what you are worried of.

> I might
> want to be able to click on a filename in the output of the remote
> compilation but have it open the corresponding file locally.

Fair expectation from the user's point of view. iTerm2's scp feature does this in simple setups.

> So it might be
> useful to have one or more -fdiagnostic-xxx options that allow me to set the
> hostname explicitly,

Or gcc could simply prefer $HOSTNAME if defined.

> and/or to map from one path to another. So when I click
> on myfile.c on the remote host I can get a URL to a local copy of that file
> (which I would edit and push to the remote machine before recompiling).

It's unclear to me how that local copy would be created -- or do you only consider setups where the entire source tree has a local copy available? And what about files that are outside of your project, e.g. system headers? If gcc wants to point to /usr/include/stdio.h of the remote host, how would it map to a local one?

That being said, I'm not against this approach at all. Maybe you could define how your remote files map to local ones, and ones that don't have a 1:1 local counterpart (with guaranteed same content) would remain unlinkified by gcc. Or maybe you could rewrite all filenames so that they use sshfs that lets you access the entire remote filesystem. Seems perhaps a bit cumbersome to implement, and also potentially cumbersome for users to set up, and your use case might IMHO be a quite special one, so I'm not sure if it's all worth it, but it might be.

I assume however that the majority of developers compile locally. (Do you gcc devs have any stats on this?) For them linkifying the files would just work, and might be a great convenience feature. IMO a convenience feature not working for all of gcc's users, only for the majority of them, should not be a reason per se for rejecting it.

And let me emphasize again: Those who compile remotely wouldn't get a sneaky faulty behavior where a different local counterpart is opened and they waste precious time pondering why the hell that's relevant at all (assuming that the remote hostname differs from the local one, a pretty reasonable assumption I hope). The worst they would get is a hyperlink that fails to open. GCC could even employ other simple low hanging fruit tricks, e.g. to enable/disable linkifying filenames by default based on the absence/presence of some $SSH_* env var or so, or the build system could do such magic.


(In reply to David Malcolm from comment #4)

> Is there a way to encode the line number in the URL?

Unfortunately no, not yet. See the comments under the gist (especially the ones from Aug 2019) for the reasons and where we are stuck at the moment.