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: Fix missing search results #50482

Closed
wants to merge 1 commit into from

Conversation

ollie27
Copy link
Member

@ollie27 ollie27 commented May 6, 2018

This code seemed to be trying to remove duplicates from the search results but that should already have happened by this point. It was removing results that only differed in their parent causing missing results.

For example f64::is_nan is missing from https://doc.rust-lang.org/nightly/std/?search=is_nan

r? @GuillaumeGomez

This code seemed to be trying to remove duplicates from the search results but that should already have happened
by this point. It was removing results that only differed in their parent causing missing results.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 6, 2018
@GuillaumeGomez
Copy link
Member

Please add a test in test/rustdoc-js. It'll allow to avoid regression. Otherwise good catch, thanks a lot!

@ollie27
Copy link
Member Author

ollie27 commented May 7, 2018

Adding a test would be quite difficult because the addTab function isn't tested by rustdoc-js.

@GuillaumeGomez
Copy link
Member

The results are tested. You said that is_nan was missing from the results. Then just check that it is present now.

@ollie27
Copy link
Member Author

ollie27 commented May 10, 2018

rustdoc-js doesn't test the code that I've modified. This test already passes:

const QUERY = 'is_nan';

const EXPECTED = {
    'others': [
        { 'path': 'std::f32', 'name': 'is_nan' },
        { 'path': 'std::f64', 'name': 'is_nan' },
    ],
};

Are you suggesting I add the capability to test the rendering code in rustdoc-js?

This is fixing a stable to beta regression so will need to be backported if it is accepted.

@ollie27 ollie27 mentioned this pull request May 10, 2018
@ollie27
Copy link
Member Author

ollie27 commented May 11, 2018

It turns out that this code was removing some real duplicates caused by multiple query searches so I'm closing this in favour of an issue: #50658

@ollie27 ollie27 closed this May 11, 2018
@ollie27 ollie27 deleted the rustdoc_search_dedupe branch June 18, 2018 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants