rustdoc: remove redundant toggles for "undocumented items"

[jsha]

Rustdoc has some code to hide "undocumented items." As far as I can tell, that applies to trait implementors (on trait pages) and trait implementations (on struct pages). However, there's already an outer layer of toggles for both the implementors and implementations. For instance, this is what the "implementations" section looks like for the Read trait, if your settings expand implementations by default:

---

---

The toggle for each implementation is redundant with the inner toggle for the undocumented items. I think we should remove either the outer toggle or the inner. I'm not sure which is better. One argument in favor of removing the outer toggles: If someone has gone to the trouble of documenting a trait implementation, we should try to display that by default. For instance, here's what the implementations section looks like for String, by default:

---

---

However, if you know to expand it, there's actually some really nice documentation there:

---

---

There's a similar situation in rustls: Its Read implementation has important documentation on how to use it properly, but you wouldn't know that documentation was there because it's toggled closed by default.

[jsha]

Also worth noting that the "Show hidden undocumented items" don't respond to the "show all / collapse all" toggle in the upper right, and don't have a corresponding settings.html entry to show or hide them by default.

Part of #83332

[GuillaumeGomez]

Done in #84754. Don't hesitate to re-open if I missed something but from what I read, it's now done.

[jsha]

In #84754, we migrated the existing toggles to <details> But what I'd like to consider is either removing the toggles for "undocumented items", or removing the toggles for "implementors," or otherwise finding some improvement so we don't have two levels of toggles when it's generally not necessary.

[jsha]

I think we should split this into two cases:

Implementations on struct and enum pages

Example (default view)

Example (toggles expanded)

This demonstrates that the wording "Show hidden undocumented items" is incorrect - these items actually are documented, on the trait itself. That documentation gets copied to the struct or enum page. We're paying the cost in bytes and DOM size for these docs; we should show them by default. And that default can be modified by the "Auto-hide trait implementations" setting.

Or, another way of looking at it: If you are looking at some code that calls a method on ErrorKind, and you want to know what that method does, you'll go to the ErrorKind page and Ctrl-F for it. You should be able to find the method regardless of whether it was implemented as part of a trait or not. This is a use case I definitely had early in my Rust journey, and still occasionally do.

Implementors on trait pages

Example (default view)

Example (toggles expanded)

When you're on a trait page, the thing that's most useful to know is that type T implements the trait. The "undocumented methods" really are repetitive on a trait page and should be hidden. But I think, more than hidden, they should actually be entirely unrendered, since they duplicate information already on the page in the trait definition.

There are two things that should potentially be shown under each implementation in the Implementors section of a trait page:

• Methods that have documentation (but this probably belongs on the docs for the implementing type, not on the trait page)
• Whether the type implements any of the optional methods of the trait.

Proposal

I propose to remove the toggle for "undocumented items" on struct and enum pages, making the contents always visible. I propose to remove the method documentation for Implementors on trait pages, leaving that information to the implementor's own doc page.

[GuillaumeGomez]

That would simplify the code a bit. I don't have a strong opinion on this topic though.

What do you think @rust-lang/rustdoc?

[jyn514]

I really like both those ideas :) although maybe it makes sense to collapse the method descriptions by default on type pages, since they're always the same and can be long? I don't feel strongly about that though.

[Nemo157]

One thing I'm unsure about is how this will feel on pages like Box that have all the traits.

I somewhat commonly skim through the trait list looking for specific traits I care about, but I could just turn "Auto-hide trait implementation documentation" back on (sidenote, it appears to be broken on nightly).

[GuillaumeGomez]

Please open an issue for the broken part. ;)

[Manishearth]

I propose to remove the toggle for "undocumented items" on struct and enum pages, making the contents always visible.

Strong +1

I propose to remove the method documentation for Implementors on trait pages, leaving that information to the implementor's own doc page.

Should we show it only if there are explicit method docs on the implementors?

[GuillaumeGomez]

Should we show it only if there are explicit method docs on the implementors?

Currently it's kinda done, but badly. If there is no doc on the implementor method, we put it under "undocumented item" (which is really not clear). So any improvement would be nice.

I think that if there is implementor doc, we should show it, otherwise just display something like "read more" and that's it.

[jsha]

One thing I'm unsure about is how this will feel on pages like Box that have all the traits.

@Nemo157 this is a good point. As an example, Box has 5 implementations of Default, for various specializations. Here they are with everything fully expanded:

This can be repetitive, and also potentially confusing for a new user. If I see default being called, which of these is it?

One potential approach would be to "group" these implementations. So we would have one sub-heading for the Default trait, which would list all the implementations of Default for this type, and then all the methods. Something like (forgive the lack of formatting):

Default

impl<T> Default for Box<[T], Global>
impl Default for Box<str, Global>
impl<T> Default for Box<T, Global>
impl Default for Box<CStr>
impl Default for Box<OsStr>

fn default() -> Box<OsStr>
  Returns the “default value” for a type. Read more

I think that if there is implementor doc, we should show it, otherwise just display something like "read more" and that's it.

I think to keep things simpler, on trait Foo's page we should only show comments that are set on the impl Foo for Bar of a struct or enum. Comments on individual methods within impl Foo for Bar can be found on Bar's page on don't need be on Foo's page. This is simpler for the doc author and the doc reader to reason about, and as a side benefit I suspect it will be a little easier for us to implement as well.

[GuillaumeGomez]

I think to keep things simpler, on trait Foo's page we should only show comments that are set on the impl Foo for Bar of a struct or enum. Comments on individual methods within impl Foo for Bar can be found on Bar's page on don't need be on Foo's page. This is simpler for the doc author and the doc reader to reason about, and as a side benefit I suspect it will be a little easier for us to implement as well.

Not sure to understand your comment so instead, code:

trait Foo {
    /// default doc
    fn default_fn();
    fn another_one();
}

struct Bar;

impl Foo for Bar {
    fn default_fn() {}
    /// item doc!
    fn another_one() {}
}

So on Bar page, only show show the doc of another_one, default_fn will just have "read more". Is it what you meant?

[jsha]

On Foo's page: Show only impl Foo for Bar, no methods.

On Bar's page: Show default_fn with default doc and another_one with item doc!.

[GuillaumeGomez]

On Foo's page: Show only impl Foo for Bar, no methods.

Agreed, that'd be duplicated (I actually thought we already did but I guess not so nice idea!).

On Bar's page: Show default_fn with default doc and another_one with item doc!.

However I'm not sure to agree on that point: that'd make a lot of duplicated content in all pages...

[jsha]

On Bar's page: Show default_fn with default doc and another_one with item doc!.

However I'm not sure to agree on that point: that'd make a lot of duplicated content in all pages...

This is how we currently do it. For example https://doc.rust-lang.org/nightly/std/fs/struct.File.html#impl-Read:

So I was proposing this way to be conservative with regards to what we currently do. You're right that there's a lot of duplicated content across many pages!

Is your proposal that we would show no documentation for Read::read on File's page? Or just the method signature but not copy the doccomment from Read::read onto File's page?

[GuillaumeGomez]

Currently we display the first line/paragraph on "default docs". I'm not sure it's very useful so I would only keep the "read more" part.

Is your proposal that we would show no documentation for Read::read on File's page? Or just the method signature but not copy the doccomment from Read::read onto File's page?

The second one: it would show the method signature (very important to keep that part!) but no documentation unless it's this implementation which added it.

[jsha]

The second one: it would show the method signature (very important to keep that part!) but no documentation unless it's this implementation which added it.

Great. I like this approach.

[jyn514]

no documentation unless it's this implementation which added it.

Hmm, that seems like it might confuse people into thinking the trait method has no description - maybe instead we could collapse the description by default?

[GuillaumeGomez]

That's what we currently do. :p Also, there will still be a "read more" link.

[jyn514]

(also, I'm not fully following all the proposed changes, maybe someone could write up the current proposal in the issue description? Presumably it will be in the PR too which seems fine)

[GuillaumeGomez]

I *think* we just agreed on all details, so I guess we'll have the sum up written shortly.

[jsha]

Current proposal, which I think should be a few separate PRs:

1. On struct and enum pages: remove the toggle for "undocumented items," making the contents always visible.
2. On struct and enum pages: group implementations by the trait implemented. This reduces duplication when the struct or enum implements the same trait for various different specializations
3. On struct and enum pages: remove the doccomment on trait methods, if that doccomment was copied from the trait. Keep the doccomment if it was written on the struct or enum's implementation of that method.
4. On trait pages: remove the method documentation for Implementors on trait pages, leaving that information to the implementor's own doc page.

[Nemo157]

1. On struct and enum pages: remove the toggle for "undocumented items," making the contents always visible.

But note that the default setting of "Auto-hide trait implementation documentation" is on, so you still won't see this content by default.

[jyn514]

On struct and enum pages: remove the doccomment on trait methods, if that doccomment was copied from the trait. Keep the doccomment if it was written on the struct or enum's implementation of that method.

Ok yes, this still seems confusing to me: #84326 (comment)

[jsha]

no documentation unless it's this implementation which added it.

Hmm, that seems like it might confuse people into thinking the trait method has no description - maybe instead we could collapse the description by default?

I'd like to avoid adding more things that we collapse by default. It adds page weight for stuff that is usually never seen, and it feels like a way of not addressing head-on the questions of what information needs to be visible where.

We could add a link for those trait methods to the trait page, e.g. "Read documentation at Foo."

I'm also not particularly opposed to keeping doccomments that came from the trait (as we currently do).

[GuillaumeGomez]

I'm also not particularly opposed to keeping doccomments that came from the trait (as we currently do).

We don't, we just have the first line. ;)

We could add a link for those trait methods to the trait page, e.g. "Read documentation at Foo."

As long as the trait's items are listed, I'm ok with it.

2. On struct and enum pages: group implementations by the trait implemented. This reduces duplication when the struct or enum implements the same trait for various different specializations

I'm not too sure about this. It's actually quite useful in some cases like wrapping types where the generic wrapped type can implement (or not) some traits based on what they implement themselves.

On trait pages: remove the method documentation for Implementors on trait pages, leaving that information to the implementor's own doc page.

Just listing the implementors and not their documentation sounds like a good idea!

[jyn514]

@jsha this can be closed now that #85970 is merged, right?

[jsha]

Yes. Note that these two items are unimplemented:

On struct and enum pages: group implementations by the trait implemented. This reduces duplication when the struct or enum implements the same trait for various different specializations
On struct and enum pages: remove the doccomment on trait methods, if that doccomment was copied from the trait. Keep the doccomment if it was written on the struct or enum's implementation of that method.

But it seems from subsequent comments that these were not particularly popular, so I'm going to close this without doing them.

[jyn514]

Oh hmm I really liked the idea of grouping by trait, I forgot that was suggested.

I'm not too sure about this. It's actually quite useful in some cases like wrapping types where the generic wrapped type can implement (or not) some traits based on what they implement themselves.

@GuillaumeGomez I'm not sure what you mean by this. Rustdoc will still show just as many impl blocks and they'll have generic parameters, it will just be grouped by the trait.

[GuillaumeGomez]

Then all good. I thought it meant having one impl displayed if the same trait was implemented multiple times.

[cynecx]

On trait pages: remove the method documentation for Implementors on trait pages, leaving that information to the implementor's own doc page.

Instead of completely removing the methods from the implementors pages. Can we just show their signature without docs? That would certainly help with navigation and seeing what kind of methods are implemented (excluding default methods), the same way it does with the associate types (which #88490 adds back).

@GuillaumeGomez, @jyn514 ?

cc #88490

[joseluis]

I was searching a way to remove the "read more" link from the doc-comments in the automatically implemented methods and constants from the trait, and be able to show instead the full doc-comment. But it seems there's not currently a way to customize this behaviour, right?

For certain APIS, specially those having a lot of default methods, I find it very inconvenient to have to navigate back and forth between the trait page documentation and the implementation type documentation in order to read more than 1 line of information for many of the methods, and it would be a much better experience if the complete doc-comment was shown for every method in the type page, independently of whether it's a default implementation or not.

[GuillaumeGomez]

We removed the complete doc comment and replaced it with "read more" because the size of some trait pages was just completely out of control.

[joseluis]

We removed the complete doc comment and replaced it with "read more" because the size of some trait pages was just completely out of control.

I see.

Fortunately I could make a workaround for my usecase, with a macro that duplicates the items in both the trait definition and the implementation. This way the doc-comments can be seen fully both in the trait and in the type doc pages:

macro_rules! impl_api {
    ($type:ident, $trait:ident, $($i:item),*) => {
        #[doc = concat!("Associates methods and constants to the [`", stringify!($type), "`] type.")]
        pub trait $trait {
            $($i)*
        }
        impl $trait for $type {
            $($i)*
        }
    };
}

// usage:
struct MyType;
impl_api![MyType, MyTrait,
    /// `BAR` is 0
    const BAR: u8 = 0;,
    /// `foo` returns 1
    fn foo() -> u8 { 1 }
];