Rustdoc - display since version for stable items by wesleywiser · Pull Request #30686 · 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
Conversation18 Commits1 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 }})
Here's some screenshots after this change:
I tried to click through the std docs and make sure everything that can have stability attributes has it rendered but I'm probably missing some. I'd also appreciate any feedback on the css changes. I had difficulty getting the since labels aligning correctly for enum variants. If anyone has a better idea for that, I'd be glad to implement it.
Fixes #27607
I would prefer if the text was less prominent: either greyed or with smaller font. It is too distracting at the moment for something that's so meta.
I also have the sense that this is giving a great deal of prominence to a rather unimportant piece of information.
One thing we could do is remove 'since' - this word doesn't convey any information every time it's repeated.
This is also a new way to style stability information, and I already count two - See for example where stability of the element on the page is indicated by a colored box while stability of its methods is indicated by the word '[Unstable]' while fading out the text. It would be easier to understand that this and that are both types of stability information if they were styled similarly.
@brson re 'since': I agree. Here's what it looks like without 'since':
I think agree that adding yet another way to show stability information is bad. I'm having some trouble imagining what a common style might look like.
One idea I had that might further minimize the visual noise would be to only show the version number when it is different than the containing object's version number. For example, the previous screenshot would look like this because Option is marked 1.0.0:
I like the second option:
...only show the version number when it is different than the containing object's version number.
@wesleywiser I do like that idea of not displaying them when they are the same. With that I think I'm ok with trying this.
I basically feel the same as @brson on this thread. Let's give it a try. Let me know when this PR implements that, and we'll merge after the release thursday, assuming you can update it before then.
@brson @steveklabnik Will do. Sorry, I've been pretty busy lately but hopefully I'll have some time to work on it later this week. I'll post new screenshots when when I update the PR.
@wesleywiser ping! Any chance you've had the time to work on this?
| @@ -339,6 +339,14 @@ impl Item { |
|---|
| _ => String::new(), |
| } |
| } |
| pub fn stable_since(&self) -> Option { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use -> Option<&str> here instead? It should be enough since the string is only ever passed down. Change s.since.clone() to &s.since[..] too and update the other impacted code.
I think this change looks great otherwise.
@steveklabnik Great timing Steve! I think it's mostly working at this point (I looked over the generated docs and didn't see any glaring issues). I've updated this PR with the new code (and also implemented @bluss's suggestion).
@bors: r+
okay, let's give it a try! 🎊
📌 Commit 75acee2 has been approved by steveklabnik
bors added a commit that referenced this pull request
☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt
This was referenced
Nov 29, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request
bors added a commit to rust-lang-ci/rust that referenced this pull request
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request