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

Fix rustdoc pathes search #50432

Merged
merged 2 commits into from
May 10, 2018
Merged

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented May 3, 2018

Fixes #50086.

Depends on #50302.

r? @QuietMisdreavus

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 3, 2018
@GuillaumeGomez GuillaumeGomez changed the title Fix vec new search Fix rustdoc pathes search May 3, 2018
@kennytm kennytm added S-blocked Status: Blocked on something else such as an RFC or other implementation work. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels May 3, 2018
@QuietMisdreavus
Copy link
Member

I'm confused why you added the exact-check flag but didn't actually use it.

Also, now that #50302 is merged, can you rebase to filter that commit out of this PR?

@GuillaumeGomez
Copy link
Member Author

I'm confused why you added the exact-check flag but didn't actually use it.

Good catch! I now use it in the pinbox test. :)

@QuietMisdreavus
Copy link
Member

Note to self when the new exact-check flag is added to rustc-guide: The difference between setting exact-check and not setting ignore-order is that "no flags" only means that the actual results include the expected results in the right order, but exact-check ensures that no additional items are in the actual result. In other words, the actual result is exactly the same as the input.

@QuietMisdreavus
Copy link
Member

Okay, after a bit of a chat to piece together what's going on, i'm okay with this. The key takeaway is that the conditional being changed was being used to short-circuit any query which matched the input, regardless of whether it was part of a longer path. The added conditional makes it so that this branch only gets hit on queries that don't have a :: in it. That makes it so that other sorters come in to order the results, thus putting std::vec::Vec::new back on top when you're searching for vec::new.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented May 7, 2018

📌 Commit 2bfbe55 has been approved by QuietMisdreavus

@bors 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 May 7, 2018
@kennytm
Copy link
Member

kennytm commented May 9, 2018

@bors r-

Checking "pinbox-new.js" ... FAILED
==> Exact check failed at position 1: expected '{"path":"std::boxed::PinBox","name":"new"}' but found '{"crate":"std","ty":11,"name":"new","path":"std::boxed","desc":"Allocate memory on the heap, move the data into it and pin it.","parent":{"ty":3,"name":"PinBox"},"type":{"inputs":[{"name":"t"}],"output":{"name":"pinbox"}},"lev":1}'

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 9, 2018
@GuillaumeGomez
Copy link
Member Author

Fixed.

@kennytm kennytm 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 May 9, 2018
@QuietMisdreavus
Copy link
Member

Since the travis builder doesn't have node, we won't know whether this works until bors gets its hands on it.

@bors r+ rollup-

@bors
Copy link
Contributor

bors commented May 9, 2018

📌 Commit 2c91b49 has been approved by QuietMisdreavus

@bors 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 May 9, 2018
@bors
Copy link
Contributor

bors commented May 10, 2018

⌛ Testing commit 2c91b49 with merge c8a3ec1...

bors added a commit that referenced this pull request May 10, 2018
@bors
Copy link
Contributor

bors commented May 10, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing c8a3ec1 to master...

@bors bors merged commit 2c91b49 into rust-lang:master May 10, 2018
@GuillaumeGomez GuillaumeGomez deleted the fix-vec-new-search branch May 10, 2018 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants