rustdoc: simplify JS search routine by not messing with lev distance

[notriddle]

Since the sorting function accounts for an index field, there's not much reason to also be applying changes to the levenshtein distance. Instead, we can just not treat lev as a filter if there's already a non-sentinel value for index.

This change gives slightly more weight to the index and path part, as search criteria, than it used to. This changes some of the test cases, but not in any obviously-"worse" way, and, in particular, substring matches are a bigger deal than levenshtein distances (we're assuming that a typo is less likely than someone just not typing the entire name).

The biggest change is the addition of a path_lev field to result items. It's always zero if the search query has no parent path part and for type queries, making the check in the sortResults function a no-op. When it's present, it is used to implement different precedence for the parent path and the tail.

Consider the query hashset::insert, a test case that already exists and can be found here. We want the ordering shown in the test case:

        { 'path': 'std::collections::hash_set::HashSet', 'name': 'insert' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_with' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_owned' },
        { 'path': 'std::collections::hash_map::HashMap', 'name': 'insert' },

We do not want this ordering, which is the ordering that would occur if substring position took priority over path_lev:

        { 'path': 'std::collections::hash_set::HashSet', 'name': 'insert' },
        { 'path': 'std::collections::hash_map::HashMap', 'name': 'insert' }, // BAD
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_with' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_owned' },

We also do not want HashSet::iter to appear before HashMap::insert, which is what would happen if path_lev took priority over the appearance of any substring match. This is why the sortResults function has path_lev sandwiched between a index < 0 check and a index comparison check:

        { 'path': 'std::collections::hash_set::HashSet', 'name': 'insert' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_with' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_owned' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'iter' }, // BAD
        { 'path': 'std::collections::hash_map::HashMap', 'name': 'insert' },

The old code implemented a similar feature by manipulating the lev member based on whether a substring match was found and averaging in the path distance (item.lev = name_lev + path_lev / 10), so the path lev wound up acting like a tie breaker, but it gives slightly different results for Vec::new, changing the test case because of the slight changes to ordering priority.

Based on #103710 (comment)

Previews:

• https://notriddle.com/notriddle-rustdoc-demos/rustdoc-search-stop-doing-demerits/std/index.html
• https://notriddle.com/notriddle-rustdoc-demos/rustdoc-search-stop-doing-demerits-compiler/index.html

[rustbot]

r? @GuillaumeGomez

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

[GuillaumeGomez]

Looks good to me. Considering search is a bit sensitive, let's also check if it's okay for others.

cc @rust-lang/rustdoc

[camelid]

I tested a couple of cases, and none of the changes seemed like an issue to me. So this PR is fine with me.

[notriddle]

@GuillaumeGomez Should this go through a FCP?

[GuillaumeGomez]

I think it'd be a good idea to prevent it to be stuck on unknown status. Can you write up differences more in details so we can have a better overview of the changes brought by this PR please?

[notriddle]

Also added this to the PR description:

The biggest change is the addition of a path_lev field to result items. It's always zero if the search query has no parent path part and for type queries, making the check in the sortResults function a no-op. When it's present, it is used to implement different precedence for the parent path and the tail.

Consider the query hashset::insert, a test case that already exists and can be found here. We want the ordering shown in the test case:

        { 'path': 'std::collections::hash_set::HashSet', 'name': 'insert' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_with' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_owned' },
        { 'path': 'std::collections::hash_map::HashMap', 'name': 'insert' },

We do not want this ordering, which is the ordering that would occur if substring position took priority over path_lev:

        { 'path': 'std::collections::hash_set::HashSet', 'name': 'insert' },
        { 'path': 'std::collections::hash_map::HashMap', 'name': 'insert' }, // BAD
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_with' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_owned' },

We also do not want HashSet::iter to appear before HashMap::insert, which is what would happen if path_lev took priority over the appearance of any substring match. This is why the sortResults function has path_lev sandwiched between a index < 0 check and a index comparison check:

        { 'path': 'std::collections::hash_set::HashSet', 'name': 'insert' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_with' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'get_or_insert_owned' },
        { 'path': 'std::collections::hash_set::HashSet', 'name': 'iter' }, // BAD
        { 'path': 'std::collections::hash_map::HashMap', 'name': 'insert' },

The old code implemented a similar feature by manipulating the lev member based on whether a substring match was found and averaging in the path distance (item.lev = name_lev + path_lev / 10), so the path lev wound up acting like a tie breaker, but it gives slightly different results for Vec::new, changing the test case because of the slight changes to ordering priority.

[Manishearth]

I'm in favor of this

[GuillaumeGomez]

Thanks for the explanation @notriddle. Let's start the FCP then.

@rfcbot fcp merge

[rfcbot]

Team member @GuillaumeGomez has proposed to merge this. The next step is review by the rest of the tagged team members:

• @GuillaumeGomez
• @Manishearth
• @Nemo157
• @aDotInTheVoid
• @camelid
• @jsha
• @notriddle

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[GuillaumeGomez]

Thanks!

@bors r+

[bors]

📌 Commit db558b4 has been approved by GuillaumeGomez

It is now in the queue for this repository.

[notriddle]

It vanished from the queue.

@bors r=GuillaumeGomez

[bors]

📌 Commit db558b4 has been approved by GuillaumeGomez

It is now in the queue for this repository.