Add tabs for search for better information access #45055

GuillaumeGomez · 2017-10-05T22:55:38Z

A few screenshots:

r? @rust-lang/docs

QuietMisdreavus · 2017-10-05T23:18:28Z

[00:04:01] tidy error: /checkout/src/librustdoc/html/static/main.js:896: line longer than 100 chars
[00:04:01] tidy error: /checkout/src/librustdoc/html/static/main.js:898: line longer than 100 chars
[00:04:01] some tidy checks failed

GuillaumeGomez · 2017-10-06T08:49:00Z

Arf, forgot tidy check once again...

killercup

Nice! This is a very nice step to making the search more accessible.

At RustFest, we talked a bit about what the search can already do, but one thing I didn't think of: Can you search for a function that has "foo" in its name that returns a usize? I'm not sure I have a good idea how to do the UI for this, but I guess the code itself might already support this in some capacity.

killercup · 2017-10-06T10:24:26Z

src/librustdoc/html/static/main.js

@@ -342,6 +342,18 @@
                }
            }

+            function findArg(obj, val) {
+                if (!obj || !obj.type || obj.type.inputs.length === 0) {


Heh, that's such a negative way to write this ;) How about

obj && obj.type && obj.type.inputs && obj.type.inputs.length > 0

I wanted to avoid putting all my code in an if condition but either way is fine for me so let's go for it!

killercup · 2017-10-06T10:28:00Z

src/librustdoc/html/static/main.js

+                        return true;
+                    }
+                }
+                return false;


You replace the whole for thing with

return obj.type.inputs.some(function (x) { return x.name === name; })

but I'm not sure if we still need to support browsers which don't have Array.prototype.some.

That's the point. I'm supporting as much browsers as I can.

killercup · 2017-10-06T10:30:16Z

src/librustdoc/html/static/main.js

+                '<div id="titles">' +
+                '<div class="selected">Types/modules</div>' +
+                '<div>As parameters</div>' +
+                '<div>Returned</div></div><div id="results">';


Maybe

As return value

for consistency?

Good point!

killercup · 2017-10-06T10:30:45Z

src/librustdoc/html/static/main.js

+                (query.type ? ' (type: ' + escape(query.type) + ')' : '') + '</h1>' +
+                '<div id="titles">' +
+                '<div class="selected">Types/modules</div>' +
+                '<div>As parameters</div>' +


Maybe

As a parameter

You can look for multiple parameters at once (String, usize -> *).

killercup · 2017-10-06T10:31:59Z

src/librustdoc/html/static/main.js

+            var elems = document.getElementById('titles').childNodes;
+            elems[0].onclick = function() { printTab(0); };
+            elems[1].onclick = function() { printTab(1); };
+            elems[2].onclick = function() { printTab(2); };


Protip:

elems[2].onclick = function() { printTab(2); };

can be

elems[2].onclick = printTab.bind(null, 2)

I find it less readable. :-/

GuillaumeGomez · 2017-10-06T11:05:43Z

At RustFest, we talked a bit about what the search can already do, but one thing I didn't think of: Can you search for a function that has "foo" in its name that returns a usize? I'm not sure I have a good idea how to do the UI for this, but I guess the code itself might already support this in some capacity.

You can't do both at once. You can say that you're looking for a function called to_string:

fn: to_string

Or for a function which returns usize:

fn: -> usize

QuietMisdreavus · 2017-10-08T21:05:19Z

src/librustdoc/html/static/main.js

+                        if (results['others'].length < maxResults) {
+                            results['others'].push(obj);
+                        }
+                    } else {


Forgive my ignorance of the JS part of rustdoc, but i think i'm missing something. What is obj.type here, and why does its presence keep it from getting into results['others']? Is that on purpose?

You can see everything as a hashmap in js, so obj.type == obj['type']. In this case, if there is no type (which is just a regular field, nothing particular), it means it's not a function nor a method so I put it in "others".

So is results['others'] not meant to be the same as the current search results? I'm concerned that you won't be able to find a function by name if it's not returning something with the same name. For example, are you able to find str::to_lowercase by searching for something like lowercase? Will you be able to find that method on any tab in your new layout?

Make tabs work

QuietMisdreavus · 2017-10-09T22:24:20Z

src/librustdoc/html/static/main.js

+                        }
+                    }
+                    if (results['others'].length < maxResults &&
+                        ((query.search && obj.name.indexOf(query.search)) || added === false)) {


Why is this condition not just results['others'].length < maxResults? I just checked out your PR and didn't see any difference between the longer form you have here and taking these extra conditions out. What is this trying to do?

In case the only thing(s ?) match are the arguments or the returned type, I don't want to add extra information. For me, the 'others' tab is also about the type/function/module's name so it seems logical to not add everything (even if the filter is pretty light).

After some checking, it seems like it won't stop things getting into results if their name matches as well as having a parameter type that matches (e.g. "Result" will show Try::into_result on both the "As return value" tab as well as the "Types/modules" tab) so i'll retract this concern.

QuietMisdreavus · 2017-10-10T22:09:25Z

@bors r+

I'm incredibly excited to see this land. I think it greatly helps discoverability in docs. It's not quite perfect (e.g. searches for io things won't get a lot of type-based results since they're all wrapped up in Results, but searching for Result doesn't return the mass of responses you would expect - in fact, it only returns places where Result was used with generic types, like on the Try trait, or on Option/Result combinators) but i'd like to get this landed and work on tweaking the search index in follow-up PRs.

cc #44024 since this helps that out

bors · 2017-10-10T22:09:26Z

📌 Commit 3a65d12 has been approved by QuietMisdreavus

bors · 2017-10-11T00:20:45Z

⌛ Testing commit 3a65d12 with merge 8dcd0e127953f5eaff18aad9f1ea54f4c9712d02...

bors · 2017-10-11T00:28:13Z

💔 Test failed - status-travis

kennytm · 2017-10-11T06:37:09Z

@bors retry

Android SDK HTTPS issue (fixed in ci: Fix installing the Android SDK #45193)

bors · 2017-10-13T11:34:18Z

⌛ Testing commit 3a65d12 with merge d21c023...

@killercup

Add tabs for search for better information access A few screenshots: <img width="1440" alt="screen shot 2017-10-06 at 00 54 51" src="https://user-images.githubusercontent.com/3050060/31256148-032c1a06-aa31-11e7-8e4c-fec59786b8e6.png"> <img width="1440" alt="screen shot 2017-10-06 at 00 54 58" src="https://user-images.githubusercontent.com/3050060/31256150-03312cb2-aa31-11e7-86f7-8c9f0d8d6d4f.png"> <img width="1440" alt="screen shot 2017-10-06 at 00 55 00" src="https://user-images.githubusercontent.com/3050060/31256149-0330d456-aa31-11e7-8f89-3b3c824e30b4.png"> r? @rust-lang/docs cc @killercup @QuietMisdreavus

bors · 2017-10-13T14:04:49Z

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

killercup reviewed Oct 6, 2017

View reviewed changes

GuillaumeGomez force-pushed the search-tabs branch 2 times, most recently from dbff8b7 to 52bcc43 Compare October 6, 2017 11:02

shepmaster added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 6, 2017

QuietMisdreavus reviewed Oct 8, 2017

View reviewed changes

GuillaumeGomez force-pushed the search-tabs branch 2 times, most recently from 1cb08c0 to 8a53447 Compare October 9, 2017 21:16

Add tabs for search for better information access

3a65d12

Make tabs work

GuillaumeGomez force-pushed the search-tabs branch from 8a53447 to 3a65d12 Compare October 9, 2017 21:26

QuietMisdreavus reviewed Oct 9, 2017

View reviewed changes

QuietMisdreavus mentioned this pull request Oct 10, 2017

Generated doc pages for types should include more info about the type's relationships #44024

Open

kennytm 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 Oct 13, 2017

bors merged commit 3a65d12 into rust-lang:master Oct 13, 2017

GuillaumeGomez deleted the search-tabs branch October 14, 2017 13:40

Seeker14491 mentioned this pull request Oct 29, 2017

[rustdoc] Various issues with search tabs #45608

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tabs for search for better information access #45055

Add tabs for search for better information access #45055

GuillaumeGomez commented Oct 5, 2017

QuietMisdreavus commented Oct 5, 2017

GuillaumeGomez commented Oct 6, 2017

killercup left a comment

killercup Oct 6, 2017

GuillaumeGomez Oct 6, 2017

killercup Oct 6, 2017

GuillaumeGomez Oct 6, 2017

killercup Oct 6, 2017

GuillaumeGomez Oct 6, 2017

killercup Oct 6, 2017

GuillaumeGomez Oct 6, 2017

killercup Oct 6, 2017

GuillaumeGomez Oct 6, 2017

GuillaumeGomez commented Oct 6, 2017

QuietMisdreavus Oct 8, 2017

GuillaumeGomez Oct 8, 2017

QuietMisdreavus Oct 8, 2017

QuietMisdreavus Oct 9, 2017

GuillaumeGomez Oct 10, 2017

QuietMisdreavus Oct 10, 2017

QuietMisdreavus commented Oct 10, 2017

bors commented Oct 10, 2017

bors commented Oct 11, 2017

bors commented Oct 11, 2017

kennytm commented Oct 11, 2017

bors commented Oct 13, 2017

bors commented Oct 13, 2017

Add tabs for search for better information access #45055

Add tabs for search for better information access #45055

Conversation

GuillaumeGomez commented Oct 5, 2017

QuietMisdreavus commented Oct 5, 2017

GuillaumeGomez commented Oct 6, 2017

killercup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Oct 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuietMisdreavus commented Oct 10, 2017

bors commented Oct 10, 2017

bors commented Oct 11, 2017

bors commented Oct 11, 2017

kennytm commented Oct 11, 2017

bors commented Oct 13, 2017

bors commented Oct 13, 2017