Skip to content
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

Merged
merged 9 commits into from
Oct 11, 2023

Conversation

notriddle
Copy link
Contributor

@notriddle notriddle commented Sep 18, 2023

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
Macro Module
In crate image image
In module1 image image

Whole sidebar screenshots

Macro Module
In crate image image
In module image image

Different logo configurations

Short crate name Long crate name
Root short-root long-root
Subpage short-subpage long-subpage
Without a logo

image

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

  • Fix things so that clicking an item in the sidebar doesn't cause it to scroll back to the top.
    • The Internals demo does this right: clicking an item in the sidebar changes the content area, but the sidebar itself does not change. This is nice, because clicking is cheap and I can skim the opening few paragraphs while browsing.
    • The layout of the docs sidebar causes trouble to implement this, because it's different on different pages, but at least fix this on the file browser.
  • Come up with a less cluttered way to do disclosure. There's a lot of [-] on the page.
    • We don't lack ideas to fix this one. We have too many.
  • Do a better job of separating local navigation (vec::Vec links to vec::IntoIter) and the table of contents (vec::Vec links to vec::Vec::new).
    • A possibility: add a Back arrow next to the "In [module]" header?
      image
  • Give readers more control of how much rustdoc shows them, and giving doc authors more control of how much it generates. Basically, rustdoc: allow resizing the sidebar / hiding the top bar #115660 is great, let's do it too.

But those are mostly orthogonal, not future possibilities unlocked by this change.

Footnotes

  1. This PR also includes a bug fix for derive macros not showing up in the lateral navigation part of the sidebar

@rustbot
Copy link
Collaborator

rustbot commented Sep 18, 2023

r? @fmease

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 18, 2023
@rustbot
Copy link
Collaborator

rustbot commented Sep 18, 2023

Some changes occurred in GUI tests.

cc @GuillaumeGomez

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@notriddle notriddle changed the title rustdoc: always show crate name beside logo rustdoc: show crate name beside smaller logo Sep 18, 2023
@rustbot
Copy link
Collaborator

rustbot commented Sep 18, 2023

Some changes occurred in GUI tests.

cc @GuillaumeGomez

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@GuillaumeGomez
Copy link
Member

I really like when the name is small and right beside the logo. But for the rest, no opinion. I'm not sure removing Crate from the text is a good idea too (for long names).

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@notriddle
Copy link
Contributor Author

But for the rest, no opinion. I'm not sure removing Crate from the text is a good idea too (for long names).

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.

I really like when the name is small and right beside the logo.

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,

image

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.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Sep 19, 2023
@rust-log-analyzer

This comment has been minimized.

@notriddle notriddle force-pushed the notriddle/logo-lockup branch 2 times, most recently from 7f0d183 to 942f98f Compare September 19, 2023 01:00
@rust-log-analyzer

This comment has been minimized.

@notriddle notriddle force-pushed the notriddle/logo-lockup branch 3 times, most recently from 51f43e5 to b2be1c3 Compare September 19, 2023 01:57
@fmease
Copy link
Member

fmease commented Sep 19, 2023

If we turned it off on non-standard crates by default, it would line wrap crate names a lot less often.

For non-standard library crates, I'd really like to know if there's a written rationale for having the Rust logo there.

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.

it would line wrap crate names a lot less often

We could also experiment with inserting <wbr>s after underscores in crate names (or any identifier actually) which might lead to more legible output, although that's probably outside the scope of this PR.

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.

Agreed.


I haven't looked into the finicky bits of the implementation yet (like all those negative margins) but generally it looks reasonable.

@rustbot rustbot added the A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) label Sep 19, 2023
@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2023

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

notriddle added a commit to notriddle/cargo that referenced this pull request Oct 10, 2023
@fmease
Copy link
Member

fmease commented Oct 10, 2023

Theoretically, you could've updated the Cargo tests directly through the git submodule src/tools/cargo which is part of this repo if I'm not mistaken instead of opening a PR over at rust-lang/cargo.

bors added a commit to rust-lang/cargo that referenced this pull request Oct 10, 2023
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.
@notriddle
Copy link
Contributor Author

@bors r=fmease

@bors
Copy link
Contributor

bors commented Oct 11, 2023

📌 Commit 8222335 has been approved by fmease

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Oct 11, 2023
@bors
Copy link
Contributor

bors commented Oct 11, 2023

⌛ Testing commit 8222335 with merge 6d05c43...

@bors
Copy link
Contributor

bors commented Oct 11, 2023

☀️ Test successful - checks-actions
Approved by: fmease
Pushing 6d05c43 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 11, 2023
@bors bors merged commit 6d05c43 into rust-lang:master Oct 11, 2023
12 checks passed
@rustbot rustbot added this to the 1.75.0 milestone Oct 11, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6d05c43): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.4% [-0.4%, -0.4%] 1
Improvements ✅
(secondary)
-0.5% [-0.5%, -0.5%] 1
All ❌✅ (primary) -0.4% [-0.4%, -0.4%] 1

Max RSS (memory usage)

Results

This 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.

mean range count
Regressions ❌
(primary)
2.4% [1.9%, 2.9%] 2
Regressions ❌
(secondary)
2.1% [0.9%, 5.1%] 6
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.7% [-2.6%, -1.4%] 4
All ❌✅ (primary) 2.4% [1.9%, 2.9%] 2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 626.975s -> 627.49s (0.08%)
Artifact size: 271.30 MiB -> 271.28 MiB (-0.01%)

@notriddle notriddle deleted the notriddle/logo-lockup branch October 11, 2023 13:30
bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Oct 12, 2023
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>
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Oct 12, 2023
bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Oct 13, 2023
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>
epage pushed a commit to epage/cargo that referenced this pull request Oct 18, 2023
@@ -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))]
Copy link
Member

@Veykril Veykril Nov 8, 2023

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

Copy link
Member

@fmease fmease Nov 8, 2023

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)?

Copy link
Member

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.

Copy link
Member

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

Copy link
Member

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?

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 13, 2023
…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
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 15, 2023
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.