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: "Methods from Deref<...>" too low on sidebar #85618

Closed
jsha opened this issue May 24, 2021 · 5 comments · Fixed by #86564
Closed

rustdoc: "Methods from Deref<...>" too low on sidebar #85618

jsha opened this issue May 24, 2021 · 5 comments · Fixed by #86564
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jsha
Copy link
Contributor

jsha commented May 24, 2021

Steps to reproduce:

  1. Visit https://doc.rust-lang.org/beta/std/string/struct.String.html
  2. Scroll down the sidebar looking for "Methods from Deref<...>"

Expected result:

Methods from Deref section comes immediately after Methods.

Actual result:

Methods from Deref section comes below Trait Implementations, Auto Trait Implementations, and Blanket Implementations.

Compare vs https://doc.rust-lang.org/std/string/struct.String.html, which has the correct ordering.

@jsha jsha added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. C-bug Category: This is a bug. A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels May 24, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 24, 2021
@jsha
Copy link
Contributor Author

jsha commented May 24, 2021

Ah, looks like this was intentional in #83826, after conversation in #83133. Removing C-bug and regression-from-stable-to-beta. Keeping this open as a discussion issue: I think putting "Methods from Deref" further down than trait implementations is wrong for String, because so many of its important methods are on str. I'd like to either reconsider #83133 or find some way to make this work nicely for String. Vec is probably in the same boat, with its impl Deref<Target = [T]>.

@jsha jsha removed C-bug Category: This is a bug. I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels May 24, 2021
@ilyvion
Copy link

ilyvion commented May 24, 2021

I'd like to voice that I also think this is a kind of regression. 99% of the time I go to a type's page, I'm looking for methods, not traits; I've gotten used to seeing str's methods directly under Strings, and it feels like a mistake moving away from this.

@jsha
Copy link
Contributor Author

jsha commented Jun 13, 2021

@jyn514, @slightlyoutofphase, what do you think of reverting #83826 while we look for a better solution. From #83133:

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

The problem with this is sometimes the "Methods from Deref" section refers to a type in the crate. Particularly for Vec and String, about half of the most useful methods are on [T] and str, respectively.

Those are types that are a critical part of the new user learning path, and it's a good user experience for those to be right next to the other methods, both in the sidebar and in the doc.

I suspect there are other ways to solve the need expressed in #83133 without affecting the page layout of Vec and String in this way.

@jyn514
Copy link
Member

jyn514 commented Jun 21, 2021

@jsha that's fine - #83826 was a response to #80653, which has since been reverted.

@slightlyoutofphase
Copy link
Contributor

slightlyoutofphase commented Dec 23, 2021

I suspect there are other ways to solve the need expressed in #83133 without affecting the page layout of Vec and String in this way.

FWIW, I originally opened that issue with something more like a directly configurable way of specifying the page layout order for your own crate in mind. I still think that would ultimately be the best solution here. Unsure what the best approach would be, though, as far as top level attribute vs. command-line flag vs. any other way of doing it.

Other stuff like what is and isn't "collapsed" by default should probably be configurable also, I'd say.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants