-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Remove underlines from non-top docblocks. #90156
Conversation
Some changes occurred in HTML/CSS/JS. |
Looks good to me. I'm not sure if adding a test would help here since we're removing something... My test-maniac brain tells me yes but I don't see how a test could really help here. Can you maybe check the font-size at least in different docblocks for different headings please? |
Good idea! I added a test that checks the font size of a variety of different headings, in both Prose and Rusty headings. It's a nice snapshot of how we do headings today, and the HTML it generates is a nice way to look at a variety of heading levels and see if they make sense. |
b472605
to
d28b194
Compare
Considering it's small and easy to backport, I'd be in favour of backporting it to beta. |
@Mark-Simulacrum would it be reasonable to open a PR to the beta branch that just fixes the heading level (it's a very small fix)? I don't think it'd be good to backport this whole PR since it makes other changes and somewhat depends on previous PRs that haven't made it to beta yet. If Mark says it's okay, could you open a PR to the beta branch with just the bugfix @jsha? |
Actually, poking around I'm finding a couple of other places that need the H2->H4 fix. I'll start a separate PR that has those fixes, plus the rustdoc-gui unittest. For now I'll pull those changes out of this PR. |
d28b194
to
c8a917d
Compare
I'm going to consider this blocked on #90186 because without that change, the styles won't apply to the heading tags (since they're being generated with the wrong heading level). |
…omez Fix documentation header sizes And add a rustdoc-gui test confirming various header sizes. Split off from rust-lang#90156. This fixes a regression in rust-lang#89506 where the heading level of titles within Markdown was too high (h2) for docblocks under structs, unions, and enum impls. r? `@camelid` Demo: https://jacob.hoffman-andrews.com/rust/fix-header-sizes/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E Stable: https://doc.rust-lang.org/stable/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E Beta: https://doc.rust-lang.org/beta/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E
…omez Fix documentation header sizes And add a rustdoc-gui test confirming various header sizes. Split off from rust-lang#90156. This fixes a regression in rust-lang#89506 where the heading level of titles within Markdown was too high (h2) for docblocks under structs, unions, and enum impls. r? ``@camelid`` Demo: https://jacob.hoffman-andrews.com/rust/fix-header-sizes/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E Stable: https://doc.rust-lang.org/stable/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E Beta: https://doc.rust-lang.org/beta/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E
…omez Fix documentation header sizes And add a rustdoc-gui test confirming various header sizes. Split off from rust-lang#90156. This fixes a regression in rust-lang#89506 where the heading level of titles within Markdown was too high (h2) for docblocks under structs, unions, and enum impls. r? `@camelid` Demo: https://jacob.hoffman-andrews.com/rust/fix-header-sizes/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E Stable: https://doc.rust-lang.org/stable/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E Beta: https://doc.rust-lang.org/beta/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E
Fix documentation header sizes And add a rustdoc-gui test confirming various header sizes. Split off from rust-lang#90156. This fixes a regression in rust-lang#89506 where the heading level of titles within Markdown was too high (h2) for docblocks under structs, unions, and enum impls. r? `@camelid` Demo: https://jacob.hoffman-andrews.com/rust/fix-header-sizes/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E Stable: https://doc.rust-lang.org/stable/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E Beta: https://doc.rust-lang.org/beta/std/string/struct.String.html#impl-Add%3C%26%27_%20str%3E
c8a917d
to
8c87b0c
Compare
I've rebased on top of #90186. @GuillaumeGomez want to take a second look and give bors approval if it's still looking good? |
We still had a number of places where underlined section headings would show up, like under Implementations.
8c87b0c
to
a65c98f
Compare
Thanks! @bors: r+ rollup |
📌 Commit a65c98f has been approved by |
…laumeGomez Rollup of 5 pull requests Successful merges: - rust-lang#90156 (Remove underlines from non-top docblocks.) - rust-lang#90183 (Show all Deref implementations recursively) - rust-lang#90202 (Improve and test cross-crate hygiene) - rust-lang#90375 (Use `is_global` in `candidate_should_be_dropped_in_favor_of`) - rust-lang#90399 (Skipping verbose diagnostic suggestions when calling .as_ref() on type not implementing AsRef) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
We still had a number of places where underlined section headings would
show up, like under Implementations.
Follow-up to #89506 (thanks @yaymukund!) and #90036. Related to #59829.
r? @camelid
Demo:
Before:
After: