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.

Sign up for GitHub

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

Jump to bottom

rustdoc: it should be possible to leave "Methods From Deref" empty, as was previously the default behaviour #83133

Closed
slightlyoutofphase opened this issue Mar 14, 2021 · 7 comments · Fixed by #83826
Closed

rustdoc: it should be possible to leave "Methods From Deref" empty, as was previously the default behaviour #83133

slightlyoutofphase opened this issue Mar 14, 2021 · 7 comments · Fixed by #83826
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@slightlyoutofphase
Copy link
Contributor

slightlyoutofphase commented Mar 14, 2021 •
edited
Loading

For example, I would much rather have the documentation for my crate Staticvec look as it did here, as opposed to pulling in the docs for every single slice method as it does now.

Edit: Some additional thoughts:

  • even if this isn't possible for some reason, it would almost certainly be better to have the crate-native "Trait Implementations" section appear before the "Methods From Deref" section does, since "Trait Implementations" will pretty much always be describing things that are unique to the crate being documented and so actually directly relevant
  • a good idea might also be to have the "Methods From Deref" header amount to a clickable link that redirects to the actual primary docs for whatever is being derefed to in any case where it is being left empty, if that is still possible
@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Mar 15, 2021
@slightlyoutofphase
Copy link
Contributor Author

slightlyoutofphase commented Mar 21, 2021 •
edited
Loading

Are there any details on when precisely this changed, also? I've noticed that even though the auto-generated docs on docs.rs previously left the "Methods From Deref" section empty for user-submitted crates, it was seemingly always filled out for stuff like the official libstd docs for Vec, so presumably there has always been some kind of way of controlling rustdoc's behaviour in this regard, unless I'm missing something.

@camelid camelid added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Mar 21, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 30, 2021

Are there any details on when precisely this changed, also?

Yes, it was changed intentionally in #80653 and mentioned in the release notes: https://github.com/rust-lang/rust/blob/master/RELEASES.md#rustdoc.

I've noticed that even though the auto-generated docs on docs.rs previously left the "Methods From Deref" section empty for user-submitted crates, it was seemingly always filled out for stuff like the official libstd docs for Vec, so presumably there has always been some kind of way of controlling rustdoc's behaviour in this regard, unless I'm missing something.

Vec derefs directly to slice - maybe the user crates you're talking about always deref to Vec?

@jyn514
Copy link
Member

jyn514 commented Mar 30, 2021

even if this isn't possible for some reason, it would almost certainly be better to have the crate-native "Trait Implementations" section appear before the "Methods From Deref" section does, since "Trait Implementations" will pretty much always be describing things that are unique to the crate being documented and so actually directly relevant

👍 I like this idea.

a good idea might also be to have the "Methods From Deref" header amount to a clickable link that redirects to the actual primary docs for whatever is being derefed to in any case where it is being left empty, if that is still possible

Hmm, I'm a little concerned the methods will be harder to find that way. We get a lot of complaints that the "Trait Implementations" section is hard to read because you have to click on the name of the trait to see what methods are available.

@slightlyoutofphase
Copy link
Contributor Author

Hmm, I'm a little concerned the methods will be harder to find that way. We get a lot of complaints that the "Trait Implementations" section is hard to read because you have to click on the name of the trait to see what methods are available.

Yeah, that makes sense. Honestly just showing Trait Implementations before Methods From Deref in any case where both exist would likely pretty much cover the kind of thing I was mostly concerned about by itself anyways.

@jyn514
Copy link
Member

jyn514 commented Mar 31, 2021

Honestly just showing Trait Implementations before Methods From Deref in any case where both exist would likely pretty much cover the kind of thing I was mostly concerned about by itself anyways.

Are you interested in making a PR for that? I think it would be as simple as moving

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

Lines 1934 to 1940 in a5029ac

if let Some(impl_) = v
.iter()
.filter(|i| i.inner_impl().trait_.is_some())
.find(|i| i.inner_impl().trait_.def_id_full(cache) == cx.cache.deref_trait_did)
{
sidebar_deref_methods(cx, out, impl_, v);
}
after

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

Lines 2001 to 2007 in a5029ac

if !blanket_format.is_empty() {
out.push_str(
"<a class=\"sidebar-title\" href=\"#blanket-implementations\">\
Blanket Implementations</a>",
);
write_sidebar_links(out, blanket_format);
}
:)

@slightlyoutofphase
Copy link
Contributor Author

Sure, I'll take a look at it tomorrow.

@slightlyoutofphase slightlyoutofphase mentioned this issue Apr 3, 2021
Merged
@slightlyoutofphase
Copy link
Contributor Author

Got busy with some other stuff, but I've now opened a PR that make those changes, which do accomplish what you thought they would.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 4, 2021
@GuillaumeGomez
Rollup merge of rust-lang#83826 - slightlyoutofphase:rustdoc-sidebar-…
038cb1e 
…order-shuffle, r=jyn514

List trait impls before deref methods in doc's sidebar

This PR is acting directly on a suggestion made by `@jyn514` in rust-lang#83133. I've tested the changes locally, and can confirm that it does in fact properly achieve what he thought it would. This PR also in turn closes rust-lang#83133.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 4, 2021
@GuillaumeGomez
Rollup merge of rust-lang#83826 - slightlyoutofphase:rustdoc-sidebar-…
473b8e1 
…order-shuffle, r=jyn514

List trait impls before deref methods in doc's sidebar

This PR is acting directly on a suggestion made by ``@jyn514`` in rust-lang#83133. I've tested the changes locally, and can confirm that it does in fact properly achieve what he thought it would. This PR also in turn closes rust-lang#83133.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 4, 2021
@GuillaumeGomez
Rollup merge of rust-lang#83826 - slightlyoutofphase:rustdoc-sidebar-…
8e37df4 
…order-shuffle, r=jyn514

List trait impls before deref methods in doc's sidebar

This PR is acting directly on a suggestion made by ```@jyn514``` in rust-lang#83133. I've tested the changes locally, and can confirm that it does in fact properly achieve what he thought it would. This PR also in turn closes rust-lang#83133.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 5, 2021
@Dylan-DPC
Rollup merge of rust-lang#83826 - slightlyoutofphase:rustdoc-sidebar-…
b706a07 
…order-shuffle, r=jyn514

List trait impls before deref methods in doc's sidebar

This PR is acting directly on a suggestion made by ````@jyn514```` in rust-lang#83133. I've tested the changes locally, and can confirm that it does in fact properly achieve what he thought it would. This PR also in turn closes rust-lang#83133.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 5, 2021
@Dylan-DPC
Rollup merge of rust-lang#83826 - slightlyoutofphase:rustdoc-sidebar-…
14214df 
…order-shuffle, r=jyn514

List trait impls before deref methods in doc's sidebar

This PR is acting directly on a suggestion made by `````@jyn514````` in rust-lang#83133. I've tested the changes locally, and can confirm that it does in fact properly achieve what he thought it would. This PR also in turn closes rust-lang#83133.
jyn514 pushed a commit to jyn514/rust that referenced this issue Apr 5, 2021
Rollup merge of rust-lang#83826 - slightlyoutofphase:rustdoc-sidebar-…
1a0997b 
…order-shuffle, r=jyn514

List trait impls before deref methods in doc's sidebar

This PR is acting directly on a suggestion made by ``````@jyn514`````` in rust-lang#83133. I've tested the changes locally, and can confirm that it does in fact properly achieve what he thought it would. This PR also in turn closes rust-lang#83133.
@bors bors closed this as completed in d60cf78 Apr 5, 2021
@jsha jsha mentioned this issue May 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

List trait impls before deref methods in doc's sidebar slightlyoutofphase/rust
4 participants
@GuillaumeGomez @jyn514 @camelid @slightlyoutofphase