-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Use intra-doc links in str
and BTreeSet
#74470
Conversation
r? @kennytm (rust_highfive has picked a reviewer for you, use r? to override) |
📌 Commit 748634e has been approved by |
🌲 The tree is currently closed for pull requests below priority 5, this pull request will be tested once the tree is reopened |
@Manishearth why did you open a new PR? AFAIK the concerns in #74453 (comment) were never addressed. @bors r- cc @ehuss |
@jyn514 I accidentally deleted some branches and then force pushed It did not seem like those were blocking concerns. They cannot, at any rate, be fixed in this PR. |
Hmm ... Would it makes sense to use intra-doc links only for things that aren't primitives? Or is it not possible to use relative links because this module is re-exported in both std and core? |
This wouldn't fix the original problem though. Note that it's not just primitives, it's methods of primitives, etc. |
Ok I misunderstood what happened - currently these links work in libcore but are broken in libstd. This PR makes it so they work in both crates, but link externally from libcore (they'll go to rust-lang.org/nightly). The second bit is unfortunate but IMO better than the status quo. We should open an issue about the primitives, this has popped up a few different times now. @bors r+ |
📌 Commit 748634e has been approved by |
🌲 The tree is currently closed for pull requests below priority 5, this pull request will be tested once the tree is reopened |
@jyn514 I was starting to draft an issue about the primitives, but I remembered that there may not be a solution here because primitives do not exist in docs before std. The code that makes this an external link is deliberately written to handle this case. |
@Manishearth why are primitives in libstd and not libcore? That seems weird. |
Those issues can't be closed until the exceptions in linkchecker are removed: rust/src/tools/linkchecker/main.rs Lines 122 to 127 in d3df851
rust/src/tools/linkchecker/main.rs Lines 132 to 141 in d3df851
|
@bors r- |
@ollie27 hmm. Unfortunately not all of those exceptions can be removed, because some of the links are forward references, which is out of scope for intra doc links |
@ollie27 would you be okay if we can land this, fixing most of the cases, and unregistering the concern on intra doc stabilization? |
@bors r=jyn514 removed the "fixes foo" keyword from the PR body |
📌 Commit 748634e has been approved by |
Partial fix for #32129 and #32130
A slight degradation in quality is that the
#method.foo
links would previously link to the same page onString
's documentation, and now they will navigate tostr
. Not a big deal IMO, and we can also try to improve that.