Conversation
GuillaumeGomez
force-pushed
the
fix-global-search
branch
from
91cb73e to
620a004
Compare
|
Seems like it messes with generics. |
GuillaumeGomez
force-pushed
the
fix-global-search
branch
from
620a004 to
cce9da2
Compare
|
Ok, generics are working again. Added comments to avoid removing the same code again. |
src/librustdoc/html/static/main.js
Outdated
There was a problem hiding this comment.
I'm still wondering if I should return the lowest lev distance or if I should return the average lev value... Any preference?
There was a problem hiding this comment.
I can see how returning the average would be better, but i'm not sure how effective it will be. A user would need to search for something with multiple generics (which i'm not sure works right now? a search for Result<File, Error> returns a lot of bad lev results before any of the functions that return that type show up) before the difference will even show up. I think we can leave this as-is for now and investigate it in the future.
There was a problem hiding this comment.
I'll leave the comment just in case then if you don't mind.
kennytm
added
the
S-waiting-on-review
src/librustdoc/html/static/main.js
Outdated
There was a problem hiding this comment.
Not really an issue with this PR, but I don't like how there's inconsistent var styles being used with some functions having one var per line, and others using commas like they're going out of style.
There was a problem hiding this comment.
I grouped them by "theme". But if you want I can put each on a line.
src/librustdoc/html/static/main.js
Outdated
There was a problem hiding this comment.
The same as anywhere else in the code using searchIndex? :p How would you name it?
src/librustdoc/html/static/main.js
Outdated
There was a problem hiding this comment.
Also not really an issue with this PR, but the isType !== true in an if thing really isn't saving you any speed of !isType.
There was a problem hiding this comment.
Don't care. In principle it's faster since the comparison is explicit.
src/librustdoc/html/static/main.js
Outdated
There was a problem hiding this comment.
This line seems like it was left behind by accident.
|
☔ The latest upstream changes (presumably #46081) made this pull request unmergeable. Please resolve the merge conflicts. |
GuillaumeGomez
force-pushed
the
fix-global-search
branch
from
cce9da2 to
0a12198
Compare
|
Rebased. |
|
@bors r+ |
|
📌 Commit 0a12198 has been approved by |
|
I suggest we backport this (and possibly #46081 also) to 1.23-beta, as search is broken on beta as well. |
|
I agree - if possible, i'd rather not break the search for docs that show up on stable. |
kennytm
added
the
beta-nominated
|
Let's move up the priority then. @bors: p=10 |
…eavus Fix global search Fixes #46021. r? @QuietMisdreavus
|
☀️ Test successful - status-appveyor, status-travis |
[beta] Doc search backports This is a backport of #46081, #46175, #46433, and #46672. They all merged cleanly but I haven't tried a build; let's see what Travis says. These PRs fix pretty annoying issues with doc search and so I think it's important they don't slip to stable, but these PRs have *NOT* been `beta-accepted` yet. cc @steveklabnik @GuillaumeGomez can you tag the docs team to talk about beta-acceptance?
|
Looks like we forgot to backport this to 1.23.0 (sorry about that!) so removing beta tags |
alexcrichton
removed
the
beta-nominated
|
Er sorry looks like this was backported in #46886 |
alexcrichton
added
the
beta-accepted
6 participants
Fixes #46021.
r? @QuietMisdreavus