[rustdoc] Sort impl associated items by kinds and then by appearance #129471

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

tests/rustdoc/impl-associated-items-order.rs

+    //@ 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```

…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 8f9c4b3 Compare 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:master Sep 5, 2024
6 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rustdoc] Sort impl associated items by kinds and then by appearance #129471

[rustdoc] Sort impl associated items by kinds and then by appearance #129471

GuillaumeGomez commented Aug 23, 2024 •

edited

Loading

GuillaumeGomez Aug 23, 2024

notriddle commented Aug 24, 2024

rfcbot commented Aug 24, 2024 •

edited by jsha

Loading

camelid commented Aug 25, 2024

notriddle commented Aug 25, 2024

GuillaumeGomez commented Aug 25, 2024

camelid commented Aug 25, 2024

GuillaumeGomez commented Aug 25, 2024

GuillaumeGomez commented Aug 25, 2024

notriddle commented Aug 25, 2024 •

edited

Loading

GuillaumeGomez commented Aug 26, 2024

camelid commented Sep 3, 2024

GuillaumeGomez commented Sep 3, 2024

camelid commented Sep 3, 2024

camelid commented Sep 3, 2024

GuillaumeGomez commented Sep 3, 2024

notriddle commented Sep 3, 2024

GuillaumeGomez commented Sep 3, 2024

bors commented Sep 3, 2024

GuillaumeGomez commented Sep 5, 2024

bors commented Sep 5, 2024

[rustdoc] Sort impl associated items by kinds and then by appearance #129471

[rustdoc] Sort impl associated items by kinds and then by appearance #129471

Conversation

GuillaumeGomez commented Aug 23, 2024 • edited Loading

GuillaumeGomez Aug 23, 2024

Choose a reason for hiding this comment

notriddle commented Aug 24, 2024

rfcbot commented Aug 24, 2024 • edited by jsha Loading

camelid commented Aug 25, 2024

notriddle commented Aug 25, 2024

GuillaumeGomez commented Aug 25, 2024

camelid commented Aug 25, 2024

GuillaumeGomez commented Aug 25, 2024

GuillaumeGomez commented Aug 25, 2024

notriddle commented Aug 25, 2024 • edited Loading

GuillaumeGomez commented Aug 26, 2024

camelid commented Sep 3, 2024

GuillaumeGomez commented Sep 3, 2024

camelid commented Sep 3, 2024

camelid commented Sep 3, 2024

GuillaumeGomez commented Sep 3, 2024

notriddle commented Sep 3, 2024

GuillaumeGomez commented Sep 3, 2024

bors commented Sep 3, 2024

GuillaumeGomez commented Sep 5, 2024

bors commented Sep 5, 2024

GuillaumeGomez commented Aug 23, 2024 •

edited

Loading

rfcbot commented Aug 24, 2024 •

edited by jsha

Loading

notriddle commented Aug 25, 2024 •

edited

Loading