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

Search over generic types in docs #45673

Merged
merged 6 commits into from
Nov 13, 2017

Conversation

GuillaumeGomez
Copy link
Member

This is what I was talking about @QuietMisdreavus. Now we have generics.

Waiting for #45617 to get merged.

@rust-highfive
Copy link
Collaborator

r? @QuietMisdreavus

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

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 1, 2017
@GuillaumeGomez GuillaumeGomez mentioned this pull request Nov 1, 2017
@GuillaumeGomez GuillaumeGomez force-pushed the rustdoc-type-search-generic branch 3 times, most recently from fc2fb75 to 6f88227 Compare November 1, 2017 23:54
@GuillaumeGomez
Copy link
Member Author

It's now fully functional. You can look for type as generic parameters and look for types such as "Result". A little screenshot of the last commit:

screen shot 2017-11-02 at 01 02 13

@GuillaumeGomez GuillaumeGomez changed the title [WIP] Only search over generic types in docs Only search over generic types in docs Nov 3, 2017
@GuillaumeGomez GuillaumeGomez changed the title Only search over generic types in docs Search over generic types in docs Nov 3, 2017
@QuietMisdreavus
Copy link
Member

Shouldn't some of these be in the "As return value" instead?

image

I would expect the "As return value" tab to prioritize perfect matches of one of the type variables over ones that have no type variables but have a low lev score:

image

For public perusal, i have a rendering of the current PR here.

@QuietMisdreavus
Copy link
Member

Now that #45617 is merged, can you rebase on top of it to factor out the commits that were taken from there?

return b;
}

function nbElements(obj) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think numElements would be a better name for this.

if (obj.generics &&
obj.generics.length >= val.generics.length) {
var elems = obj.generics.slice(0);
for (var y = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that this would be better off on the same line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, only issue: line > 100 columns. :)

if (obj && obj.type && obj.type.inputs.length > 0) {
for (var i = 0; i < obj.type.inputs.length; i++) {
if (obj.type.inputs[i].name === val) {
var tmp = checkType(obj.type.inputs[i], val, literalSearch);
if (literalSearch && tmp === true) {
Copy link
Contributor

@projektir projektir Nov 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check is literalSearch === true in other parts of the code.


if (obj && obj.type && obj.type.output) {
var tmp = checkType(obj.type.output, val, literalSearch);
if (literalSearch && tmp === true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@shepmaster shepmaster 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 11, 2017
@shepmaster
Copy link
Member

Ping from triage — @GuillaumeGomez will you be able to address some of the feedback you've received?

@GuillaumeGomez
Copy link
Member Author

I'll be able to address all of them.

@bors
Copy link
Contributor

bors commented Nov 11, 2017

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

@GuillaumeGomez
Copy link
Member Author

Updated.

@QuietMisdreavus
Copy link
Member

This still seems to have a problem of taking all the items that should be in "As parameters" and putting them into "Types/modules" instead.

Your PR:

image

image

Current nightly:

image

image

@GuillaumeGomez
Copy link
Member Author

Hum, still not good then... Let's see what's happening.

@GuillaumeGomez
Copy link
Member Author

Ok, the mystery has been solved: I forgot to add the results into the tab. We're good now. :)

Copy link
Member

@QuietMisdreavus QuietMisdreavus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Home stretch! With that last change, this totally works. Just one last nit and i'll be ready to call it good.

// FIXME: add all from clean::Type.
_ => None
}
}

fn get_generics(clean_type: &clean::Type) -> Option<Vec<String>> {
match *clean_type {
clean::ResolvedPath { ref path, .. } => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this just reuse Type::generics rather than copying it out? That way, if other Types support generics, both this and the "document impls when the type appears in the traits generics" benefit.

@QuietMisdreavus
Copy link
Member

Excellent! r=me pending travis.

Thanks so much for doing this! This makes the extended search tabs much nicer.

@GuillaumeGomez
Copy link
Member Author

@bors: r=QuietMisdreavus

@bors
Copy link
Contributor

bors commented Nov 12, 2017

📌 Commit 0e4c829 has been approved by QuietMisdreavus

@bors
Copy link
Contributor

bors commented Nov 13, 2017

⌛ Testing commit 0e4c829 with merge c8c1424...

bors added a commit that referenced this pull request Nov 13, 2017
…QuietMisdreavus

Search over generic types in docs

This is what I was talking about @QuietMisdreavus. Now we have generics.

Waiting for #45617 to get merged.
@bors
Copy link
Contributor

bors commented Nov 13, 2017

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

@bors bors merged commit 0e4c829 into rust-lang:master Nov 13, 2017
@GuillaumeGomez GuillaumeGomez deleted the rustdoc-type-search-generic branch November 13, 2017 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants