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

[Manishearth]

https://twitter.com/mcclure111/status/1361025145125634061

By default, we:

• hide attributes
• hide structs
• hide unions
• hide traits
• hide trait implementation
• hide implementor list

I feel like traits probably should be shown by default, since everything on the trait is important, especially for implementors.

Furthermore, it might be worth splitting "structs" into "structs with all-public fields" and "structs with private fields" for the same reason (with unit structs being treated as having all public). For the former kind the actual struct declaration is super useful, for the latter kind it's not at all.

cc @rust-lang/rustdoc

[GuillaumeGomez]

The traits were the original reason why we added the possibility to change the settings. As far as I can tell, it's the same methods' header, but maybe I missed something? If not, I'm not sure to see how it would improve anything. As for the rest, why not!

[camelid]

Another way to solve the problem: #81977

[mcclure]

Hi, I'm the person linked on Twitter-- my perspective is that I am learning Rust and the first time I looked at
https://doc.rust-lang.org/std/convert/trait.Into.html
I was very confused because I was scrolling down the page trying to find the type, and once I found the type there was a "T" but I could not see where T was defined/bound. I had scrolled right past the light gray
[+] Show declaration
without even taking note that there was a clickable thing there that I was failing to click on.
Then I asked for help and one person explained it to me.
Now I know to expect that docs.rs pages have light gray [+] expandables, and I probably won't make this mistake again.
But on my very first visit, I did get confused until I asked for help.
I don't know if something needs to be fixed here or what the right fix is.

[GuillaumeGomez]

@camelid: Oh wow. thats so simple yet so nice! What do you think of it @mcclure?

[mcclure]

Honestly yes I think @camelid 's suggestion would help. Since the name up top looks like a "header" for the entire document, it is natural that we should expect "T" to be reused again and again in the same way throughout the same page if it is in that "header" name.

But it is not so obvious to me that if in a hideable (hidden by default...) section at the top you define a T within a { bracket } section you then close again, that every T throughout the page should be expected to be that T from the { brackets } at the top (if that way of putting it makes sense).

[jyn514]

I would rather show the trait definition by default than show the generic parameters. Showing the parameters won't help much for associated items, and could get overwhelming if there are a lot of parameters.

[GuillaumeGomez]

Sorry, I'm not sure to fully understand what you mean. In #81977, it's suggested that we add the generics in the "title" but also in the fields/variants.

I thought that your issue was that currently, those generics were only visible in the hidden section. If so, wouldn't #81977 fix this problem? If I'm completely off the plate, more explanations of your problem are welcome. :)

[GuillaumeGomez]

I would rather show the trait definition by default than show the generic parameters. Showing the parameters won't help much for associated items, and could get overwhelming if there are a lot of parameters.

That is a good point too. If possible, I'd prefer to not have the trait definition displayed by default. Just look at Iterator, it's (in my opinion) far too big to be really useful. As a reminder: we hid the traits' definition by default in the first place because people complained about it.

[jyn514]

As a reminder: we hid the traits' definition by default in the first place because people complained about it.

Do you have a link to that discussion? It was before I joined the team.

[GuillaumeGomez]

Time for speleology!

[Manishearth]

That is a good point too. If possible, I'd prefer to not have the trait definition displayed by default. Just look at Iterator, it's (in my opinion) far too big to be really useful. As a reminder: we hid the traits' definition by default in the first place because people complained about it.

This is kinda what I'm getting at with "nuanced take". What did people actually complain about, trait definitions or Iterator? I think short traits like Into/etc lose out by being hidden, but yes, giant traits like Iterator do not. There's a similar nuanced take to be had on Options-style structs with public fields vs structs with private fields.

I actually think our current situation with a bunch of prefs for kinds of items is suboptimal, I think we should have the prefs, but perhaps have better takes on what the defaults should be, takes that are not just based on the types of items.

[GuillaumeGomez]

That seems reasonable indeed. But on big items, wouldn't the missing generics information still be an issue?

[jyn514]

What about still showing the definition, but skipping associated items if there are more than e.g. 3 of them? Remove the option for toggling the whole definition and allow toggling the associated items instead.

[Manishearth]

@GuillaumeGomez i think generics can be solved with @camelid's proposal

@jyn514 something like that ,yeah. we should figure out exactly where the configurability should lie, but i'll point out: not everything needs to be completely configurable

[porglezomp]

It might also be nice to use thresholds on required vs optional parts, so for instance Iterator could by default look like:

pub trait Iterator {
    type Item;
    pub fn next(&mut self) -> Option<Self::Item>;
    ... optional methods ...
}

and have the "expand to full" show all the optional/provided methods?

[GuillaumeGomez]

I think the discussion was mostly offline based on the comments from #49412... Do you remember it maybe @Manishearth?

@GuillaumeGomez i think generics can be solved with @camelid's proposal

@jyn514 pointed out that it might be an issue in case there are a lot of generics, and I think it's a good point that we need to examine.

@jyn514 something like that ,yeah. we should figure out exactly where the configurability should lie, but i'll point out: not everything needs to be completely configurable

Agreed.

@porglezomp This is a bit ahead of the conversation but yes, something like that could be nice.

[Manishearth]

@porglezomp oh that's clever, yeah. Though.... optional/provided only really matters for implementors, right? Might be confusing if the optional method is what people are primarily expected to use.

It's worth drilling down to see what different people need to see.

Like, as a user of iterator, I absolutely don't care about next(). Implementors are rarer, and usually can expand.

But there are smaller traits like Into/etc where not showing the definition seems counterproductive.

I think switching to collapsing based on size rather than type is probably the core thing we need to do here.

[Walther]

Possibility to consider: "This definition is large and has been collapsed. Click here to expand" with some heuristic for the size. Might not be perfect, but could be 1. improvement over the status quo 2. a more flexible solution than trying to decide based on type
EDIT: jinx'd by Manish above 😂

[Manishearth]

Actually, I think I haven't been thinking enough out of the box here.

Why do we need to collapse the entire declaration in the first place?

This seems more useful as a default for everything, structs, etc. Always show the declaration, hide public fields if there are too many, hide methods if there are too many, hide methods and assoc items if you really really have to.

It's way more obvious that there's an expandable thing here, as opposed to the current state where if you don't notice the + you won't be able to see the impl.

[camelid]

Instead of showing everything except provided methods, we could hide all associated functions and only show associated types and constants since there are usually only a few.

EDIT: Manish and I had the same idea at the same time 😆

[camelid]

Showing the parameters won't help much for associated items

Isn't there already a section for associated types and constants?

and could get overwhelming if there are a lot of parameters.

Yeah, that is one limitation. Another alternative is to have a section at the top of the page with all the generics. But if we're going that route, just showing a trimmed-down definition would probably still be better.

[GuillaumeGomez]

This seems more useful as a default for everything, structs, etc. Always show the declaration, hide public fields if there are too many, hide methods if there are too many, hide methods and assoc items if you really really have to.

It's way more obvious that there's an expandable thing here, as opposed to the current state where if you don't notice the + you won't be able to see the impl.

Only the traits have methods. The other types only have fields/variants, and in those cases, we have them with their documentation just under, so if we go this road, we might need to think about maybe updating that part too. Ideas are welcome! :)

[Manishearth]

Only the traits have methods. The other types only have fields/variants, and in those cases, we have them with their documentation just under, so if we go this road, we might need to think about maybe updating that part too. Ideas are welcome! :)

Right, uh, that's what I'm saying, we should be applying this logic to everything. To be more explicit:

• Always display the struct Foo<T> { etc part of the item declaration, we only collapse contents, as in my comment above
• Enums: display all variants unless there's a really large number of them (>50?)
• Structs:
  • Display public fields, collapse if there are >10 or so.
  • Display the /* private fields */ if it's all private
  • If it's a unit struct, display the semicolon.
• Traits:
  • Show associated types as much as possible, only cut them if there are a lot of associated types.
  • If there are a lot of associated items, collapse the methods and associated constants. If there are still a lot of assoc types left, collapse everything
  • When collapsing it should all collapse into a single thing, users shouldn't have to pick out of a "show associated types" "show methods" etc option.
• Unions: I don't really care about this one, probably do the same as structs
• Consts: shrug

[jyn514]

Unions: I don't really care about this one, probably do the same as structs

I would treat unions the same as enums.

Consts: shrug

Right now we show the pretty-printed initializer expression, I haven't heard complaints about it.

[Nemo157]

Enums: display all variants unless there's a really large number of them (>50?)

What about enums with few variants, but many fields in each variant?

I would treat unions the same as enums.

Unions are much closer to structs than enums, they have typed members instead of variants (each of which can be a struct-like-variant).

[mcclure]

A couple of thoughts—

I recognize that you've spun off into some kind of other discussion that isn't specifically about my experience getting confused by the docs. But if my beginner's perspective is helpful—

• I wouldn't say the problem I had was really a problem caused by information being hidden. More so, I'd say the problem I had was information was hidden and I did not know information was hidden, because the emphasis in the design of the "collapsed section" widget was on making it unobtrusive rather than making it unambiguous as an interactive element.
• The specific reason I would consider putting <T> in the class title helpful is because T creates a binding (not just in the rust sense but in the sense "T" is referred to throughout the english text of the page). Things like variants, enums, fields do not necessarily have this characteristic— I don't think you need to know the union fields in order to interpret the text of the page.
• One thing that strikes me as a little weird, looking at for example the Into page, is that what seems to me to be "type information" is in two different places. When I visited that page, my goal was just to figure out what Into's type is, what it is "defined as", and my "idiot in a hurry" approach to doing so was to scroll down until I saw something that looked like type declarations. The only thing that looks like types is the "Required methods" and "implementors" sections. If the declaration had been bunched together with those sections— IE if it had gone [Show Declaration], Required Methods, Implementors— maybe when I scrolled down and didn't see what I was looking for (definition of T) I would have been more likely to notice the Show Declaration right next to it rather than up at the top above the summary. (Again I'm commenting here only on the "rust newbie visiting the page for the first time" experience.)

[Manishearth]

Thanks for the feedback!

I recognize that you've spun off into some kind of other discussion that isn't specifically about my experience getting confused by the docs. But if my beginner's perspective is helpful—

Yeah I'd explicitly started this discussion as a "we need to solve this problem in general" with your specific example as a motivator but not the focus

I wouldn't say the problem I had was really a problem caused by information being hidden. More so, I'd say the problem I had was information was hidden and I did not know information was hidden, because the emphasis in the design of the "collapsed section" widget was on making it unobtrusive rather than making it unambiguous as an interactive element.

This is the problem I'm hoping to solve by making the type declaration always be shown and have an explicit "expand this" thing:

The specific reason I would consider putting in the class title helpful is because T creates a binding (not just in the rust sense but in the sense "T" is referred to throughout the english text of the page). Things like variants, enums, fields do not necessarily have this characteristic— I don't think you need to know the union fields in order to interpret the text of the page.

So we're discussing that in #81977, but I'll point out that by making the type decl be always shown this problem is solved too. Iterator in my screenshot above isn't a parametrized trait, but you would see pub trait Into<T> { .. for Into

When I visited that page, my goal was just to figure out what Into's type is, what it is "defined as", and my "idiot in a hurry" approach to doing so was to scroll down until I saw something that looked like type declarations. T

Yeah this is why I don't want to ever completely hide the type declaration on the top

[Manishearth]

PR up at #83337

[jsha]

Note that there's a lot of redundancy in showing the type declaration:

Trait foo_bar::Iterator

pub trait Iterator {
type Item;
[+] show all methods
}

In the second line, "trait" and "iterator" are duplicates of information that's already in the first line, and "pub" is implicit in the fact that this is documentation. Also the trailing } takes up a whole line. I think it would be nice to use this space efficiently since it's "above the fold" and the first part of documentation that people see.

I think it makes sense for the declaration to include the body of the item, but not the pub trait Iterator, which is already mostly represented in the header. This would require moving type parameters into the heading, as was suggested up-thread. So what I propose is like (assuming a parameterized Iterator)

Trait foo_bar::Iterator

type item;
[+] show all methods

[Manishearth]

@jsha I'm not really in favor of that, it does look like redundancy, but it's not just type parameters -- it's also associated types, and attributes and such. Special casing all of those seems like it would be brittle.

Furthermore, there's a lot of value in just showing the body as is, that's typically the most useful for me when I am quickly visiting an item.

This is why I want to introduce a threshold, so that items large enough to push textual docs below the fold get collapsed, but otherwise they don't, and the only part which gets collapsed is the contents. It feels the most intuitive to show the declaration as Rust-like code as opposed to scattering it everywhere.

I don't always think redundancy is bad. Title vs docblock seems pretty okay to me, and I don't think it's worth it to bend over backwards in order to get rid of the redundancy.

[camelid]

"pub" is implicit in the fact that this is documentation

Actually it's not implicit; some docs, such as the rustc API docs, use --document-private-items.

[jsha]

it's also associated types, and attributes and such. Special casing all of those seems like it would be brittle.

Associated types are inside the declaration, so would be shown without resorting to a special case. But yes, it's true about attributes. Worth noting, though, that the current experience for attributes is not good: Even if there's only one attribute, it gets hidden behind a toggle. Opening the toggle to see just one attribute makes me wonder "why was this hidden?"

It feels the most intuitive to show the declaration as Rust-like code as opposed to scattering it everywhere.

Note that we already have a place where the full, exact Rust code is shown: the [src] link. What about implementing "show more" as a link to the src?

[Manishearth]

Even if there's only one attribute, it gets hidden behind a toggle. Opening the toggle to see just one attribute makes me wonder "why was this hidden?"

I think this would be a good followup to my PR.

Currently rustdoc hides based on a binary of types of things, whereas IMO we should be doing based on what we think is worth hiding. Single attributes? No. Multiple? Maybe, picking some threshold. It's the same thing with my PR, I'm trying to move away from "hide structs" "hide enums" etc to "hide all kinds of things when they get too big".

Note that we already have a place where the full, exact Rust code is shown: the [src] link. What about implementing "show more" as a link to the src?

No, because it's more clicks: If I'm perusing the docs I'm going to want to check out types with a single click, not having to look for src (and src isn't that discoverable)

[jsha]

Even if there's only one attribute, it gets hidden behind a toggle. Opening the toggle to see just one attribute makes me wonder "why was this hidden?"
I think this would be a good followup to my PR.

Great! By the way, how common is it for there to be enough attributes that we would need to hide them? I'm working on the CSS for your PR right now, and I think it might simplify things if we could say "attributes are never hidden." In my limited experience, I've only ever seen a single attribute. I know there can be more, but it seems quite rare that there would be so many they need hiding.

Currently rustdoc hides based on a binary of types of things, whereas IMO we should be doing based on what we think is worth hiding

Definitely. And further on that theme: I think as a general rule, the toggles should not be nested (in the default set of open/closed settings). So if someone opens up a section to see detail, they should not have to open up additional sections within that section. This has the added benefit of reducing the DOM complexity somewhat. And it might wind up simplifying the CSS and JS a tiny amount.

What about implementing "show more" as a link to the src?
If I'm perusing the docs I'm going to want to check out types with a single click, not having to look for src (and src isn't that discoverable)

I was proposing that the docs would look like:

Trait foo_bar::Iterator

type item;
see all methods

In other words, a link rather than a toggle, so no additional clicks. But looking at the source I see a problem: the methods are all preceded by a long comment block, so it's hard to get a bird's-eye view of them. So consider this suggestion dropped.

[Manishearth]

Great! By the way, how common is it for there to be enough attributes that we would need to hide them?

Probably pretty rare! I'm okay with taking this approach for now.

Definitely. And further on that theme: I think as a general rule, the toggles should not be nested (in the default set of open/closed settings). So if someone opens up a section to see detail, they should not have to open up additional sections within that section. This has the added benefit of reducing the DOM complexity somewhat. And it might wind up simplifying the CSS and JS a tiny amount.

I think this makes sense in the case of attributes, but not as a general rule: for example we let you collapse impl blocks and also their individual contents and I think that's fine.

[camelid]

Great! By the way, how common is it for there to be enough attributes that we would need to hide them?

Probably pretty rare! I'm okay with taking this approach for now.

For traits it's probably relatively rare to have a lot of attributes on the associated items, but I think the standard library tends to have several attributes on structs, enums, enum variants, and fields. For example, Option looks like this:

#[derive(Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)]
#[rustc_diagnostic_item = "option_type"]
#[stable(feature = "rust1", since = "1.0.0")]
pub enum Option<T> {
    /// No value
    #[lang = "None"]
    #[stable(feature = "rust1", since = "1.0.0")]
    None,
    /// Some value `T`
    #[lang = "Some"]
    #[stable(feature = "rust1", since = "1.0.0")]
    Some(#[stable(feature = "rust1", since = "1.0.0")] T),
}

However, we don't currently show attributes for stuff in types, so the declaration looks like this:

If we ever start showing attributes for these, we might need more complex attribute-hiding behavior.

[jsha]

Interesting. Is there special logic for omitting the attributes on enums? If you look at e.g. https://doc.rust-lang.org/nightly/std/iter/trait.Iterator.html, and hit "Show declaration," it has the attributes.

[camelid]

Interesting. Is there special logic for hiding the attributes on enums? If you look at e.g. https://doc.rust-lang.org/nightly/std/iter/trait.Iterator.html, and hit "Show declaration," it has the attributes.

I feel like I saw Manish say somewhere that he was going to fix that?

[camelid]

It looks like we only render attributes for methods:

rust/src/librustdoc/html/render/mod.rs

Line 1043 in f811f14

render_attributes(w, meth, false);

Probably we just need to call render_attributes in more places.

[jsha]

Probably we just need to call render_attributes in more places.

Actually, it looks like there is a list of allowed attributes, and the attributes you point out on Option aren't in the list:

rust/src/librustdoc/html/render/mod.rs

Lines 1095 to 1103 in f811f14

	const ALLOWED_ATTRIBUTES: &[Symbol] = &[
	sym::export_name,
	sym::lang,
	sym::link_section,
	sym::must_use,
	sym::no_mangle,
	sym::repr,
	sym::non_exhaustive,
	];

[Manishearth]

However, we don't currently show attributes for stuff in types, so the declaration looks like this:

@camelid This has nothing to do with enums vs traits; we only show a small set of attributes. I think the only relevant attribute is non_exhaustive on structlike enum variants; but when I last tried we don't render that anyway