rustdoc: Hide item contents, not items #83337

Manishearth · 2021-03-21T03:26:07Z

This tweaks rustdoc to hide item contents instead of items, and only when there are too many of them.

This means that users will always see the type parameters, and will often see fields/etc as long as they are small. Traits have some heuristics for hiding only the methods or only the methods and the consts, since the associated types are super important.

I'm happy to play around with the heuristics here; we could potentially make it so that structs/enums/etc are always hidden but traits will try really hard to show type aliases.

This needs a test, but you can see it rendered at https://manishearth.net/sand/doc_render/bar/

Code example

pub struct PubStruct {
    pub a: usize,
    pub b: usize,
}

pub struct BigPubStruct {
    pub a: usize,
    pub b: usize,
    pub c: usize,
    pub d: usize,
    pub e: usize,
    pub f: usize,
}

pub union BigUnion {
    pub a: usize,
    pub b: usize,
    pub c: usize,
    pub d: usize,
    pub e: usize,
    pub f: usize,
}

pub union Union {
    pub a: usize,
    pub b: usize,
    pub c: usize,
}

pub struct PrivStruct {
    a: usize,
    b: usize,
}

pub enum Enum {
    A, B, C,
    D {
        a: u8,
        b: u8
    }
}

pub enum LargeEnum {
    A, B, C, D, E, F, G, H, I, J
}


pub trait Trait {
    type A;
    #[must_use]
    fn foo();
    fn bar();
}

pub trait GinormousTrait {
    type A;
    type B;
    type C;
    type D;
    type E;
    type F;
    const N: usize = 1;
    #[must_use]
    fn foo();
    fn bar();
}

pub trait HugeTrait {
    type A;
    const M: usize = 1;
    const N: usize = 1;
    const O: usize = 1;
    const P: usize = 1;
    const Q: usize = 1;
    #[must_use]
    fn foo();
    fn bar();
}

pub trait BigTrait {
    type A;
    #[must_use]
    fn foo();
    fn bar();
    fn baz();
    fn quux();
    fn frob();
    fn greeble();
}

#[macro_export]
macro_rules! foo {
    (a) => {a};
}

Fixes #82114

rust-highfive · 2021-03-21T03:26:09Z

A change occurred in the Ayu theme.

cc @Cldfire

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

Some changes occurred in HTML/CSS themes.

cc @GuillaumeGomez

rust-highfive · 2021-03-21T03:26:10Z

r? @ollie27

(rust-highfive has picked a reviewer for you, use r? to override)

Manishearth · 2021-03-21T03:26:25Z

r? @jyn514

src/librustdoc/html/static/rustdoc.css

camelid · 2021-03-21T04:05:24Z

src/librustdoc/html/render/print_item.rs

+            // We also do this if the types + consts is large because otherwise we could
+            // render a bunch of types and _then_ a bunch of consts just because both were
+            // _just_ under the limit
+            if !toggle & should_hide_fields(types.len() + consts.len()) {


Why are you using eager-evaluating & instead of && here?

Looks like this is still using &, and so is line 475.

The syntax typo is still here. ;)

src/librustdoc/html/render/print_item.rs

jyn514 · 2021-03-21T15:09:58Z

This needs a test, but you can see it rendered at https://manishearth.net/sand/doc_render/bar/

Could you publish a version of the same crate with a nightly rustdoc so I can compare the two?

jyn514

I left a couple comments on the implementation - I'm not super familiar with how the JS frontend works.

If you have time, it would be really helpful to see examples of docs that change before and after for each commit - "enum" and "struct" are clear enough, but I'm not sure what "Stop hiding entire item declarations" does. I know you made a whole site with examples, I'm just not sure which examples have changed.

src/librustdoc/html/render/print_item.rs

src/librustdoc/html/static/rustdoc.css

src/librustdoc/html/render/print_item.rs

Manishearth · 2021-03-21T21:45:40Z

Not at home right now, but you can just copy the decls and run rustdoc :)

"Stop hiding item declarations" turns off the thing where we hide entire item decls at once

camelid · 2021-03-21T21:58:52Z

I think this will need an FCP since it's a fairly significant insta-stable behavioral change. @rustbot label needs-fcp

camelid · 2021-03-21T21:59:47Z

It seems like there's an extra blank line after the toggle that disappears when you click the toggle?

camelid · 2021-03-21T22:00:36Z

It seems a bit redundant to show the declaration in this case:

I think we should maybe hide struct fields by default?

camelid · 2021-03-21T22:01:48Z

(Sorry for all the comments, I'm posting comments as I look through the rendered output.)

Rendering glitch for the attributes, not sure if it was pre-existing or not:

camelid · 2021-03-21T22:02:35Z

It doesn't look like you have an example for a small union, just a big one. Could you add an example with a small union?

Manishearth · 2021-03-22T15:59:38Z

It seems like there's an extra blank line after the toggle that disappears when you click the toggle?

Yeah some of this ties in to the styling for toggles being extremely haphazard. I tried fixing this and ended up breaking other things. I can try again.

I think we should maybe hide struct fields by default?

Maybe! I think we definitely should be applying the complex heuristic for traits, but hidning enums/structs by default seems okay. I personally don't like it when small types get hidden.

Could you add an example with a small union?

It's the same as a struct, but will do

Manishearth · 2021-03-22T16:02:18Z

Rendering glitch for the attributes, not sure if it was pre-existing or not:

Ugh no I had to introduce this to make it so that "expand attributes" didn't overlap with "expand methods".

If someone can figure out the CSS for me that would be really helpful; I tried a bunch and couldn't get it to work well. @camelid are you interested in focusing on this part of the work?

Manishearth · 2021-03-22T16:11:57Z

@jyn514 I added the input code to the PR body

GuillaumeGomez · 2021-03-22T21:36:54Z

src/librustdoc/html/render/print_item.rs

+            let has_visible_fields = count_fields > 0;
+            let toggle = should_hide_fields(count_fields);
+            if toggle {
+                toggle_open(w, "fields");


Looking at the code, I'd feel much better if we handled it using a closure like it was suggested. It seems very easy to forget... :-/

I don't think it's possible to have a closure for more complex things like the trait. I could use a drop guard so that it ICEs instead, but that's about it.

What's preventing to have something like:

fn wrap_with_toggle<T, E, F: FnOnce() -> Result<T, E>>(f: F) -> Result<T, E> { // toggle_open code let ret = f()?; // toggle_close code ret }

? I might miss something obvious here. :-/

Look at the trait code; it calls toggle_open at three different points, and toggle_close once.

Again, I tried this, it was too ugly and even more confusing, so I decided not to. I don't think this is too bad. We could add a destructor bomb but honestly we're using this a couple times and shouldn't have to yet. I'd rather get the entire toggle stuff revamped first.

GuillaumeGomez · 2021-03-22T21:39:02Z

src/librustdoc/html/static/rustdoc.css

+
+	/* The click event for this is defined on the document,
+	   so bubbling does not work. See https://github.com/rust-lang/rust/issues/83332 */
+	z-index: 10;


Questions about this:

Does it go above the menu on mobile when the burger button has been pressed?

Does it go above the help popup (which appears after "?" has been pressed)?

bors · 2021-04-16T11:51:53Z

📌 Commit 55b2944 has been approved by GuillaumeGomez

Rollup of 4 pull requests Successful merges: - rust-lang#83337 (rustdoc: Hide item contents, not items) - rust-lang#83944 (Fix a couple resolve bugs from binder refactor) - rust-lang#84145 (Address comments for vecdeque_binary_search rust-lang#78021) - rust-lang#84172 (Compiler error messages: reduce assertiveness of message E0384) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

bors · 2021-04-16T16:36:40Z

⌛ Testing commit 55b2944 with merge 2faef12...

…llaumeGomez rustdoc: Reduce visual weight of attributes. Followup from rust-lang#83337. As part of that PR, we stopped hiding attributes behind a toggle, because most things have just zero or one attributes. However, this made clear that the current rendering of attributes emphasizes them a lot, which distracts from function signatures. This PR changes their color of attributes to be the same as the toggles, and reduces their font weight. This also removes `#[lang]` from the list of ALLOWED_ATTRIBUTES. This attribute is an implementation detail rather than part of the public-facing documentation. ![image](https://user-images.githubusercontent.com/220205/115131061-cc407d80-9fa9-11eb-9a77-ad3f3217f391.png) Demo at https://hoffman-andrews.com/rust/de-emph-attr/std/string/struct.String.html#method.trim

…aumeGomez rustdoc: Reduce visual weight of attributes. Followup from rust-lang#83337. As part of that PR, we stopped hiding attributes behind a toggle, because most things have just zero or one attributes. However, this made clear that the current rendering of attributes emphasizes them a lot, which distracts from function signatures. This PR changes their color of attributes to be the same as the toggles, and reduces their font weight. This also removes `#[lang]` from the list of ALLOWED_ATTRIBUTES. This attribute is an implementation detail rather than part of the public-facing documentation. ![image](https://user-images.githubusercontent.com/220205/115131061-cc407d80-9fa9-11eb-9a77-ad3f3217f391.png) Demo at https://hoffman-andrews.com/rust/de-emph-attr/std/string/struct.String.html#method.trim

…earth,Nemo157,GuillaumeGomez Use details tag for trait implementors. Part of rust-lang#83332 and following on from rust-lang#83337 and rust-lang#83355. This removes one category of JS-generated toggles (implementors), and replaces them with a `<details>` tag. This simplifies the JS, and fixes some bugs where things that were supposed to be hidden by the toggle were not hidden. Compare https://hoffman-andrews.com/rust/details-implementors/std/io/trait.Read.html#impl-Read vs https://doc.rust-lang.org/nightly/std/io/trait.Read.html#implementors. This introduces a `left: -23px` to put the toggle in the correct place, matching the current style for `.collapse-toggle`. It's worth noting this introduces a slight behavior change: since the entire line is now a `<summary>`, any part of the line is clickable. So for instance, in `impl Read for File`, clicking `impl` or `for` will collapse / expand the docs. Clicking `Read` or `File` still links to the appropriate documentation as before.

…laumeGomez rustdoc: Convert sub-variant toggle to HTML Instead of creating a JS toggle, this injects details/summary for sub-variants of enums. This also fixes the CSS so that the toggle button does not jump when expanding/collapsing. Takes inspiration from rust-lang#83337 and should be considered part of rust-lang#83332. Not quite sure if the `.sub-variant` selectors could be further simplified? AFAICS it is only used in that place, and that does not seem to allow any recursion.

…earth,Nemo157,GuillaumeGomez Use details tag for trait implementors. Part of rust-lang#83332 and following on from rust-lang#83337 and rust-lang#83355. This removes one category of JS-generated toggles (implementors), and replaces them with a `<details>` tag. This simplifies the JS, and fixes some bugs where things that were supposed to be hidden by the toggle were not hidden. Compare https://hoffman-andrews.com/rust/details-implementors/std/io/trait.Read.html#impl-Read vs https://doc.rust-lang.org/nightly/std/io/trait.Read.html#implementors. This introduces a `left: -23px` to put the toggle in the correct place, matching the current style for `.collapse-toggle`. It's worth noting this introduces a slight behavior change: since the entire line is now a `<summary>`, any part of the line is clickable. So for instance, in `impl Read for File`, clicking `impl` or `for` will collapse / expand the docs. Clicking `Read` or `File` still links to the appropriate documentation as before.

Manishearth requested a review from jyn514 March 21, 2021 03:26

rust-highfive assigned ollie27 Mar 21, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 21, 2021

rust-highfive assigned jyn514 and unassigned ollie27 Mar 21, 2021

Manishearth mentioned this pull request Mar 21, 2021

[rustdoc] Have a more nuanced take on what is hidden by default #82114

Closed

This comment has been minimized.

Sign in to view

camelid reviewed Mar 21, 2021

View reviewed changes

src/librustdoc/html/static/rustdoc.css Outdated Show resolved Hide resolved

camelid reviewed Mar 21, 2021

View reviewed changes

camelid added A-rustdoc-ui Area: Rustdoc UI (generated HTML) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 21, 2021

jyn514 reviewed Mar 21, 2021

View reviewed changes

Manishearth mentioned this pull request Mar 21, 2021

[WIP] Start transforming documentation toggles into pure css ones #83355

Closed

rustbot added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Mar 21, 2021

This comment has been minimized.

Sign in to view

GuillaumeGomez reviewed Mar 22, 2021

View reviewed changes

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. labels Apr 16, 2021

Dylan-DPC-zz mentioned this pull request Apr 16, 2021

Rollup of 4 pull requests #84241

Merged

bors merged commit a5c68d7 into rust-lang:master Apr 16, 2021

rustbot added this to the 1.53.0 milestone Apr 16, 2021

jsha mentioned this pull request Apr 18, 2021

rustdoc: Reduce visual weight of attributes. #84283

Merged

jsha mentioned this pull request Apr 18, 2021

Use details tag for trait implementors. #84320

Merged

Swatinem mentioned this pull request Apr 18, 2021

rustdoc: Convert sub-variant toggle to HTML #84321

Merged

jsha mentioned this pull request Apr 19, 2021

Clean up toggle logic in rustdoc #83332

Closed

jsha mentioned this pull request Apr 22, 2021

rustdoc: "Show declaration" causes layout shift during page load #83075

Closed

jsha mentioned this pull request May 20, 2021

rustdoc: when hiding item contents, include a count #85494

Closed

jsha mentioned this pull request Jun 3, 2021

rustdoc: Remove the brief of structs and traits #39952

Closed

jsha mentioned this pull request Jun 26, 2021

Hiding of struct declarations means constructability isn't really documented #51860

Closed

weihanglo mentioned this pull request Jul 10, 2021

rustdoc: show count of item contents when hidden #87024

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: Hide item contents, not items #83337

rustdoc: Hide item contents, not items #83337

Manishearth commented Mar 21, 2021 •

edited

Loading

rust-highfive commented Mar 21, 2021

rust-highfive commented Mar 21, 2021

Manishearth commented Mar 21, 2021

This comment has been minimized.

camelid Mar 21, 2021

Manishearth Mar 21, 2021

jsha Mar 28, 2021

GuillaumeGomez Apr 12, 2021

jyn514 commented Mar 21, 2021

jyn514 left a comment

Manishearth commented Mar 21, 2021

camelid commented Mar 21, 2021

camelid commented Mar 21, 2021

camelid commented Mar 21, 2021

camelid commented Mar 21, 2021

camelid commented Mar 21, 2021

Manishearth commented Mar 22, 2021 •

edited

Loading

Manishearth commented Mar 22, 2021

Manishearth commented Mar 22, 2021

This comment has been minimized.

GuillaumeGomez Mar 22, 2021

Manishearth Mar 22, 2021

GuillaumeGomez Mar 22, 2021

Manishearth Mar 23, 2021

GuillaumeGomez Mar 22, 2021

bors commented Apr 16, 2021

bors commented Apr 16, 2021

rustdoc: Hide item contents, not items #83337

rustdoc: Hide item contents, not items #83337

Conversation

Manishearth commented Mar 21, 2021 • edited Loading

rust-highfive commented Mar 21, 2021

rust-highfive commented Mar 21, 2021

Manishearth commented Mar 21, 2021

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyn514 commented Mar 21, 2021

jyn514 left a comment

Choose a reason for hiding this comment

Manishearth commented Mar 21, 2021

camelid commented Mar 21, 2021

camelid commented Mar 21, 2021

camelid commented Mar 21, 2021

camelid commented Mar 21, 2021

camelid commented Mar 21, 2021

Manishearth commented Mar 22, 2021 • edited Loading

Manishearth commented Mar 22, 2021

Manishearth commented Mar 22, 2021

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Apr 16, 2021

bors commented Apr 16, 2021

Manishearth commented Mar 21, 2021 •

edited

Loading

Manishearth commented Mar 22, 2021 •

edited

Loading