rustdoc: Only include a stability span if needed. #39740
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
src/librustdoc/html/render.rs
Outdated
| write!(w, "<span class='stab {stab}'></span>", | ||
| stab = stability_class)?; | ||
| } | ||
| write!(w, "</span>")?; |
There was a problem hiding this comment.
This extra close looks suspicious, and is likely the cause of the failing Travis build (which is failing on a rustdoc test).
src/librustdoc/clean/mod.rs
Outdated
| if !s.deprecated_since.is_empty() { | ||
| base.push_str(" deprecated"); | ||
| pub fn stability_class(&self) -> Option<String> { | ||
| match self.stability { |
There was a problem hiding this comment.
I believe (I'm no expert either) this would be more idiomatic:
self.stability.as_ref().and_then(|ref s| {
// ...
})
one level less of indentation, no need to map None to None. I made a tiny playground script to make sure they both mean the same thing if you wanna take a look https://is.gd/5fSbZf
There was a problem hiding this comment.
Ah, yes, you're right. It was originally written that way, but I changed it because I got confused between Option::map and Option::and_then, thinking the closure couldn't return an option. I'll switch it back.
|
Thanks for the changes -- looks great now! The remaining Travis failures appear to be spurious. Let's see what bors says. @bors: r+ |
|
📌 Commit 1fa9dbc has been approved by |
…, r=aturon rustdoc: Only include a stability span if needed. This patch gets rid of the empty stability boxes in docs by only including the span that creates it when the item actually has a stability class. Here are images of the issue on `std::process::Output`: Before: <img width="340" alt="before" src="https://cloud.githubusercontent.com/assets/122457/22853638/ff88d1b2-f010-11e6-90d6-bf3d10e2fffa.png"> After: <img width="333" alt="after" src="https://cloud.githubusercontent.com/assets/122457/22853639/06bfe7cc-f011-11e6-9892-f0ea2cc6ec90.png"> This is my first non-trivial patch to Rust, so I'm sure some of my approach is not idiomatic. Let me know how you'd like me to adjust!
This patch gets rid of the empty stability boxes in docs by only including the span that creates it when the item actually has a stability class.
Here are images of the issue on
std::process::Output:Before:
After:
This is my first non-trivial patch to Rust, so I'm sure some of my approach is not idiomatic. Let me know how you'd like me to adjust!