[rustdoc] Sort impl associated items by kinds and then by appearance by GuillaumeGomez · Pull Request #129471 · rust-lang/rust

GuillaumeGomez · 2024-08-23T14:27:49Z

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

GuillaumeGomez · 2024-08-23T14:28:40Z

+ //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[2]/h4' \
+ // 'pub const X: u8 = 12u8'
+ pub const X: u8 = 12;
+ //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[1]/h4' \


For reviewers: in XPath, indexes start at 1, not 0.

notriddle · 2024-08-24T19:56:48Z

Seems okay.

@rfcbot poll

rfcbot · 2024-08-24T19:56:51Z

Team member @notriddle has asked teams: T-rustdoc-frontend, for consensus on:

camelid · 2024-08-25T17:52:04Z

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.

notriddle · 2024-08-25T19:17:10Z

@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 · 2024-08-25T19:29:56Z

Dully noted. Thanks for pointing to the location!

camelid · 2024-08-25T19:49:35Z

@GuillaumeGomez could you also upload a demo to crud? It'd be nice to see how it looks once it's finalized.

GuillaumeGomez · 2024-08-25T20:04:19Z

Sure! I uploaded doc here.

GuillaumeGomez · 2024-08-25T20:05:12Z

Updated the order: constants now come first, followed by types and then methods.

notriddle · 2024-08-25T21:11:05Z

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

GuillaumeGomez · 2024-08-26T13:11:06Z

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 !

camelid · 2024-09-03T15:08:24Z

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).

GuillaumeGomez · 2024-09-03T15:11:48Z

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?

camelid · 2024-09-03T15:16:10Z

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...

camelid · 2024-09-03T15:16:42Z

We now have 3/4 approval, so this should be good to go now I think.

GuillaumeGomez · 2024-09-03T15:17:33Z

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).

notriddle · 2024-09-03T17:07:33Z

My concern is resolved.

GuillaumeGomez · 2024-09-03T17:52:55Z

Perfect, then time to r+ it. Thanks everyone!

@bors r=t-rustdoc-frontend rollup

bors · 2024-09-03T17:52:58Z

📌 Commit 4a80840 has been approved by t-rustdoc-frontend

It is now in the queue for this repository.

…d-items, r=t-rustdoc-frontend [rustdoc] Sort impl associated items by kinds and then by appearance Following [this zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/.22Freeze.22.20order.20of.20items.20in.20.28trait.29.20impls.3F), 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: 1. Constants 2. Types 3. 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`

…llaumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#127021 (Add target support for RTEMS Arm) - rust-lang#128871 (bypass linker configuration and cross target check for specific commands) - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance) - rust-lang#129529 (Add test to build crates used by r-a on stable) - rust-lang#129706 (Rename dump of coroutine by-move-body to be more consistent, fix ICE in dump_mir) - rust-lang#129796 (Unify scraped examples with other code examples) - rust-lang#129939 (explain why Rvalue::Len still exists) r? `@ghost` `@rustbot` modify labels: rollup

…d-items, r=t-rustdoc-frontend [rustdoc] Sort impl associated items by kinds and then by appearance Following [this zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/.22Freeze.22.20order.20of.20items.20in.20.28trait.29.20impls.3F), 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: 1. Constants 2. Types 3. 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``

…d-items, r=t-rustdoc-frontend [rustdoc] Sort impl associated items by kinds and then by appearance Following [this zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/.22Freeze.22.20order.20of.20items.20in.20.28trait.29.20impls.3F), 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: 1. Constants 2. Types 3. 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````

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#128919 (Add an internal lint that warns when accessing untracked data) - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance) - rust-lang#129653 (clarify that addr_of creates read-only pointers) - rust-lang#129775 (bootstrap: Try to track down why `initial_libdir` sometimes fails) - rust-lang#129939 (explain why Rvalue::Len still exists) - rust-lang#129940 (s390x: Fix a regression related to backchain feature) - rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc) - rust-lang#129943 (use the bootstrapped compiler for `test-float-parse` test) - rust-lang#129944 (Add compat note for trait solver change) - rust-lang#129947 (Add digit separators in `Duration` examples) - rust-lang#129955 (Temporarily remove fmease from the review rotation) - rust-lang#129957 (forward linker option to lint-docs) r? `@ghost` `@rustbot` modify labels: rollup

GuillaumeGomez · 2024-09-05T10:35:10Z

Fixed merge conflict.

@bors r=t-rustdoc-frontend

bors · 2024-09-05T10:35:12Z

📌 Commit 8f9c4b3 has been approved by t-rustdoc-frontend

It is now in the queue for this repository.

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#128919 (Add an internal lint that warns when accessing untracked data) - rust-lang#129472 (fix ICE when `asm_const` and `const_refs_to_static` are combined) - rust-lang#129653 (clarify that addr_of creates read-only pointers) - rust-lang#129775 (bootstrap: Try to track down why `initial_libdir` sometimes fails) - rust-lang#129939 (explain why Rvalue::Len still exists) - rust-lang#129940 (s390x: Fix a regression related to backchain feature) - rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc) - rust-lang#129943 (use the bootstrapped compiler for `test-float-parse` test) - rust-lang#129944 (Add compat note for trait solver change) - rust-lang#129947 (Add digit separators in `Duration` examples) - rust-lang#129955 (Temporarily remove fmease from the review rotation) - rust-lang#129957 (forward linker option to lint-docs) Failed merges: - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#129529 (Add test to build crates used by r-a on stable) - rust-lang#129624 (Adjust `memchr` pinning and run `cargo update`) - rust-lang#129720 (Simplify DestProp memory management) - rust-lang#129796 (Unify scraped examples with other code examples) - rust-lang#129891 (Do not request sanitizers for naked functions) - rust-lang#129938 (Elaborate on deriving vs implementing `Copy`) Failed merges: - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#128919 (Add an internal lint that warns when accessing untracked data) - rust-lang#129472 (fix ICE when `asm_const` and `const_refs_to_static` are combined) - rust-lang#129653 (clarify that addr_of creates read-only pointers) - rust-lang#129775 (bootstrap: Try to track down why `initial_libdir` sometimes fails) - rust-lang#129939 (explain why Rvalue::Len still exists) - rust-lang#129940 (s390x: Fix a regression related to backchain feature) - rust-lang#129942 (copy rustc rustlib artifacts from ci-rustc) - rust-lang#129943 (use the bootstrapped compiler for `test-float-parse` test) - rust-lang#129944 (Add compat note for trait solver change) - rust-lang#129947 (Add digit separators in `Duration` examples) - rust-lang#129955 (Temporarily remove fmease from the review rotation) - rust-lang#129957 (forward linker option to lint-docs) Failed merges: - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance) r? `@ghost` `@rustbot` modify labels: rollup

…d-items, r=t-rustdoc-frontend [rustdoc] Sort impl associated items by kinds and then by appearance Following [this zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/.22Freeze.22.20order.20of.20items.20in.20.28trait.29.20impls.3F), 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: 1. Constants 2. Types 3. 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`

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#128820 (fix: get llvm type of global val) - rust-lang#129028 (`impl_trait_overcaptures`: Don't worry about uncaptured contravariant lifetimes if they outlive a captured lifetime) - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance) - rust-lang#129706 (Rename dump of coroutine by-move-body to be more consistent, fix ICE in dump_mir) - rust-lang#129720 (Simplify DestProp memory management) - rust-lang#129796 (Unify scraped examples with other code examples) - rust-lang#129938 (Elaborate on deriving vs implementing `Copy`) - rust-lang#129973 (run_make_support: rename `Command::stdin` to `stdin_buf` and add `std{in,out,err}` config helpers) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#129471 - GuillaumeGomez:sort-impl-associated-items, r=t-rustdoc-frontend [rustdoc] Sort impl associated items by kinds and then by appearance Following [this zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/.22Freeze.22.20order.20of.20items.20in.20.28trait.29.20impls.3F), 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: 1. Constants 2. Types 3. 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``

GuillaumeGomez added the T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. label Aug 23, 2024

rustbot assigned notriddle Aug 23, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 23, 2024

GuillaumeGomez commented Aug 23, 2024
View reviewed changes

notriddle approved these changes Aug 24, 2024
View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 3, 2024

GuillaumeGomez mentioned this pull request Sep 3, 2024
Rollup of 7 pull requests #129946
Closed

tgross35 mentioned this pull request Sep 4, 2024
Rollup of 8 pull requests #129949
Closed

matthiaskrgr mentioned this pull request Sep 5, 2024
Rollup of 13 pull requests #129977
Closed

matthiaskrgr mentioned this pull request Sep 5, 2024
Rollup of 12 pull requests #129979
Closed

GuillaumeGomez added 5 commits September 5, 2024 12:13

Sort impl associated items by kinds and then by appearance
a459403

Add regression test for impl associated items sorting
f96aff9

Make impl associated constants sorted first
1884983

Add missing sidebar associated items
d059f37

Add regression test for sidebar associated items
238944c

This was referenced Sep 5, 2024
Rollup of 12 pull requests #129984
Closed
Rollup of 6 pull requests #129985
Closed

Update to new rustdoc internal API
8f9c4b3

GuillaumeGomez force-pushed the sort-impl-associated-items branch from 4a80840 to 8f9c4b3Compare September 5, 2024 10:33

This was referenced Sep 5, 2024
Rollup of 9 pull requests #129993
Closed
Rollup of 8 pull requests #129994
Merged

bors merged commit 6a23ee5 into rust-lang:masterSep 5, 2024

rustbot added this to the 1.83.0 milestone Sep 5, 2024

GuillaumeGomez deleted the sort-impl-associated-items branch September 6, 2024 14:04

GuillaumeGomez mentioned this pull request Dec 11, 2025
TODO list GuillaumeGomez/TODO-list#1
Open
24 tasks

Uh oh!

Uh oh!

Conversation

GuillaumeGomez commented Aug 23, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GuillaumeGomezAug 23, 2024

Choose a reason for hiding this comment

Uh oh!

notriddle commented Aug 24, 2024

Uh oh!

rfcbot commented Aug 24, 2024• edited by jsha Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camelid commented Aug 25, 2024

Uh oh!

notriddle commented Aug 25, 2024

Uh oh!

GuillaumeGomez commented Aug 25, 2024

Uh oh!

camelid commented Aug 25, 2024

Uh oh!

GuillaumeGomez commented Aug 25, 2024

Uh oh!

GuillaumeGomez commented Aug 25, 2024

Uh oh!

notriddle commented Aug 25, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GuillaumeGomez commented Aug 26, 2024

Uh oh!

camelid commented Sep 3, 2024

Uh oh!

GuillaumeGomez commented Sep 3, 2024

Uh oh!

camelid commented Sep 3, 2024

Uh oh!

camelid commented Sep 3, 2024

Uh oh!

GuillaumeGomez commented Sep 3, 2024

Uh oh!

notriddle commented Sep 3, 2024

Uh oh!

GuillaumeGomez commented Sep 3, 2024

Uh oh!

bors commented Sep 3, 2024

Uh oh!

GuillaumeGomez commented Sep 5, 2024

Uh oh!

bors commented Sep 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

GuillaumeGomez commented Aug 23, 2024•
edited
Loading

rfcbot commented Aug 24, 2024•
edited by jsha
Loading

notriddle commented Aug 25, 2024•
edited
Loading