-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rustdoc: link to stable/beta docs consistently in documentation #84942
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
I tested this locally by applying setting |
d060275
to
8e0f56c
Compare
This is not super complicated, but is a large enough change it's probably not suitable for a backport, especially since beta is being promoted today or tomorrow. |
// @has 'associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/collections/btree/map/struct.BTreeMap.html#method.into_iter"]' 'std::collections::BTreeMap::into_iter' | ||
// @has 'associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/string/struct.String.html#method.from"]' 'String::from' | ||
// @has 'associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/vec/struct.Vec.html#method.into_iter"]' 'Vec::into_iter' | ||
// @has 'associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/{{channel}}/alloc/collections/btree/map/struct.BTreeMap.html#method.into_iter"]' 'std::collections::BTreeMap::into_iter' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should probably also add a tidy lint for /nightly/
in src/test/rustdoc, to avoid future regressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we add a test builder with channel = "stable"
like @pietroalbini was suggesting on Zulip, this won't be necessary.
This comment has been minimized.
This comment has been minimized.
Note that you'll also need to update the Rustdoc UI test suite, as some channel-dependent links are included in the error messages. |
src/librustdoc/clean/utils.rs
Outdated
@@ -544,12 +544,14 @@ crate fn has_doc_flag(attrs: ty::Attributes<'_>, flag: Symbol) -> bool { | |||
} | |||
|
|||
/// Return a channel suitable for using in a `doc.rust-lang.org/{channel}` format string. | |||
/// | |||
/// NOTE: keep this in sync with `bootstrap::Builder::doc_rust_lang_org_channel`, or tests will fail on beta/stable. | |||
crate fn doc_rust_lang_org_channel() -> &'static str { | |||
match env!("CFG_RELEASE_CHANNEL") { | |||
"stable" => env!("CFG_RELEASE_NUM"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, linking to CFG_RELEASE_NUM here seems pretty unfortunate - it implies we're going to end up with doc.rust-lang.org/1.45.0 being linked to itself, and will probably only make the "stale docs in google" problem worse?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I don't want to unconditionally link to /stable is because the docs could be different there - the section headings are not part of the stable API, and rustdoc/rustc occasionally delete entire pages of their books.
I haven't noticed stale docs being a problem in practice. The most discussion I've seen is on #56700, which looks like it's been fixed by #69992 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said, this is ultimately a @rust-lang/libs decision, happy to go with whatever you decide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I don't want to unconditionally link to /stable is because the docs could be different there - the section headings are not part of the stable API, and rustdoc/rustc occasionally delete entire pages of their books.
I would think that almost all docs pages that weren't built with a very recent release are using Docs.rs, and those will be using nightly, so they won't link to /1.52.0
. So maybe linking to /stable
wouldn't be that much of an issue? Also, linking to /stable
when appropriate is at least better than unconditionally linking to /nightly
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@camelid this is comparing linking /1.52.0 to linking to /stable; I don't think nightly is relevant.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@bors r+ |
📌 Commit cbe7f6b5183c198c788675ba7b8ea0c1ece9752b has been approved by |
@bors r- I need to fix the ci failures |
This comment has been minimized.
This comment has been minimized.
## User-facing changes - Intra-doc links to primitives that currently go to rust-lang.org/nightly/std/primitive.x.html will start going to channel that rustdoc was built with. Nightly will continue going to /nightly; Beta will link to /beta; stable compilers will link to /1.52.1 (or whatever version they were built as). - Cross-crate links from std to core currently go to /nightly unconditionally. They will start going to /1.52.0 on stable channels (but remain the same on nightly channels). - Intra-crate links from std to std (or core to core) currently go to the same URL they are hosted at; they will continue to do so. Notably, this is different from everything else because it can preserve the distinction between /stable and /1.52.0 by using relative links. Note that "links" includes both intra-doc links and rustdoc's own automatically generated hyperlinks. ## Implementation changes - Update the testsuite to allow linking to /beta and /1.52.1 in docs - Use an html_root_url for the standard library that's dependent on the channel This avoids linking to nightly docs on stable. - Update rustdoc to use channel-dependent links for primitives from an unknown crate - Set DOC_RUST_LANG_ORG_CHANNEL from bootstrap to ensure it's in sync - Include doc.rust-lang.org in the channel
@bors r=Manishearth |
📌 Commit 7411a9e has been approved by |
Rollup of 10 pull requests Successful merges: - rust-lang#83653 (Remove unused code from `rustc_data_structures::sync`) - rust-lang#84466 (rustdoc: Remove `PrimitiveType::{to_url_str, as_str}`) - rust-lang#84880 (Make match in `register_res` easier to read) - rust-lang#84942 (rustdoc: link to stable/beta docs consistently in documentation) - rust-lang#85853 (Warn against boxed DST in `improper_ctypes_definitions` lint) - rust-lang#85939 (Fix suggestion for removing &mut from &mut macro!().) - rust-lang#85966 (wasm: Make simd types passed via indirection again) - rust-lang#85979 (don't suggest unsized indirection in where-clauses) - rust-lang#85983 (Update to semver 1.0.3) - rust-lang#85988 (Note that `ninja = false` goes under `[llvm]`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…ishearth rustdoc: link consistently to stable/beta in diagnostic messages Builds on rust-lang#84942. This makes the diagnostics consistent with the links.
…-blessing, r=Mark-Simulacrum Htmldocck: Substitute the doc channel when blessing Since rust-lang#84942, the snippet `{{channel}}` gets substituted with the concrete “doc channel” (e.g. `https://doc.rust-lang.org/nightly`) when snapshot files are checked against the actual rustdoc output. However, when you `--bless` rustdoc tests, htmldocck just dumps the concrete channel into the snapshot file and you have to manually do a find-and-replace after blessing to uphold what rust-lang#84942 set out to fix. I admit it's a bit fragile to blindly replace URLs like this but I guess it's not too bad in practice. Feel free to close this PR if you don't think that this is a good idea. `@rustbot` label T-rustdoc A-testsuite
This is an alternative to #84941 which fixes the problem consistently by linking to stable/beta for all items, not just for primitives.
User-facing changes
Note that "links" includes both intra-doc links and rustdoc's own
automatically generated hyperlinks.
Implementation changes
Update the testsuite to allow linking to /beta and /1.52.1 in docs
Use an html_root_url for the standard library that's dependent on the channel
This avoids linking to nightly docs on stable.
Update rustdoc to use channel-dependent links for primitives from an
unknown crate
Set DOC_RUST_LANG_ORG_CHANNEL from bootstrap to ensure it's in sync
Include doc.rust-lang.org in the channel
cc Mark-Simulacrum - I know you were dubious about this in the past, but I'm not quite sure why? I see this as "just a bugfix", I don't know why rustdoc should unconditionally link to nightly.
cc dtolnay who commented in #30693:
Fixes #30693 (note that issue is marked as feature-accepted, although I don't see where it was discussed).