Use intra-doc links in str and BTreeSet by Manishearth · Pull Request #74470 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation20 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Partial fix for #32129 and #32130
A slight degradation in quality is that the #method.foo links would previously link to the same page on String's documentation, and now they will navigate to str. Not a big deal IMO, and we can also try to improve that.
r? @kennytm
(rust_highfive has picked a reviewer for you, use r? to override)
📌 Commit 748634e has been approved by jyn514
🌲 The tree is currently closed for pull requests below priority 5, this pull request will be tested once the tree is reopened
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
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
@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.
To be clear, it is better to fix the broken links on the String page, but I consider it a backstep in other regards that should (eventually) be fixed.
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?
Hmm ... Would it makes sense to use intra-doc links only for things that aren't primitives?
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 jyn514
🌲 The tree is currently closed for pull requests below priority 5, this pull request will be tested once the tree is reopened
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
@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:
| // FIXME(#32129) |
|---|
| if file.ends_with("std/io/struct.IoSlice.html") |
| | |
| { |
| return None; |
| } |
and
| // FIXME(#32130) |
|---|
| if file.ends_with("alloc/collections/btree_map/struct.BTreeMap.html") |
| | |
| | |
| | |
| | |
| | |
| { |
| return None; |
| } |
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
@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 jyn514
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
This was referenced
Aug 3, 2020
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.