Add html_root_url to rustc_hir crate

[pierwill]

No description provided.

[rust-highfive]

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

[pierwill]

Should we add this attribute to all crates in compiler/?

[oli-obk]

cc @rust-lang/rustdoc

What does this attribute change?

[jyn514]

This shouldn't be necessary. The only time it's useful is when rustc_hirid is documented as a dependency and rustdoc is passed --no-deps, which isn't the case for the docs published on /nightly-rustc.

[jyn514]

See also rust-lang/api-guidelines#229.

[camelid]

@pierwill what's the reason you want to add this attribute?

[pierwill]

In my local setup, it seems this attribute makes it possible to use LSP commands that open the selected item in external documentation (docs.rs).

…

On Sat, Dec 11, 2021 at 3:56 PM Noah Lev ***@***.***> wrote: @pierwill <https://github.com/pierwill> what's the reason you want to add this attribute? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#91810 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEV3NIHOBO4CFUO6Z74XIH3UQPCJZANCNFSM5J3OVYOQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[camelid]

In my local setup, it seems this attribute makes it possible to use LSP commands that open the selected item in external documentation (docs.rs).

Hmm, that's weird. Why would adding a doc.rust-lang.org root URL help with docs.rs? That seems like a tooling bug IMO.

[pierwill]

In my local setup, it seems this attribute makes it possible to use LSP commands that open the selected item in external documentation (docs.rs).

Hmm, that's weird. Why would adding a doc.rust-lang.org root URL help with docs.rs? That seems like a tooling bug IMO.

Ah sorry. It does help with doc.rust-lang.org.

[camelid]

Hmm, IMO it still feels like overkill to add this attribute to every crate.

[Mark-Simulacrum]

Can you share more about your local setup? In general, it seems like fixing this with attributes in each(?) of the rustc_ crates in-tree is likely not the right approach. Maybe this can be fixed upstream?

It might also be worth asking what is different about these crates vs. other crates you're editing -- maybe they're just unilaterally prefixing with docs.rs without such an attribute? The docs.rs/std redirect to doc.rust-lang.org may be something we could match, there, though I'm a little uncertain about the concept in general (IIRC, there's no actual restrictions on std or rustc_ crates being published to crates.io).

[camelid]

(IIRC, there's no actual restrictions on std or rustc_ crates being published to crates.io).

I think I recall seeing a discussion recently where someone said that std is not allowed on crates.io and that there's a blocklist somewhere in the crates.io sources.

EDIT: Here it is, although it's from a 2017 migration, so maybe it's no longer accurate: https://github.com/rust-lang/crates.io/blob/7844bf4c543859f351fd7d82d5a971c49e80ccec/migrations/20170305095748_create_reserved_crate_names/up.sql#L6-L12

[Nemo157]

There was some previous discussion about having docs.rs do forwarding for the internal rustc crates in rust-lang/docs.rs#79. I wonder whether the LSP implementations parse/use the [doc.extern-map] section, though I don't know whether it's possible to specify a "local path" registry key in there to give an external location for the local crate's docs.

[pierwill]

I'm using Emacs and lsp-mode, which includes lsp-rust-analyzer-open-external-docs. For rustc crates, lsp-rust-analyzer-open-external-docs seems to only work when this attribute is present.

[jyn514]

Relevant code in RA: https://github.com/rust-analyzer/rust-analyzer/blob/4f04d8477a1aac357e137df47f4e1ff277325c89/crates/ide/src/doc_links.rs#L298 / https://github.com/rust-analyzer/rust-analyzer/blob/4f04d8477a1aac357e137df47f4e1ff277325c89/crates/ide/src/doc_links.rs#L415

I think this is an ok change, but it shouldn't live in the crate proper, which causes lots of churn any time it needs to be updated - can you put it in impl Step for Rustc in src/bootstrap/doc.rs instead? That adds the attribute for all crates at once; see how libstd does it.

Hmm actually I think that doesn't actually work because RA bypasses x.py ... @matklad do you have suggestions for how to make this work without having to add the attribute to each crate one at a time?

[jyn514]

Also, if you can mention this bug on the RFC and comment why the RFC helps, that would be extremely helpful :)

// BUG: For Option::Some
// Returns https://doc.rust-lang.org/nightly/core/prelude/v1/enum.Option.html#variant.Some
// Instead of https://doc.rust-lang.org/nightly/core/option/enum.Option.html
//
// This should cease to be a problem if RFC2988 (Stable Rustdoc URLs) is implemented
// rust-lang/rfcs#2988

[Mark-Simulacrum]

It basically seems like https://github.com/rust-analyzer/rust-analyzer/blob/4f04d8477a1aac357e137df47f4e1ff277325c89/crates/ide/src/doc_links.rs#L420 could (should?) be expanded to all rustc_ crates, or perhaps some kind of rust-analyzer configuration option added.

I'm OK with adding some kind of once-off mention of the nightly-rustc docs prefix, per-crate changes feel a little excessive to me. At the very least, if we go down that path, we should add a tidy lint requiring them...

In general it feels problematic to expect that this configuration is something that can be guessed by rust-analyzer -- e.g., if I'm developing serde or something locally, I might want rust-analyzer to open cargo doc local docs, not the public ones. So I'd guess that a more holistic answer is needed here eventually even if there's temporarily OK workarounds.

[matklad]

Yeah, I feel it's best to just add a configuration option to rust-analyzer to swap our default of doc.rs for something else.

I don't think it makes sense to expand the list at https://github.com/rust-analyzer/rust-analyzer/blob/4f04d8477a1aac357e137df47f4e1ff277325c89/crates/ide/src/doc_links.rs#L420 manually. What that list contains is the runtime crates a rust user can get into by clicking goto definition (/library). Things like rustc_hir are different, they are completely invisible to the user (/compiler).

What might have also make sense is some workspace-global Cargo configuration for html_root_url, but, even if that's a good idea, it's better and simpler to start with ra-specific config for the same thing.