Rustdoc improvements #50259

GuillaumeGomez · 2018-04-26T22:03:49Z

Fixes #50658. (last commit)

A lot of small improvements.

r? @QuietMisdreavus

QuietMisdreavus · 2018-04-26T22:05:25Z

It looks like this includes the commit from #50229. Does this require it to be merged first?

GuillaumeGomez · 2018-04-27T09:01:25Z

Oh sorry, forgot to precise: this PR depends on #50229.

GuillaumeGomez · 2018-04-28T14:38:28Z

Rebased.

QuietMisdreavus · 2018-05-02T21:38:18Z

This is a bit above my abstraction threshold for knowing what these are doing offhand. Can you give a quick description of your commits to say what they're supposed to be doing? Since there's no "Fixes (issue link)" i can't tell what you're trying to do here.

GuillaumeGomez · 2018-05-02T22:09:17Z

Sure:

first commit: avoid results duplicates when performing multiple queries at once
second commit: improve alias rendering (so users can easily see that it's not a "normal" result)
third commit: add description in aliases results (I noticed it wasn't there so I added it back)
fourth commit: now, the focus/blur on the themes switch buttons is way better (when clicking outside, the menu closes itself, etc...)

QuietMisdreavus

Looks good! Just have a couple clarifying questions.

QuietMisdreavus · 2018-05-04T19:25:38Z

src/librustdoc/html/static/main.js

+        }
+
+        function pathSplitter(path) {
+            return '<span>' + path.replace(/::/g, '::</span><span>');


Is there a problem if this function opens a <span> tag without closing it?

Good catch. It's an old bug in addition of that!

QuietMisdreavus · 2018-05-04T19:34:50Z

src/librustdoc/html/static/main.js

@@ -1362,19 +1381,23 @@
                function mergeArrays(arrays) {
                    var ret = [];
                    var positions = [];
+                    var notDuplicates = {};


What's with the split between duplicates above and notDuplicates here?

Explanation time! 😄

mergeArrays is only used when we have multiple queries (for example: "str,[]"). The second one (the one using duplicates) allows to remove more global duplicates and is called on every search.

That makes sense, but why the inversion? Why is one duplicates and one notDuplicates?

I have no explanation for this... I didn't write the two functions at the same time so I wrote both without thinking to the other.

GuillaumeGomez · 2018-05-05T15:54:41Z

My last commit reduces the size of aliases.js and search-index.js (for the second we go from 3458462 to 2993110).

rust-highfive · 2018-05-05T17:16:34Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

travis_time:start:test_rustdoc
Check compiletest suite=rustdoc mode=rustdoc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:08:20] 
[01:08:20] running 231 tests
[01:09:37] ...F...............i.....................................................F..........................
[01:10:22] .........i...........................................................F.........................F....
[01:10:31] failures:
[01:10:31] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:489:22
[01:10:31] 
[01:10:31] ---- [rustdoc] rustdoc/assoc-consts.rs stdout ----
[01:10:31] ---- [rustdoc] rustdoc/assoc-consts.rs stdout ----
[01:10:31]  
[01:10:31] error: htmldocck failed!
[01:10:31] status: exit code: 1
[01:10:31] command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/assoc-consts.stage2-x86_64-unknown-linux-gnu" "/checkout/src/test/rustdoc/assoc-consts.rs"
[01:10:31] ------------------------------------------
[01:10:31] 
[01:10:31] ------------------------------------------
[01:10:31] stderr:
[01:10:31] stderr:
[01:10:31] ------------------------------------------
[01:10:31] 114: @has check failed
[01:10:31]  `XPATH PATTERN` did not match
[01:10:31]      // @has - '//*[@class="docblock"]' "Docs for QUX_DEFAULT0 in trait."
[01:10:31] 123: @has check failed
[01:10:31]  `XPATH PATTERN` did not match
[01:10:31]      // @has - '//*[@class="docblock"]' "Docs for QUX_DEFAULT2 in trait."
[01:10:31] Encountered 2 errors
[01:10:31] 
[01:10:31] ------------------------------------------
[01:10:31] 
[01:10:31] 
[01:10:31] thread '[rustdoc] rustdoc/assoc-consts.rs' panicked at 'explicit panic', tools/co01:10:31] thread '[rustdoc] rustdoc/issue-12834.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3001:9
[01:10:31] ---- [rustdoc] rustdoc/manual_impl.rs stdout ----
[01:10:31]  
[01:10:31]  
[01:10:31] error: htmldocck failed!
[01:10:31] status: exit code: 1
[01:10:31] command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/manual_impl.stage2-x86_64-unknown-linux-gnu" "/checkout/src/test/rustdoc/manual_impl.rs"
[01:10:31] ------------------------------------------
[01:10:31] 
[01:10:31] ------------------------------------------
[01:10:31] stderr:
[01:10:31] stderr:
[01:10:31] ------------------------------------------
[01:10:31] 37: @has check failed
[01:10:31]  `XPATH PATTERN` did not match
[01:10:31]  // @has - '//*[@class="docblock"]' 'Docs associated with the trait b_method definition.'
[01:10:31] 38: @has check failed
[01:10:31]  `XPATH PATTERN` did not match
[01:10:31]  // @has - '//*[@class="docblock"]' 'Docs associated with the trait b_method definition.'
[01:10:31] 39: @has check failed
[01:10:31]  `XPATH PATTERN` did not match
[01:10:31]  // @has - '//*[@class="docblock"]' 'Docs associated with the trait c_method definition.'
[01:10:31] 58: @has check failed
[01:10:31]  `XPATH PATTERN` did not match
[01:10:31]  // @has - '//*[@class="docblock"]' 'Docs associated with the trait b_method definition.'
[01:10:31] 78: @has check failed
[01:10:31]  `XPATH PATTERN` did not match
[01:10:31]  // @has - '//*[@class="docblock"]' 'Docs associated with the trait a_method definition.'
[01:10:31] Encountered 5 errors
[01:10:31] 
[01:10:31] ------------------------------------------
[01:10:31] 
[01:10:31] 
[01:10:31] thread '[rustdoc] rustdoc/manual_impl.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3001:9
[01:10:31] 
[01:10:31] ---- [rustdoc] rustdoc/search-index-summaries.rs stdout ----
[01:10:31]  
[01:10:31] error: htmldocck failed!
[01:10:31] status: exit code: 1
[01:10:31] command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/search-index-summaries.stage2-x86_64-unknown-linux-gnu" "/checkout/src/test/rustdoc/search-index-summaries.rs"
[01:10:31] ------------------------------------------
[01:10:31] 
[01:10:31] ------------------------------------------
[01:10:31] stderr:
[01:10:31] stderr:
[01:10:31] ------------------------------------------
[01:10:31] 13: @has check failed
[01:10:31]  `PATTERN` did not match
[01:10:31]  // @has 'search-index.js' 'Foo short link.'
[01:10:31] Encountered 1 errors
[01:10:31] 
[01:10:31] ------------------------------------------
[01:10:31] 
---
[01:10:31] test result: FAILED. 225 passed; 4 failed; 2 ignored; 0 measured; 0 filtered out
[01:10:31] 
[01:10:31] 
[01:10:31] 
[01:10:31] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--rustdoc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "--src-base" "/checkout/src/test/rustdoc" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "rustdoc" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:10:31] 
[01:10:31] 
[01:10:31] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:10:31] Build completed unsuccessfully in 0:26:33
[01:10:31] Build completed unsuccessfully in 0:26:33
[01:10:31] make: *** [check] Error 1
[01:10:31] Makefile:58: recipe for target 'check' failed
travis_time:end:091031ca:start=15255363579409 find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
travis_time:end:0fc71328:start=1525540591226231257,finish=1525540591232662503,duration=6431246
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:09e61c96

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

QuietMisdreavus · 2018-05-07T20:10:23Z

src/librustdoc/html/static/main.js

+            if (tmp.endsWith("<span>")) {
+                return tmp.slice(0, tmp.length - 6);
+            }
+            return tmp;


What about situations where the string doesn't end in <span>? Doesn't that still have an unclosed span?

No, it always ends with ::. This is on this string that we make the replacement.

ollie27 · 2018-05-10T22:53:53Z

src/librustdoc/html/static/main.js

        function addTab(array, query, display) {
            var extraStyle = '';
            if (display === false) {
                extraStyle = ' style="display: none;"';
            }

            var output = '';
+            var duplicates = {};


When can duplicates occur? Do you have some examples of when this is needed? The shown variable below is also trying to remove duplicates and I've tried to remove it in #50482 but if there are duplicates then we'll need to find a better way of dealing with them. addTab definitely isn't the correct place to be removing duplicates because it means the search result count will be incorrect: https://doc.rust-lang.org/nightly/std/?search=is_nan.

It can occurs in case you have aliases. I need to fix the count then.

Can this de-duplication happen inside execSearch so it can be tested by rustdoc-js?

If I did this, it'd force the js to go through the array one more time. I'm not sure we'd want this...

ollie27

I'd like to see tests for queries that could create duplicate results.

Do you plan to backport this PR to 1.27?

ollie27 · 2018-05-12T14:45:45Z

src/librustdoc/html/static/main.js

        function addTab(array, query, display) {
            var extraStyle = '';
            if (display === false) {
                extraStyle = ' style="display: none;"';
            }

            var output = '';
+            var duplicates = {};


Can this de-duplication happen inside execSearch so it can be tested by rustdoc-js?

ollie27 · 2018-05-12T14:47:14Z

src/librustdoc/html/static/main.js

-                    var href = res[1];
-                    var displayPath = res[0];
+                    if (item.is_alias !== true) {
+                        var fullPath = item.displayPath + name;


Can't this use item.fullPath?

ollie27 · 2018-05-12T15:05:24Z

src/librustdoc/html/render.rs

-        format!("{{'crate':'{}','ty':{},'name':'{}','path':'{}'{}}}",
-                krate, item.ty as usize, item.name, item.path,
+        format!("{{'crate':'{}','ty':{},'name':'{}','desc':'{}','p':'{}'{}}}",
+                krate, item.ty as usize, item.name, item.desc.replace("'", "\\'"), item.path,


.replace("'", "\\'") isn't how you escape strings for javascript. You need to use something like as_json.

Bad surprise, quotes aren't escaped when using Json object or json conversion functions so for now, it's the best solution.

QuietMisdreavus · 2018-05-13T21:37:13Z

@bors delegate=ollie27

There’s enough going on here that i’m not too sure what each part is doing, especially since new things have been added into the PR since it’s been opened. On its face, it all seems fine, but i’m way less comfortable with Javascript than Rust, so i could be missing something.

bors · 2018-05-13T21:37:14Z

✌️ @ollie27 can now approve this pull request

ollie27 · 2018-05-14T21:09:43Z

Let's get this in.

@bors r+

bors · 2018-05-14T21:09:44Z

📌 Commit f0db2cf has been approved by ollie27

bors · 2018-05-15T05:39:42Z

⌛ Testing commit f0db2cf with merge d5ecf70...

@QuietMisdreavus

Rustdoc improvements Fixes #50658. (last commit) A lot of small improvements. r? @QuietMisdreavus

bors · 2018-05-15T08:19:01Z

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

rust-highfive assigned QuietMisdreavus Apr 26, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 26, 2018

GuillaumeGomez force-pushed the improve-results branch from 26380d3 to 7b6f80f Compare April 28, 2018 14:38

QuietMisdreavus reviewed May 4, 2018

View reviewed changes

GuillaumeGomez force-pushed the improve-results branch from 7b6f80f to ccc3562 Compare May 5, 2018 15:10

GuillaumeGomez force-pushed the improve-results branch from 593f9ae to a50b7e7 Compare May 5, 2018 16:03

GuillaumeGomez force-pushed the improve-results branch from a50b7e7 to 27c83a1 Compare May 5, 2018 17:34

QuietMisdreavus reviewed May 7, 2018

View reviewed changes

ollie27 reviewed May 10, 2018

View reviewed changes

ollie27 reviewed May 12, 2018

View reviewed changes

GuillaumeGomez added 9 commits May 12, 2018 19:16

Improve results

cf9209f

Improve alias rendering

e81b59b

Add missing description in alias items

8c80124

Handle blur on theme buttons nicely

9483e9b

Remove extra span tags

72b7c8d

Reduce js files size

6a4bdda

Fix items counts in search results

8fab482

Fix invalid deduplication

f2bc09e

Use fullpath instead of recreating it

f0db2cf

GuillaumeGomez force-pushed the improve-results branch from 7d7399f to f0db2cf Compare May 12, 2018 21:45

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 14, 2018

bors added a commit that referenced this pull request May 15, 2018

Auto merge of #50259 - GuillaumeGomez:improve-results, r=ollie27

d5ecf70

Rustdoc improvements Fixes #50658. (last commit) A lot of small improvements. r? @QuietMisdreavus

bors merged commit f0db2cf into rust-lang:master May 15, 2018

GuillaumeGomez deleted the improve-results branch May 15, 2018 08:40

ollie27 mentioned this pull request Jun 16, 2018

panic, env, and vec macros missing from std API doc search results #51095

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rustdoc improvements #50259

Rustdoc improvements #50259

GuillaumeGomez commented Apr 26, 2018 •

edited

Loading

QuietMisdreavus commented Apr 26, 2018

GuillaumeGomez commented Apr 27, 2018

GuillaumeGomez commented Apr 28, 2018

QuietMisdreavus commented May 2, 2018

GuillaumeGomez commented May 2, 2018

QuietMisdreavus left a comment

QuietMisdreavus May 4, 2018

GuillaumeGomez May 5, 2018

QuietMisdreavus May 4, 2018

GuillaumeGomez May 5, 2018

QuietMisdreavus May 7, 2018

GuillaumeGomez May 8, 2018

GuillaumeGomez commented May 5, 2018 •

edited

Loading

rust-highfive commented May 5, 2018

QuietMisdreavus May 7, 2018

GuillaumeGomez May 7, 2018

ollie27 May 10, 2018

GuillaumeGomez May 11, 2018

ollie27 May 12, 2018

GuillaumeGomez May 12, 2018

ollie27 left a comment

ollie27 May 12, 2018

ollie27 May 12, 2018

ollie27 May 12, 2018

GuillaumeGomez May 12, 2018

QuietMisdreavus commented May 13, 2018

bors commented May 13, 2018

ollie27 commented May 14, 2018

bors commented May 14, 2018

bors commented May 15, 2018

bors commented May 15, 2018

Rustdoc improvements #50259

Rustdoc improvements #50259

Conversation

GuillaumeGomez commented Apr 26, 2018 • edited Loading

QuietMisdreavus commented Apr 26, 2018

GuillaumeGomez commented Apr 27, 2018

GuillaumeGomez commented Apr 28, 2018

QuietMisdreavus commented May 2, 2018

GuillaumeGomez commented May 2, 2018

QuietMisdreavus 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

GuillaumeGomez commented May 5, 2018 • edited Loading

rust-highfive commented May 5, 2018

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

ollie27 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

QuietMisdreavus commented May 13, 2018

bors commented May 13, 2018

ollie27 commented May 14, 2018

bors commented May 14, 2018

bors commented May 15, 2018

bors commented May 15, 2018

GuillaumeGomez commented Apr 26, 2018 •

edited

Loading

GuillaumeGomez commented May 5, 2018 •

edited

Loading