rustdoc: introduce the #[doc(keyword="")] attribute for documenting keywords#51140
rustdoc: introduce the #[doc(keyword="")] attribute for documenting keywords#51140bors merged 8 commits intorust-lang:masterfrom
Conversation
fcbcfb9 to
02df550
Compare
QuietMisdreavus
left a comment
There was a problem hiding this comment.
Yay, this is exciting! With this feature we can really extend the docs for some highly-requested features. Overall it looks nice, but i have some small comments.
Can you also put #[doc(keyword="")] behind a feature flag and make a tracking issue for it? We missed the boat with #[doc(primitive="")] but we can at least get this one.
We'll also want an issue to track the keywords we want to document. It could get folded into the #[doc(keyword)] tracking issue, since i imagine we wouldn't really want to stabilize that, but it could also be its own standalone issue.
src/librustdoc/clean/mod.rs
Outdated
There was a problem hiding this comment.
Left this comment as-is from the primitives version. 😆
src/librustdoc/html/render.rs
Outdated
There was a problem hiding this comment.
Wondering if this should join the rest at 100 default capacity, since there are currently 62 valid elements in the Keyword enum, which could feasibly each get documentation.
There was a problem hiding this comment.
Let's put 62 then. :D
src/librustdoc/clean/mod.rs
Outdated
There was a problem hiding this comment.
I wonder if you could use into_iter with this to prevent the clones below. keywords isn't really used outside of this, so there's no use keeping it around.
src/test/rustdoc/keyword.rs
Outdated
There was a problem hiding this comment.
Should we also include a test here to make sure the foo/foo/index.html and related links are missing? Seems like a good chance to use the @!has_dir command.
There was a problem hiding this comment.
I was hoping to have this checked directly once we have added keywords into std docs directly.
There was a problem hiding this comment.
...wait, how would that work? Isn't it the job of these tests to make sure the feature works, without depending on how it's used to pull that together?
There was a problem hiding this comment.
What I meant is that if it fails to link once we use the keyword feature in std docs, it'll fail tests at this moment. :)
There was a problem hiding this comment.
I'm not worried about it placing broken links in the docs - the test is already checking for that. I'm worried about rustdoc creating useless unlinked pages for the original module that's supposed to not exist once rustdoc processes the keyword information out of it. (To be sure, since this code was adapted from the code for primitives, this issue shouldn't exist, but I like more tests.)
What I am suggesting is adding lines like this:
// @!has foo/index.html '//a/@href' 'foo/index.html'
// @!has foo/foo/index.html
// @!has_dir foo/foo
src/librustdoc/html/render.rs
Outdated
There was a problem hiding this comment.
Does this need to have associated items? I don't expect us to link keywords to anything.
There was a problem hiding this comment.
Just like primitives. :)
There was a problem hiding this comment.
Actually, we link loads of things to primitives, because function signatures will have links to them. Keywords, on the other hand, don't have the same association. Since they're not types, per se, there's nothing that will automatically want to reference them. (Unless we want to link every function's fn keyword to that page, every impl block to that page, the trait/struct/enum/etc in type/trait definitions to those pages, etc... I don't quite think it will be useful enough to edge out over the noise of having yet another link.)
There was a problem hiding this comment.
True, I'll remove it then.
src/librustdoc/clean/mod.rs
Outdated
There was a problem hiding this comment.
attrs doesn't need a clone here either.
d4a6c0e to
793e041
Compare
|
Ping from triage, @QuietMisdreavus ! |
|
I'm still waiting on @GuillaumeGomez to respond to #51140 (comment) and follow up on #51140 (comment). |
|
Updated. |
|
Yay, thanks so much! r=me pending travis. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@bors: r=QuietMisdreavus |
|
📌 Commit b9ede5d has been approved by |
|
☔ The latest upstream changes (presumably #50338) made this pull request unmergeable. Please resolve the merge conflicts. |
|
🔒 Merge conflict |
|
Failure is legit. |
|
Oh right, I forget every time that travis doesn't run everything... I'll add the missing piece. |
|
@bors r+ |
|
📌 Commit 2f7fa24 has been approved by |
|
⌛ Testing commit 2f7fa24 with merge 61fc80b27d9b1c1a08ed3b6a0c8f7a1f8bbae870... |
|
💔 Test failed - status-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The error being: I assume we can just retry... @bors: retry |
rustdoc: introduce the #[doc(keyword="")] attribute for documenting keywords Part of #34601. r? @QuietMisdreavus
|
☀️ Test successful - status-appveyor, status-travis |
Part of #34601.
r? @QuietMisdreavus