If a trait item appears in rustdoc search, hide the corrosponding impl items #145898

lolbinarycat · 2025-08-26T18:56:42Z

lolbinarycat · 2025-08-26T19:36:45Z

sometimes traitParent is not set properly for cross-crate implementations (eg. alloc::str::Bytes::last is not getting linked to Iterator::last).

this has been fixed (and removed from the PR description), i just needed to also be checking Cache::external_paths when resolving trait_parent_idx.

lolbinarycat · 2025-08-26T19:46:34Z

Right, I just remembered why my original design had its own deduplication pass, instead of leveraging the existing deduplication pass:

if the user searches for char::from, we should show them both ascii::Char::from and char::from. its only when a search matches the actual trait item (in this case, From::from) where we should perform this deduplication.

lolbinarycat · 2025-08-30T20:48:29Z

I believe I managed to get it working using notriddle's idea for how the search index data should be structured, but with my original idea of having two deduplication passes.

lolbinarycat · 2025-08-31T03:18:48Z

ok so apparently some impls for unnamable traits get lowered and then filtered out later? which seems mildly wasteful but ig in the meantime i need to remove this debug assertion.

rustbot · 2025-09-05T19:04:12Z

r? @notriddle

rustbot has assigned @notriddle.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-09-05T19:04:14Z

Some changes occurred in GUI tests.

cc @GuillaumeGomez

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha, @lolbinarycat

bors · 2025-09-19T22:46:53Z

☔ The latest upstream changes (presumably #146700) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2025-09-21T17:44:13Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

src/librustdoc/html/static/js/search.js

GuillaumeGomez · 2025-09-21T20:23:08Z

src/librustdoc/html/static/js/search.js

+                    } else {
+                        // FIXME: this is `O(n*m)` because we're repeatedly
+                        // shifting with Array.splice, but could be `O(n+m)` if
+                        // we did the shifting manually in a more clever way.


Do you think it might be worth doing right away? Considering we're trying to improve the search performance (and we don't have a common way to test it ><), it'd be nice to know exactly the impact of this code to know if we should care or not.

No idea if it's worth it, but I'd be happy to implement it, since its an interesting algorithm.

Can you try to check in your web browser the amount of time this function takes? They have nice performance recording tools, that would give already a pretty good idea. If it takes like 1% of the total, it's not really worth it.

tests/rustdoc-gui/search-result-color.goml

src/librustdoc/html/static/js/search.js

GuillaumeGomez · 2025-09-21T21:26:08Z

There isn't a test to ensure that items are hidden right? Or did I miss it?

lolbinarycat · 2025-09-21T21:31:08Z

There is no test currently, I will add one.

src/librustdoc/html/static/js/search.js

tests/rustdoc-gui/search-result-color.goml

notriddle · 2025-09-10T20:40:19Z

src/librustdoc/html/static/js/search.js

+                        // FIXME: this is `O(n*m)` because we're repeatedly
+                        // shifting with Array.splice, but could be `O(n+m)` if
+                        // we did the shifting manually in a more clever way.


You might want to bring up MAX_RESULTS, which means n can never exceed 20.

notriddle · 2025-09-10T20:41:30Z

tests/rustdoc-gui/search-result-color.goml

 define-function: (
    "check-search-color",
    [
-        theme, count_color, desc_color, path_color, bottom_border_color, keyword_color,


I'm confused. Why is this being removed?

It's not, it's being moved to check-alias. This unit test seemed really unreliable, and honestly I'm not entirely sure how it was passing in the first place.

src/librustdoc/html/static/js/search.js

for example, if we're showing `Iterator::next`, we don't need to also show `Range::next` in the results.

This comment has been minimized.

Sign in to view

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 738dcc9 to 8444219 Compare August 26, 2025 19:35

This comment has been minimized.

Sign in to view

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 8444219 to 0574e11 Compare August 30, 2025 20:21

This comment has been minimized.

Sign in to view

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 0574e11 to 0c19f15 Compare August 30, 2025 20:45

This comment has been minimized.

Sign in to view

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 0c19f15 to a78352a Compare August 30, 2025 20:56

This comment has been minimized.

Sign in to view

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from a78352a to 2cf6317 Compare August 31, 2025 03:19

This comment has been minimized.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment has been minimized.

Sign in to view

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 25836ae to 8f00bd5 Compare September 5, 2025 17:47

lolbinarycat marked this pull request as ready for review September 5, 2025 19:04

rustbot assigned notriddle Sep 5, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 5, 2025

lolbinarycat changed the title ~~WIP: don't show multiple instances of the same trait item in search~~ If a trait item appears in rustdoc search, hide the corrosponding impl items Sep 5, 2025

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 8f00bd5 to a45a03c Compare September 20, 2025 19:43

This comment has been minimized.

Sign in to view

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from a45a03c to 8f00bd5 Compare September 20, 2025 20:09

This comment has been minimized.

Sign in to view

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 8f00bd5 to b297e37 Compare September 21, 2025 17:44

GuillaumeGomez reviewed Sep 21, 2025

View reviewed changes

src/librustdoc/html/static/js/search.js Outdated Show resolved Hide resolved

GuillaumeGomez reviewed Sep 21, 2025

View reviewed changes

src/librustdoc/html/static/js/search.js Outdated Show resolved Hide resolved

GuillaumeGomez reviewed Sep 21, 2025

View reviewed changes

tests/rustdoc-gui/search-result-color.goml Show resolved Hide resolved

GuillaumeGomez reviewed Sep 21, 2025

View reviewed changes

src/librustdoc/html/static/js/search.js Outdated Show resolved Hide resolved

lolbinarycat added the relnotes Marks issues that should be documented in the release notes of the next release. label Sep 21, 2025

rustbot mentioned this pull request Sep 21, 2025

Tracking issue for release notes of #145898: If a trait item appears in rustdoc search, hide the corrosponding impl items #146864

Open

GuillaumeGomez reviewed Sep 21, 2025

View reviewed changes

src/librustdoc/html/static/js/search.js Show resolved Hide resolved

GuillaumeGomez reviewed Sep 21, 2025

View reviewed changes

tests/rustdoc-gui/search-result-color.goml Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from b9e60a2 to 84a6282 Compare September 22, 2025 17:12

notriddle reviewed Sep 27, 2025

View reviewed changes

lolbinarycat added 6 commits September 27, 2025 12:06

if a trait item is shown in search results, hide the impl item

58bc031

for example, if we're showing `Iterator::next`, we don't need to also show `Range::next` in the results.

fix some rustdoc-js-std tests broke by trait item filtering

be6d131

adjust broken rustdoc gui tests

f6c9937

search.js: introduce optimized removeIdxListAsc routine

b66ec61

apply suggestions from code review

3d40e93

add testcase for traitParent deduplication

1adb51f

lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 84a6282 to 1adb51f Compare September 27, 2025 17:06

If a trait item appears in rustdoc search, hide the corrosponding impl items #145898

Are you sure you want to change the base?

If a trait item appears in rustdoc search, hide the corrosponding impl items #145898

Uh oh!

Conversation

lolbinarycat commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

lolbinarycat commented Aug 26, 2025

Uh oh!

This comment has been minimized.

lolbinarycat commented Aug 26, 2025

Uh oh!

This comment has been minimized.

lolbinarycat commented Aug 30, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

lolbinarycat commented Aug 31, 2025

Uh oh!

This comment has been minimized.

This comment was marked as resolved.

This comment has been minimized.

rustbot commented Sep 5, 2025

Uh oh!

rustbot commented Sep 5, 2025

Uh oh!

bors commented Sep 19, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Sep 21, 2025

Uh oh!

Uh oh!

Uh oh!

GuillaumeGomez Sep 21, 2025

Choose a reason for hiding this comment

Uh oh!

lolbinarycat Sep 21, 2025

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez Sep 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

GuillaumeGomez commented Sep 21, 2025

Uh oh!

lolbinarycat commented Sep 21, 2025

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

notriddle Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

notriddle Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

lolbinarycat Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lolbinarycat commented Aug 26, 2025 •

edited

Loading