-
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
rustdoc: show crate name beside smaller logo #115948
Conversation
r? @fmease (rustbot has picked a reviewer for you, use r? to override) |
Some changes occurred in GUI tests. Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @jsha |
0389aa6
to
0b1555b
Compare
Some changes occurred in GUI tests. Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @jsha |
I really like when the name is small and right beside the logo. But for the rest, no opinion. I'm not sure removing |
This comment has been minimized.
This comment has been minimized.
0b1555b
to
ba2558b
Compare
This comment has been minimized.
This comment has been minimized.
AFAIK, there isn't any way to make the text different for short or long names, because that's dependent on typesetting information that librustdoc doesn't have. The layout shift is done using frontend CSS, limiting how elaborate I can get. I'm not sure it's a good idea to have the word "Crate" there, anyway. It made some sense when the crate wasn't always present, but if this part of the sidebar is going to be persistent, then we probably want it to be terse so it doesn't take up too much space.
It definitely looks good on the standard library. It's especially helpful because it gives a perpetual version indicator, even when not on the crate root page, For non-standard library crates, I'd really like to know if there's a written rationale for having the Rust logo there. If it's just a matter of wanting a link to the crate root page, then this PR can just remove it and save the screen real estate and the bandwidth. |
ba2558b
to
b91c786
Compare
This comment has been minimized.
This comment has been minimized.
7f0d183
to
942f98f
Compare
This comment has been minimized.
This comment has been minimized.
51f43e5
to
b2be1c3
Compare
Personally speaking, I'd be in favor of dropping the Rust logo in those cases. I bet it takes a bit of digging to find the PR that introduced this. It probably goes all the way back to the pre-1.0 days.
We could also experiment with inserting
Agreed. I haven't looked into the finicky bits of the implementation yet (like all those negative margins) but generally it looks reasonable. |
This PR changes Stable MIR cc @oli-obk, @celinval, @spastorino |
This comment has been minimized.
This comment has been minimized.
1a56959
to
a678cff
Compare
This comment has been minimized.
This comment has been minimized.
a678cff
to
b96c693
Compare
Needed for rust-lang/rust#115948 to merge.
Theoretically, you could've updated the Cargo tests directly through the git submodule |
rustdoc: remove the word "Version" from test cases Needed for rust-lang/rust#115948 to merge. That PR gets rid of the word "Version" in rustdoc's HTML output, and it splits spaced versions on their first space, to fit in the tight horizontal spacing. This causes Cargo's test suite to fail, because it look for the word "Version", even though things are working as they should. These tests work on both current nightly and on that pull request.
@bors r=fmease |
☀️ Test successful - checks-actions |
Finished benchmarking commit (6d05c43): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 626.975s -> 627.49s (0.08%) |
48: Pull upstream master 2023 10 12 r=tshepang a=Dajamante * rust-lang/rust#113487 * rust-lang/rust#116506 * rust-lang/rust#116448 * rust-lang/rust#116640 * rust-lang/rust#116627 * rust-lang/rust#116597 * rust-lang/rust#116436 * rust-lang/rust#116315 * rust-lang/rust#116219 * rust-lang/rust#113218 * rust-lang/rust#115937 * rust-lang/rust#116014 * rust-lang/rust#116623 * rust-lang/rust#112818 * rust-lang/rust#115948 * rust-lang/rust#116622 * rust-lang/rust#116621 * rust-lang/rust#116612 * rust-lang/rust#116611 * rust-lang/rust#116530 * rust-lang/rust#95967 * rust-lang/rust#116578 * rust-lang/rust#113915 * rust-lang/rust#116605 * rust-lang/rust#116574 * rust-lang/rust#116560 * rust-lang/rust#116559 * rust-lang/rust#116503 * rust-lang/rust#116444 * rust-lang/rust#116250 * rust-lang/rust#109422 * rust-lang/rust#116598 * rust-lang/rust#116596 * rust-lang/rust#116595 * rust-lang/rust#116589 * rust-lang/rust#116586 * rust-lang/rust#116551 * rust-lang/rust#116409 * rust-lang/rust#116548 * rust-lang/rust#116366 * rust-lang/rust#109882 * rust-lang/rust#116497 * rust-lang/rust#116532 * rust-lang/rust#116569 * rust-lang/rust#116561 * rust-lang/rust#116556 * rust-lang/rust#116549 * rust-lang/rust#116543 * rust-lang/rust#116537 * rust-lang/rust#115882 * rust-lang/rust#116142 * rust-lang/rust#115238 * rust-lang/rust#116533 * rust-lang/rust#116096 * rust-lang/rust#116468 * rust-lang/rust#116515 * rust-lang/rust#116454 * rust-lang/rust#116183 * rust-lang/rust#116514 * rust-lang/rust#116509 * rust-lang/rust#116487 * rust-lang/rust#116486 * rust-lang/rust#116450 * rust-lang/rust#114623 * rust-lang/rust#116416 * rust-lang/rust#116437 * rust-lang/rust#100806 * rust-lang/rust#116330 * rust-lang/rust#116310 * rust-lang/rust#115583 * rust-lang/rust#116457 * rust-lang/rust#116508 * rust-lang/rust#109214 * rust-lang/rust#116318 * rust-lang/rust#116501 * rust-lang/rust#116500 * rust-lang/rust#116458 * rust-lang/rust#116400 * rust-lang/rust#116277 * rust-lang/rust#114709 * rust-lang/rust#116492 * rust-lang/rust#116484 * rust-lang/rust#116481 * rust-lang/rust#116474 * rust-lang/rust#116466 * rust-lang/rust#116423 * rust-lang/rust#116297 * rust-lang/rust#114564 * rust-lang/rust#114811 * rust-lang/rust#116489 * rust-lang/rust#115304 Co-authored-by: Peter Hall <peter.hall@hyperexponential.com> Co-authored-by: Emanuele Vannacci <emanuele.vannacci@gmail.com> Co-authored-by: Neven Villani <vanille@crans.org> Co-authored-by: Alex Macleod <alex@macleod.io> Co-authored-by: Tamir Duberstein <tamird@gmail.com> Co-authored-by: Eduardo Sánchez Muñoz <eduardosm-dev@e64.io> Co-authored-by: koka <koka.code@gmail.com> Co-authored-by: bors <bors@rust-lang.org> Co-authored-by: Philipp Krones <hello@philkrones.com> Co-authored-by: Camille GILLOT <gillot.camille@gmail.com> Co-authored-by: Esteban Küber <esteban@kuber.com.ar> Co-authored-by: Ralf Jung <post@ralfj.de>
48: Pull upstream master 2023 10 12 r=tshepang a=Dajamante * rust-lang/rust#113487 * rust-lang/rust#116506 * rust-lang/rust#116448 * rust-lang/rust#116640 * rust-lang/rust#116627 * rust-lang/rust#116597 * rust-lang/rust#116436 * rust-lang/rust#116315 * rust-lang/rust#116219 * rust-lang/rust#113218 * rust-lang/rust#115937 * rust-lang/rust#116014 * rust-lang/rust#116623 * rust-lang/rust#112818 * rust-lang/rust#115948 * rust-lang/rust#116622 * rust-lang/rust#116621 * rust-lang/rust#116612 * rust-lang/rust#116611 * rust-lang/rust#116530 * rust-lang/rust#95967 * rust-lang/rust#116578 * rust-lang/rust#113915 * rust-lang/rust#116605 * rust-lang/rust#116574 * rust-lang/rust#116560 * rust-lang/rust#116559 * rust-lang/rust#116503 * rust-lang/rust#116444 * rust-lang/rust#116250 * rust-lang/rust#109422 * rust-lang/rust#116598 * rust-lang/rust#116596 * rust-lang/rust#116595 * rust-lang/rust#116589 * rust-lang/rust#116586 * rust-lang/rust#116551 * rust-lang/rust#116409 * rust-lang/rust#116548 * rust-lang/rust#116366 * rust-lang/rust#109882 * rust-lang/rust#116497 * rust-lang/rust#116532 * rust-lang/rust#116569 * rust-lang/rust#116561 * rust-lang/rust#116556 * rust-lang/rust#116549 * rust-lang/rust#116543 * rust-lang/rust#116537 * rust-lang/rust#115882 * rust-lang/rust#116142 * rust-lang/rust#115238 * rust-lang/rust#116533 * rust-lang/rust#116096 * rust-lang/rust#116468 * rust-lang/rust#116515 * rust-lang/rust#116454 * rust-lang/rust#116183 * rust-lang/rust#116514 * rust-lang/rust#116509 * rust-lang/rust#116487 * rust-lang/rust#116486 * rust-lang/rust#116450 * rust-lang/rust#114623 * rust-lang/rust#116416 * rust-lang/rust#116437 * rust-lang/rust#100806 * rust-lang/rust#116330 * rust-lang/rust#116310 * rust-lang/rust#115583 * rust-lang/rust#116457 * rust-lang/rust#116508 * rust-lang/rust#109214 * rust-lang/rust#116318 * rust-lang/rust#116501 * rust-lang/rust#116500 * rust-lang/rust#116458 * rust-lang/rust#116400 * rust-lang/rust#116277 * rust-lang/rust#114709 * rust-lang/rust#116492 * rust-lang/rust#116484 * rust-lang/rust#116481 * rust-lang/rust#116474 * rust-lang/rust#116466 * rust-lang/rust#116423 * rust-lang/rust#116297 * rust-lang/rust#114564 * rust-lang/rust#114811 * rust-lang/rust#116489 * rust-lang/rust#115304 Co-authored-by: Emanuele Vannacci <emanuele.vannacci@gmail.com> Co-authored-by: Neven Villani <vanille@crans.org> Co-authored-by: Alex Macleod <alex@macleod.io> Co-authored-by: Tamir Duberstein <tamird@gmail.com> Co-authored-by: Eduardo Sánchez Muñoz <eduardosm-dev@e64.io> Co-authored-by: koka <koka.code@gmail.com> Co-authored-by: bors <bors@rust-lang.org> Co-authored-by: Philipp Krones <hello@philkrones.com> Co-authored-by: Camille GILLOT <gillot.camille@gmail.com> Co-authored-by: Esteban Küber <esteban@kuber.com.ar> Co-authored-by: Ralf Jung <post@ralfj.de> Co-authored-by: ShE3py <52315535+she3py@users.noreply.github.com>
Needed for rust-lang/rust#115948 to merge.
@@ -9,6 +9,9 @@ | |||
html_playground_url = "https://play.rust-lang.org/", | |||
test(attr(deny(warnings))) | |||
)] | |||
#![cfg_attr(not(bootstrap), doc(rust_logo))] | |||
#![cfg_attr(not(bootstrap), allow(internal_features))] | |||
#![cfg_attr(not(bootstrap), feature(rustdoc_internals))] |
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.
// We want to be able to build this crate with a stable compiler, so no
// `#![feature]` attributes should be added.
This broke the crate on stable
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.
Oof, sorry. I absolutely missed that while reviewing. I'm gonna open a PR later. Are there projects which are already affected by this? r-a? Should I nominate that PR for any backports (none, beta, beta+stable)?
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.
No need for backports, anyone relying on these via rustc_private are required to be on nightly anyways. This was merely noticed in r-a when we wanted to bump our autopublished copy, so no rush.
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.
For crates like these it'd be nice if there was a #[forbid(feature_attribute)]
lint or something
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.
I don't think this happened yet? Can I just delete the three lines?
…GuillaumeGomez rustdoc: use `.rustdoc` class instead of `body` This didn't show up in our local tests, because the problem is actually caused by docs.rs rewritten HTML (which relocates the classes that this code looked for from the body tag to a child div). Fixes rust-lang#117290 r? `@GuillaumeGomez` Both problems are regressions introduced by rust-lang#115948
…tc, r=Nilstrieb Make `rustc_parse_format` compile on stable again Fixes [rust-lang#115948 (review comment)](https://github.com/rust-lang/rust/pull/115948/files/822233559619e7e77d984f9020d05302b784cf50#r1385932710). cc `@Veykril` `@notriddle` r? compiler
Blocked on rust-lang/cargo#12800
Summary
In this PR, the crate name and version are always shown in the sidebar, even in subpages, and the lateral navigation is always shown in the sidebar, even in modules.
Clicking the crate name does the same thing clicking the logo always did: take you to the crate root (the crate's home page, at least within Rustdoc).
The Rust logo is also no longer shown by default for non-Rust docs.
Screenshots
Before
Whole sidebar screenshots
Different logo configurations
Without a logo
Preview pages
https://notriddle.com/rustdoc-html-demo-5/sidebar-layout-rocket/rocket/index.html
https://notriddle.com/rustdoc-html-demo-5/sidebar-layout-rocket/rocket_sync_db_pools/index.html
https://notriddle.com/rustdoc-html-demo-5/sidebar-layout-rust-compiler/index.html
https://notriddle.com/rustdoc-html-demo-5/sidebar-layout-rust/std/index.html
https://notriddle.com/rustdoc-html-demo-5/sidebar-layout-rocket/tokio/index.html
Motivation
This improves visual information density (the construct with the logo and crate name is shorter than the logo on its own, because it's not square) and navigation clarity (we can now see what clicking the Rust logo does, specifically).
Compare this with the layout at Phoenix's Hexdocs (which is what this proposal is closely based on), the old proposal on Internals Discourse (which always says "Rust standard library" in the sidebar, but doesn't do the side-by-side layout).
Guide-level explanation
This PR cleans up some of the sidebar navigation.
It makes the logo in the desktop sidebar a bit smaller, and puts the crate name and version next to it (either beside it, or below it, depending on if there's space), making it clearer what clicking on it does: click the crate name to open the crate's home page. It also removes the Rust logo from non-official-Rust crates, again to make the navigation and supply chain clearer (since the crate name has been added, the logo is no longer necessary for navigation).
It adds a bit more clarifying information for lateral navigation. On items that don't add their own sidebar items, it just shows its siblings directly below the crate name and logo, but for other items, it shows "In crate alloc" instead of just "In alloc". It also shows the lateral navigation tools on module pages, making modules consistent with every other item.
Drawbacks
While this actually takes up less screen real estate than the old layout on desktop, it takes up more HTML. It's also a bit more visually complex.
Rationale and alternatives
I could do what the Internals POC did and keep the vertically stacked layout all the time, instead of doing a horizontal stack where possible. It would take up more screen real estate, though.
Prior art
This design is lifted almost verbatim from Hexdocs. It seems to work for them.
opentelemetry_process_propagator
, for example, has a long application name.Unresolved questions
Maybe we should encourage crate authors to include their own logo more often? It certainly helps give people a better sense of "place." This seems to be blocked on coming up with an API to do it without requiring them to host the file somewhere.
Future possibilities
Beyond this, plenty of other changes could be made to improve the layout, like
[-]
on the page.But those are mostly orthogonal, not future possibilities unlocked by this change.
Footnotes
This PR also includes a bug fix for derive macros not showing up in the lateral navigation part of the sidebar ↩