Make file names in error messages links to that file.

[Dragon-Hatcher]

#107838 Made error codes in error messages link to the webpage that explains the error code.

This change expands on that by making the file names in error messages link to the file. This makes it easy to open the file with the error by clicking the link to the file.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @WaffleLapkin (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[rustbot]

These commits modify the Cargo.lock file. Random changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
This was probably unintentional and should be reverted before this PR is merged.

If this was intentional then you can ignore this comment.

[Dragon-Hatcher]

After further inspection the terminal links standard is a little more involved. I've made some fixes.

https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda

We need to look into how the test is going to work. A correct file uri involves the hostname which isn't static in CI.

I should also add a test for file names with non ascii characters. Not sure of the best way to do that as of now.

[Dragon-Hatcher]

Can't really get CI to pass right now. We will need more discussion on the correct way to do that. Perhaps the hostname could be replaced with $HOST by the harness like the folder is?

[rust-log-analyzer]

The job x86_64-gnu-llvm-14 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

........................................................................................  3344/14753
......................................i.................................................  3432/14753
.....i..................................................................................  3520/14753
........................................................................................  3608/14753
........................F...........................................................iiii  3696/14753
........................................................................................  3872/14753
........................................................................................  3960/14753
........................................................................................  4048/14753
........................................................................................  4136/14753
---

---- [ui] tests/ui/diagnostic-flags/terminal_urls.rs stdout ----
diff of stderr:

1 error[ttps://doc.rust-lang.org/error_codes/E0308.htmlE0308ismatched types
-   --> ile://$DIR/terminal_urls.rs$DIR/terminal_urls.rs]8;;:3:9
+   --> ile://5f70d2ad0bb5$DIR/terminal_urls.rs$DIR/terminal_urls.rs]8;;:3:9
-    |     let () = 4;
+ LL |     let () = 4;
5    |         ^^   - this expression has type `{integer}`
6    |         |
---
To only update this specific test, also pass `--test-args diagnostic-flags/terminal_urls.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/diagnostic-flags/terminal_urls.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Zdeduplicate-diagnostics=no" "-Cstrip=debuginfo" "--remap-path-prefix=/checkout/tests/ui=fake-test-src-base" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/diagnostic-flags/terminal_urls" "-A" "unused" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/diagnostic-flags/terminal_urls/auxiliary" "-Zterminal-urls=yes"
stdout: none
--- stderr -------------------------------
error[ttps://doc.rust-lang.org/error_codes/E0308.htmlE0308ismatched types
  --> ile://5f70d2ad0bb5/checkout/tests/ui/diagnostic-flags/terminal_urls.rsfake-test-src-base/diagnostic-flags/terminal_urls.rs]8;;:3:9
   |
LL |     let () = 4; //~ ERROR
   |         ^^   - this expression has type `{integer}`
   |         expected integer, found `()`

error: aborting due to previous error

[WaffleLapkin]

r? @estebank

probably knows better how this works

[estebank]

I tried to get this working on the original PR and I encountered the same issues you did, including terminals that worked without a host, but didn't support a host being set, and others that didn't work without a host. Additionally, when using VSCode SSHd to a remote desktop, the file URLs still try to open a local file. It seems like file path support is even lower than that of file paths :-/

I don't know if we should try to accomplish this today, and maybe instead pester upstreams to gain more support?

[Dragon-Hatcher]

Another option is to just leave out the hostname. That removes the need for extra dependencies and complexity at the cost of not working correctly in some terminals/situations.

I think the cost of the link not working is fairly low (?) and it is a convenient feature. If it doesn't work correctly people will mostly understand why (if they are sshd to a remote machine). OTOH I'm not sure if this sort of kinda works attitude is in line with what Rust wants to do.

Personally I made this PR becuase this is something I want myself while coding Rust and I would rather have it be available than not exist at all.

We could also add another option or my options to the current option (e.g. yes-files or files or something).

[estebank]

It might make sense to make this a separate opt-in. In the terminals that I regularly use, they already recognize paths that can be resolved, turning them into links when pressing ctrl/command that open the file with whatever default application you have set up (and had the benefit of working with SSH + VScode). That's why I felt it wasn't necessary to spend too much time figuring that part out on the original PR :)

[Dragon-Hatcher]

Ok. What about making -Zterminal-urls accept no, err-code, files, or all (<- bikeshedable oc)

[estebank]

@Dragon-Hatcher that sounds reasonable to me. If -Zterminal-urls is passed with no value, it probably should default to err-code.

[bors]

☔ The latest upstream changes (presumably #109133) made this pull request unmergeable. Please resolve the merge conflicts.

[apiraino]

Switching to waiting on author for further design work (IIUC the context) . @Dragon-Hatcher Feel free to request a review with @rustbot ready, thanks!

@rustbot author

[JohnCSimon]

@Dragon-Hatcher

ping from triage - can you post your status on this PR? There hasn't been an update in a few months. Thanks!

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

[Dragon-Hatcher]

I'm still interested in the feature but I don't have time to work on this right now.

[JohnCSimon]

@Dragon-Hatcher

Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive