rust
rust copied to clipboard
rust-lang
[rustdoc] Sort impl associated items by kinds and then by appearance
Following this zulip discussion, I implemented it.
This brings the following change: impl associated items will now be grouped by kind and will now be first sorted by kind and then by the order they are declared in the source code (like currently).
The kinds are sorted in the following order:
- Constants
- Types
- Functions
The reason behind this order is that associated constants can be used in associated types (like length in arrays) and both associated types and associated constants can be used in associated functions. So if an associated item from the same impl is used, its definition will always be above where it's being used.
cc @camelid r? @notriddle
Team member @notriddle has asked teams: T-rustdoc-frontend, for consensus on:
- [x] @GuillaumeGomez
- [x] @camelid
- [x] @jsha
- [x] @notriddle
I understand the principle of putting types first, but I feel like the more common style is constants, then types, then functions. It's also probably more common to use constants in types, mostly array lengths.
@rfcbot concern is-this-the-right-order
I understand the principle of putting types first, but I feel like the more common style is constants, then types, then functions. It's also probably more common to use constants in types, mostly array lengths.
That's fine, as long as the sidebar also gets fixed to use the same order.
@GuillaumeGomez could you also upload a demo to crud? It'd be nice to see how it looks once it's finalized.
Updated the order: constants now come first, followed by types and then methods.
Sidebar in https://rustdoc.crud.net/imperio/sort-impl-associated-items/bar/trait.Foo.html disagrees with page content.
screenshot
Sidebar in https://rustdoc.crud.net/imperio/sort-impl-associated-items/bar/struct.Bar.html has no type Y at all.
screenshot
I keep forgetting that sidebar items are not generated at the same time as the page items. Maybe we should change that to prevent such case to happen again... I used this opportunity to add a test checking orders in the sidebar as well and updated the demo too.
Thanks @notriddle !
This looks pretty good, but I do wonder if it'd be better to just have gaps between the sidebar sections instead of headers. The headers are confusing because they all go to the same place, and they also add clutter. It should be fairly obvious to users what's what because of naming conventions (SNAKE_CASE for constants, CamelCase for types, and snake_case for methods).
Isn't it out-of-scope here? This PR doesn't change the headers after all. Might be worth debating this in another issue, don't you think?
You're totally right, it is out of scope ^^'. For some reason I forgot that we already have those headers -- I guess just used to seeing only "Methods". Checking my box now...
Yes, just need to wait for @notriddle's concern to be resolved. Also, please don't hesitate to open an issue if you think it's something we should talk about (the headers).
Perfect, then time to r+ it. Thanks everyone!
@bors r=t-rustdoc-frontend rollup
:pushpin: Commit 4a80840e272de06884115eeb970f283b8f8c459b has been approved by t-rustdoc-frontend
It is now in the queue for this repository.
