-
Notifications
You must be signed in to change notification settings - Fork 13.5k
rustdoc: show attributes on enum variants #142987
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
base: master
Are you sure you want to change the base?
rustdoc: show attributes on enum variants #142987
Conversation
rustbot has assigned @GuillaumeGomez. Use |
232bdaf
to
a73c70e
Compare
Looks good to me, thanks! Since @fmease opened the issue, I'll let them take a look before r+ing. |
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.
Attribute-related changes look fine to me.
It might be worth noting that for non-exhaustive structs, below the "introductory code snippet" we say Fields (Non-exhaustive)
and have a <details>
section explaining what that implies whereas for enum variants, we just say Fields
but we do have that same <details>
section. Not sure if y'all want to address that in this PR.
(Does this show up in Aliased Type
sections? Just curious. I know that we only allowlist repr
for aliased enums and structs and we should include #[non_exhaustive]
at some point but that's a another story for another day).
Normally it should since #140863 which unified alias rendering with the other ADTs. However I suppose adding a test to ensure it is rendering as expected would be a good idea. Can you add a type alias on the enum in your rustdoc test @lolbinarycat please? |
a73c70e
to
4bece45
Compare
Yes it shows up in the aliased type. Added a test for that (unfortunately I couldn't make it super robust due to not having access to all of XPath) |
|
||
// we would love to use the `following-sibling::` axis | ||
// (along with an `h2[@id="aliased-type"]` query), | ||
// but unfortunatly python doesn't implement that. |
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.
// but unfortunatly python doesn't implement that. | |
// but unfortunately python doesn't implement that. |
@@ -0,0 +1 @@ | |||
<code>pub type TypeAlias = <a class="enum" href="enum.Type.html" title="enum foo::Type">Type</a>;</code> |
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.
It's not what we want to check. It's the declaration of the aliased pub enum
and in particular to ensure that its variants have the attributes displayed.
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.
whoops, yeah that's just a simple mistake that wouldn't have happened if we had access to following-sibling::
, will fix.
mostly for #[non_exhaustive]
4bece45
to
b2be01c
Compare
Variant, | ||
} | ||
|
||
// we would love to use the `following-sibling::` axis |
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.
Does this comment still make sense? Current code is fine and I'm not sure to understand what follow-sibling
added value would be here.
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.
it would allow us to select "the code block in the aliased type section" instead of "the second code block" which would be robust against us adding additional code blocks or rearranging them.
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.
Fair enough!
Thanks! @bors r+ rollup |
…e-enum-v-142599, r=GuillaumeGomez rustdoc: show attributes on enum variants mostly for #[non_exhaustive] unsure if there's any attributes we should take care to *not* include, it could use `render_code_attribute` and `is_non_exhaustive` instead, if that is a concern. fixes rust-lang#142599
…rors Rollup of 8 pull requests Successful merges: - #139858 (New const traits syntax) - #140809 (Reduce special casing for the panic runtime) - #142963 (Skip unnecessary components in x64 try builds) - #142974 (Update stage0 to 1.89.0-beta.1) - #142987 (rustdoc: show attributes on enum variants) - #143002 (tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok)) - #143092 (const checks for lifetime-extended temporaries: avoid 'top-level scope' terminology) - #143096 (tag_for_variant: properly pass TypingEnv) r? `@ghost` `@rustbot` modify labels: rollup
mostly for #[non_exhaustive]
unsure if there's any attributes we should take care to not include, it could use
render_code_attribute
andis_non_exhaustive
instead, if that is a concern.fixes #142599