feat: Make browser links out of HTML file paths #12889

epage · 2023-10-28T02:35:17Z

This provides an alternative to --open, where supported.

Note: because we are relying on supports-hyperlinks, we are getting FORCE_HYPERLINK for "free". Unsure whether it and the current policy (it gets overridden by term.hyperlinks) is something we want. FORCE_HYPERLINK mirrors the npm package's behavior of the same name (see zkat/supports-hyperlinks#2) though though I also found reading of it in ohmyzsh, a go terminal library and many more places. Similarly, #12751 added indirect, undocumented support for community environment variables.

Fixes #12888

rustbot · 2023-10-28T02:35:23Z

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

epage · 2023-10-28T02:36:19Z

src/cargo/util/hostname.rs

@@ -0,0 +1,85 @@
+use std::{ffi::OsString, io};


Copied from ripgrep because the hostname and gethostname packages had what looked like less robust implementations.

Should we note this in code comment, so that people can update this copy when needed?

src/cargo/ops/cargo_doc.rs

src/cargo/core/shell.rs

weihanglo

Works for me on Kitty with Zellij.

epage · 2023-10-30T13:48:08Z

shrug no idea why wezterm isn't working for me, even when I do CARGO_TERM_HYPERLINKS=true and I instrument the code to show it is creating the link. I even dropped the hostname in case it calculated the hostname differently than us. I'm also on the latest wezterm release and the nightly changelogs dont mention this being new but its generally advertised as supported.

epage · 2023-10-30T14:01:32Z

This adds a new, insta-stable .cargo/config.toml field term.hyperlinks = <bool> to allow overriding our auto-detection of terminal hyperlink support. Technically, any terminal that supports ANSI escape codes should gracefully handle this if its unsupported but in practice some terminals are non-conforming.

This is used to turn file paths into links so people can open --timings reports or rustdoc directly without using a --open flag. This doesn't explore further uses like (1) explicit HTTP links (terminals tend to support these implicitly) (2) finding other output for us to easily link, like linking to crates.io pages for cargo search results.

@rfcbot fcp merge

rfcbot · 2023-10-30T14:01:34Z

Team member @epage has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

ehuss · 2023-10-30T16:15:51Z

I'm a little concerned about this making the interaction worse on some terminals. For example, on mine (iTerm2), it breaks it so that instead of opening the file, it opens a dialog box asking for a username to SCP the file. How important is it to include the hostname? If I was connected via ssh, how would having the hostname in the file path work? Is it possible to not include the hostname if not remotely connected?

unsupported but in practice some terminals are non-conforming

Do we have a sense of which terminals those are, or how common this is?

epage · 2023-10-30T16:39:10Z

I'm a little concerned about this making the interaction worse on some terminals. For example, on mine (iTerm2), it breaks it so that instead of opening the file, it opens a dialog box asking for a username to SCP the file. How important is it to include the hostname? If I was connected via ssh, how would having the hostname in the file path work? Is it possible to not include the hostname if not remotely connected?

By "it breaks", do you mean you could before open file paths and now it has the pop up?

From the reference gist

As such, we don't allow this sloppiness. Utilities that print hyperlinks are requested to fill out the hostname, and terminal emulators are requested to match it against the local hostname and refuse to open the file if the hostname doesn't match (or offer other possibilities, e.g. to download with scp as iTerm2 does).

That makes it sound like iTerm2 should be supported. As for how it should handle the hostname, it says

It's unreasonable for simple utilities to go into the business of hostname resolution. As such, we urge utilities to place the value from gethostname() there (shell scripts might go for $HOSTNAME).

Terminal emulators should match the given value against the local hostname. They might accept multiple values, e.g. both short and fully qualified names, but they are free to go with just the value from gethostname(). They also must accept the string localhost or the empty string as local ones. If a different hostname is present, they must not open the local counterpart with the same filename.

unsupported but in practice some terminals are non-conforming

Do we have a sense of which terminals those are, or how common this is?

Again, from the reference gist

At this moment, terminals known to be buggy (OSC 8 resulting in display corruption) are VTE versions up to 0.46.2 and 0.48.1, Windows Terminal up to 0.9, Emacs's built-in terminal, and screen with 700+ character long URLs.

In hopes to avoid problems with those is why I pulled in supports-hyperlinks as it handles detection for us.

In both cases, in case there are problems, is why I added term.hyperlinks. Someone can open an issue against us and then set that until the issue is fixed.

epage · 2023-10-30T16:43:00Z

In the ripgrep thread, I found this sub-thread devoted to iTerm2. Not seeing anyone mention the SCP problem

BurntSushi/ripgrep#2611 (comment)

epage · 2023-10-30T16:55:00Z

If this can't be resolved in a reasonable time, another potential route is to start with term.hyperlinks=false, removing supports-hyperlinks. This would make it available as a "preview" (stable but off by default). As we work through things, we can switch back to on-by-default.

ehuss · 2023-10-30T17:33:10Z

By "it breaks", do you mean you could before open file paths and now it has the pop up?

Correct.

Just a little more testing, Windows Terminal seems to work, which is nice since it doesn't seem to have native auto detection of paths (though it does auto-detect URLs). ConEmu seems to want to launch FAR, even though I don't have it installed. But it also tries to do that on bare file paths, too, so there is no change there.

Not working with Emacs seems a little worrying, though I think it depends on what it does wrong. I don't have emacs installed, so I can't really test it.

Unfortunately without a hostname, iTerm2 just opens Finder.

I don't know how widespread the issue might be. If out of the majority of terminals, iTerm2 is one of the few that has poor behavior, would it be possible to just disable it for that terminal (since it already works without this PR)?

rfcbot · 2023-11-07T15:43:07Z

🔔 This is now entering its final comment period, as per the review above. 🔔

src/cargo/core/shell.rs

weihanglo · 2023-11-08T02:49:51Z

src/cargo/util/hostname.rs

@@ -0,0 +1,85 @@
+use std::{ffi::OsString, io};


Should we note this in code comment, so that people can update this copy when needed?

src/cargo/core/shell.rs

weihanglo · 2023-11-08T15:42:04Z

I don't think this require to run 10 days for FCP, as it is only a cosmetic change for console output, and affect only --timings and cargo doc. If anyone doesn't agree, please call out. Otherwise I am going to merge this shortly.

@bors r+

bors · 2023-11-08T15:42:07Z

📌 Commit 2130a0f has been approved by weihanglo

It is now in the queue for this repository.

bors · 2023-11-08T15:47:37Z

⌛ Testing commit 2130a0f with merge b065008...

bors · 2023-11-08T16:34:49Z

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing b065008 to master...

bors · 2023-11-08T16:34:49Z

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing b065008 to master...

bors · 2023-11-08T16:35:50Z

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

Update cargo 12 commits in 7046d992f9f32ba209a8079f662ebccf9da8de25..6790a5127895debec95c24aefaeb18e059270df3 2023-11-08 03:24:57 +0000 to 2023-11-10 17:09:35 +0000 - refactor(source): Prepare for new PackageIDSpec syntax (rust-lang/cargo#12938) - credential: include license files in all published crates (rust-lang/cargo#12953) - fix: preserve jobserver file descriptors on rustc invocation in `fix_exec_rustc` (rust-lang/cargo#12951) - refactor(resolver): Consolidate logic in `VersionPreferences` (rust-lang/cargo#12930) - refactor(toml): Simplify code to make schema split easier (rust-lang/cargo#12948) - Filter `cargo-credential-*` dependencies by OS (rust-lang/cargo#12949) - refactor(util): Pull out `mod util_semver` (rust-lang/cargo#12940) - Fix the invalidate feature name message (rust-lang/cargo#12939) - refactor(util): Prepare for splitting out semver logic (rust-lang/cargo#12926) - feat: Make browser links out of HTML file paths (rust-lang/cargo#12889) - Do not allow empty feature name (rust-lang/cargo#12928) - fix(timings): unnecessary backslash when error happens (rust-lang/cargo#12934) r? ghost

Pkgsrc changes: * Adjust patches and cargo checksums to new versions. * For an external LLVM, set dependency of llvm >= 16, in accordance with the upstream changes. * Mark that on NetBSD we now need >= 9.0, so 8.x is no longer supported. * On NetBSD/sparc64 10.x, we now need GCC 12 to build the embedded LLVM, which is version 17; apparently GCC 10.4 or 10.5 mis-compiles it, resulting in an illegal instruction fault during the build. Ref. rust-lang/rust#117231 Upstream changes: Version 1.75.0 (2023-12-28) ========================== - [Stabilize `async fn` and return-position `impl Trait` in traits.] (rust-lang/rust#115822) - [Allow function pointer signatures containing `&mut T` in `const` contexts.] (rust-lang/rust#116015) - [Match `usize`/`isize` exhaustively with half-open ranges.] (rust-lang/rust#116692) - [Guarantee that `char` has the same size and alignment as `u32`.] (rust-lang/rust#116894) - [Document that the null pointer has the 0 address.] (rust-lang/rust#116988) - [Allow partially moved values in `match`.] (rust-lang/rust#103208) - [Add notes about non-compliant FP behavior on 32bit x86 targets.] (rust-lang/rust#113053) - [Stabilize ratified RISC-V target features.] (rust-lang/rust#116485) Compiler -------- - [Rework negative coherence to properly consider impls that only partly overlap.] (rust-lang/rust#112875) - [Bump `COINDUCTIVE_OVERLAP_IN_COHERENCE` to deny, and warn in dependencies.] (rust-lang/rust#116493) - [Consider alias bounds when computing liveness in NLL.] (rust-lang/rust#116733) - [Add the V (vector) extension to the `riscv64-linux-android` target spec.] (rust-lang/rust#116618) - [Automatically enable cross-crate inlining for small functions] (rust-lang/rust#116505) - Add several new tier 3 targets: - [`csky-unknown-linux-gnuabiv2hf`] (rust-lang/rust#117049) - [`i586-unknown-netbsd`] (rust-lang/rust#117170) - [`mipsel-unknown-netbsd`] (rust-lang/rust#117356) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Override `Waker::clone_from` to avoid cloning `Waker`s unnecessarily.] (rust-lang/rust#96979) - [Implement `BufRead` for `VecDeque<u8>`.] (rust-lang/rust#110604) - [Implement `FusedIterator` for `DecodeUtf16` when the inner iterator does.] (rust-lang/rust#110729) - [Implement `Not, Bit{And,Or}{,Assign}` for IP addresses.] (rust-lang/rust#113747) - [Implement `Default` for `ExitCode`.] (rust-lang/rust#114589) - [Guarantee representation of None in NPO] (rust-lang/rust#115333) - [Document when atomic loads are guaranteed read-only.] (rust-lang/rust#115577) - [Broaden the consequences of recursive TLS initialization.] (rust-lang/rust#116172) - [Windows: Support sub-millisecond sleep.] (rust-lang/rust#116461) - [Fix generic bound of `str::SplitInclusive`'s `DoubleEndedIterator` impl] (rust-lang/rust#100806) - [Fix exit status / wait status on non-Unix `cfg(unix)` platforms.] (rust-lang/rust#115108) Stabilized APIs --------------- - [`Atomic*::from_ptr`] (https://doc.rust-lang.org/stable/core/sync/atomic/struct.AtomicUsize.html#method.from_ptr) - [`FileTimes`] (https://doc.rust-lang.org/stable/std/fs/struct.FileTimes.html) - [`FileTimesExt`] (https://doc.rust-lang.org/stable/std/os/windows/fs/trait.FileTimesExt.html) - [`File::set_modified`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.set_modified) - [`File::set_times`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.set_times) - [`IpAddr::to_canonical`] (https://doc.rust-lang.org/stable/core/net/enum.IpAddr.html#method.to_canonical) - [`Ipv6Addr::to_canonical`] (https://doc.rust-lang.org/stable/core/net/struct.Ipv6Addr.html#method.to_canonical) - [`Option::as_slice`] (https://doc.rust-lang.org/stable/core/option/enum.Option.html#method.as_slice) - [`Option::as_mut_slice`] (https://doc.rust-lang.org/stable/core/option/enum.Option.html#method.as_mut_slice) - [`pointer::byte_add`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_add) - [`pointer::byte_offset`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_offset) - [`pointer::byte_offset_from`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_offset_from) - [`pointer::byte_sub`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_sub) - [`pointer::wrapping_byte_add`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_add) - [`pointer::wrapping_byte_offset`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_offset) - [`pointer::wrapping_byte_sub`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_sub) These APIs are now stable in const contexts: - [`Ipv6Addr::to_ipv4_mapped`] (https://doc.rust-lang.org/stable/core/net/struct.Ipv6Addr.html#method.to_ipv4_mapped) - [`MaybeUninit::assume_init_read`] (https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#method.assume_init_read) - [`MaybeUninit::zeroed`] (https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#method.zeroed) - [`mem::discriminant`] (https://doc.rust-lang.org/stable/core/mem/fn.discriminant.html) - [`mem::zeroed`] (https://doc.rust-lang.org/stable/core/mem/fn.zeroed.html) Cargo ----- - [Add new packages to `[workspace.members]` automatically.] (rust-lang/cargo#12779) - [Allow version-less `Cargo.toml` manifests.] (rust-lang/cargo#12786) - [Make browser links out of HTML file paths.] (rust-lang/cargo#12889) Rustdoc ------- - [Accept less invalid Rust in rustdoc.] (rust-lang/rust#117450) - [Document lack of object safety on affected traits.] (rust-lang/rust#113241) - [Hide `#[repr(transparent)]` if it isn't part of the public ABI.] (rust-lang/rust#115439) - [Show enum discriminant if it is a C-like variant.] (rust-lang/rust#116142) Compatibility Notes ------------------- - [FreeBSD targets now require at least version 12.] (rust-lang/rust#114521) - [Formally demote tier 2 MIPS targets to tier 3.] (rust-lang/rust#115238) - [Make misalignment a hard error in `const` contexts.] (rust-lang/rust#115524) - [Fix detecting references to packed unsized fields.] (rust-lang/rust#115583) - [Remove support for compiler plugins.] (rust-lang/rust#116412)

feat(tree): Control `--charset` via auto-detecting config value ### What does this PR try to resolve? This tries to generalize `cargo tree --charset` so any part of cargo's output can use it. For example, - `cargo search` could use this for fancier tables - `cargo add` could use this for fancier feature lists - #12235 could use this for fancy rendering like miette does (CC `@Muscraft` ) - Progress bars could use fancier characters <-- this is what I'm personally working towards This builds on the idea from #12889 that we can use fancier terminal features so long as we have good auto-detection and provide users an escape hatch until the auto-detection is improved or in case they disagree. As a side benefit - `cargo tree` will auto-detect when the prior default of `--charset utf8` is a good choice - Users can control `cargo tree --charset` via `.cargo/config.toml` ### How should we test and review this PR? This is gradually introduced through the individual commits. ### Additional information

rustbot assigned ehuss Oct 28, 2023

epage commented Oct 28, 2023

View reviewed changes

src/cargo/ops/cargo_doc.rs Show resolved Hide resolved

epage commented Oct 28, 2023

View reviewed changes

src/cargo/core/shell.rs Show resolved Hide resolved

epage force-pushed the hyperlink branch from 4bf6f3e to 1d885e1 Compare October 28, 2023 02:37

weihanglo reviewed Oct 28, 2023

View reviewed changes

epage force-pushed the hyperlink branch from 1d885e1 to 1c61d1c Compare October 30, 2023 13:44

rustbot added the A-timings Area: timings label Oct 30, 2023

epage added the T-cargo Team: Cargo label Oct 30, 2023

epage force-pushed the hyperlink branch from 1c61d1c to 525624e Compare October 30, 2023 13:52

epage marked this pull request as ready for review October 30, 2023 13:52

rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Oct 30, 2023

epage force-pushed the hyperlink branch 2 times, most recently from 34d82e5 to e50aaad Compare October 30, 2023 14:27

epage force-pushed the hyperlink branch from e50aaad to 04721ea Compare October 30, 2023 19:12

rfcbot added final-comment-period FCP — a period for last comments before action is taken and removed proposed-final-comment-period An FCP proposal has started, but not yet signed off. labels Nov 7, 2023

epage force-pushed the hyperlink branch from 1aeb6e5 to 39ef5e5 Compare November 7, 2023 17:29

weihanglo reviewed Nov 8, 2023

View reviewed changes

epage force-pushed the hyperlink branch from 39ef5e5 to 2130a0f Compare November 8, 2023 15:03

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 8, 2023

weihanglo approved these changes Nov 8, 2023

View reviewed changes

bors merged commit b065008 into rust-lang:master Nov 8, 2023
22 checks passed

epage deleted the hyperlink branch November 8, 2023 16:40

epage mentioned this pull request Nov 8, 2023

Use ANSI codes to report progress #11432

Open

bors mentioned this pull request Nov 8, 2023

Add option to error when warnings are emitted, or ignore warnings #12875

Open

weihanglo mentioned this pull request Nov 10, 2023

Update cargo rust-lang/rust#117797

Merged

rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Nov 17, 2023

ehuss added this to the 1.75.0 milestone Dec 6, 2023

epage mentioned this pull request Dec 11, 2023

Teach rustc to use OSC8 on nightly / Embedded links in terminal output rust-lang/compiler-team#587

Closed

3 tasks

epage mentioned this pull request Jan 22, 2024

feat(tree): Control --charset via auto-detecting config value #13337

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Make browser links out of HTML file paths #12889

feat: Make browser links out of HTML file paths #12889

epage commented Oct 28, 2023 •

edited

Loading

rustbot commented Oct 28, 2023

epage Oct 28, 2023

weihanglo Nov 8, 2023

epage Nov 8, 2023

weihanglo left a comment

epage commented Oct 30, 2023

epage commented Oct 30, 2023

rfcbot commented Oct 30, 2023 •

edited by arlosi

Loading

ehuss commented Oct 30, 2023

epage commented Oct 30, 2023

epage commented Oct 30, 2023

epage commented Oct 30, 2023

ehuss commented Oct 30, 2023

rfcbot commented Nov 7, 2023

weihanglo Nov 8, 2023

weihanglo commented Nov 8, 2023

bors commented Nov 8, 2023

bors commented Nov 8, 2023

bors commented Nov 8, 2023

bors commented Nov 8, 2023

bors commented Nov 8, 2023

feat: Make browser links out of HTML file paths #12889

feat: Make browser links out of HTML file paths #12889

Conversation

epage commented Oct 28, 2023 • edited Loading

rustbot commented Oct 28, 2023

epage Oct 28, 2023

Choose a reason for hiding this comment

weihanglo Nov 8, 2023

Choose a reason for hiding this comment

epage Nov 8, 2023

Choose a reason for hiding this comment

weihanglo left a comment

Choose a reason for hiding this comment

epage commented Oct 30, 2023

epage commented Oct 30, 2023

rfcbot commented Oct 30, 2023 • edited by arlosi Loading

ehuss commented Oct 30, 2023

epage commented Oct 30, 2023

epage commented Oct 30, 2023

epage commented Oct 30, 2023

ehuss commented Oct 30, 2023

rfcbot commented Nov 7, 2023

weihanglo Nov 8, 2023

Choose a reason for hiding this comment

weihanglo commented Nov 8, 2023

bors commented Nov 8, 2023

bors commented Nov 8, 2023

bors commented Nov 8, 2023

bors commented Nov 8, 2023

bors commented Nov 8, 2023

epage commented Oct 28, 2023 •

edited

Loading

rfcbot commented Oct 30, 2023 •

edited by arlosi

Loading