-
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
Prevent feature information to be hidden if it's on the impl directly #79300
Conversation
Some changes occurred in HTML/CSS/JS. |
if (hasClass(docblock, "stability") === false) { | ||
addClass(docblock, "hidden-by-usual-hider"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does stability have to do with it? IIUC this is hiding it when you have #[unstable]
in the source - why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that's interesting in fact! Let me check that. If you're right, we're badly handling a few things in there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so "stability" groups a few things, including the feature attributes. Inside it, it is the "portability" class the important one. However, we don't generate items like "deprecated" on implementations as it seems. And even if we did, it'd make sense to print them, and since they're stored inside the stability
element... For reference, it looks like this:
<div class="stability">
<div class="stab portability">This is supported on <strong>non-crate feature <code>traits</code></strong> only.</div>
</div>
And deprecated looks like this:
<div class="stability">
<div class="stab deprecated"><span class="emoji">👎</span> Deprecated</div>
</div>
And when you merge both:
<div class="stability">
<div class="stab deprecated"><span class="emoji">👎</span> Deprecated</div>
<div class="stab portability">This is supported on <strong>non-crate feature <code>traits</code></strong> only.</div>
</div>
So it kinda makes sense, however we should maybe rename the item class. What do you think about renaming it then? However, I'll do it in another PR to prevent mixing too much things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Printing both, even on implementations, sounds like the right approach to me :) happy to wait for another cleanup PR.
8b82849
to
28a94a3
Compare
@bors r+ rollup |
📌 Commit 28a94a3 has been approved by |
…laumeGomez Rollup of 4 pull requests Successful merges: - rust-lang#78670 (Remove FIXME comment in some incremental test suite) - rust-lang#79292 (Fix typo in doc comment for report_too_many_hashes) - rust-lang#79300 (Prevent feature information to be hidden if it's on the impl directly) - rust-lang#79302 (Add regression test for issue 73899) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixed by rust-lang/rust#79300. This reverts commit d9fb416.
…jyn514 Rename "stability" CSS class to "item-info" and combine `document_stability` with `document_short` Follow-up of rust-lang#79300 The point of this PR is to make the CSS class more accurate since it's not only about stability anymore. r? `@jyn514`
…jyn514 Rename "stability" CSS class to "item-info" and combine `document_stability` with `document_short` Follow-up of rust-lang#79300 The point of this PR is to make the CSS class more accurate since it's not only about stability anymore. r? ``@jyn514``
Fixes #79279.
So when a
#[doc(cfg...)]
is used on a trait impl directly, it's not hidden by the toggle.r? @jyn514