Auto merge of #84834 - GuillaumeGomez:sidebar-unification, r=jsha

bors · bors · commit 19579c656436 · 2021-06-03T02:31:44.000Z
Sidebar unification

This PR does a few things:
 * Put crates list at all levels (before, it was only on the "top" items)
 * Fix bug in module sidebar: the list of items was from the parent module.

The other changes (on bootstrap mostly) were to allow to generate multiple crates in a same folder so that we can ensure that clicking on the crates in the sidebar works as expected.

I added a rustdoc-gui test to ensure everything is where it should be.

r? `@jyn514`
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
@@ -526,7 +526,6 @@ impl Item {
     crate fn is_crate(&self) -> bool {
         self.is_mod() && self.def_id.as_real().map_or(false, |did| did.index == CRATE_DEF_INDEX)
     }
-
     crate fn is_mod(&self) -> bool {
         self.type_() == ItemType::Module
     }
diff --git a/src/librustdoc/html/layout.rs b/src/librustdoc/html/layout.rs
@@ -119,7 +119,7 @@ crate fn render<T: Print, S: Print>(
     {after_content}\
     <div id=\"rustdoc-vars\" data-root-path=\"{root_path}\" data-current-crate=\"{krate}\" \
        data-search-index-js=\"{root_path}search-index{suffix}.js\" \
-       data-search-js=\"{static_root_path}search{suffix}.js\"></div>
+       data-search-js=\"{static_root_path}search{suffix}.js\"></div>\
     <script src=\"{static_root_path}main{suffix}.js\"></script>\
     {extra_scripts}\
 </body>\
diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs
@@ -200,8 +200,15 @@ impl<'tcx> Context<'tcx> {
             )
         };
         let keywords = make_item_keywords(it);
+        let name;
+        let tyname_s = if it.is_crate() {
+            name = format!("{} crate", tyname);
+            name.as_str()
+        } else {
+            tyname.as_str()
+        };
         let page = layout::Page {
-            css_class: tyname.as_str(),
+            css_class: tyname_s,
             root_path: &self.root_path(),
             static_root_path: self.shared.static_root_path.as_deref(),
             title: &title,
diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
@@ -1713,7 +1713,7 @@ fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer) {
                 "<div class=\"block version\">\
                      <p>Version {}</p>\
                  </div>",
-                Escape(version)
+                Escape(version),
             );
         }
     }
@@ -1723,9 +1723,10 @@ fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer) {
         write!(
             buffer,
             "<a id=\"all-types\" href=\"all.html\"><p>See all {}'s items</p></a>",
-            it.name.as_ref().expect("crates always have a name")
+            it.name.as_ref().expect("crates always have a name"),
         );
     }
+
     match *it.kind {
         clean::StructItem(ref s) => sidebar_struct(cx, buffer, it, s),
         clean::TraitItem(ref t) => sidebar_trait(cx, buffer, it, t),
@@ -1735,7 +1736,7 @@ fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer) {
         clean::TypedefItem(_, _) => sidebar_typedef(cx, buffer, it),
         clean::ModuleItem(ref m) => sidebar_module(buffer, &m.items),
         clean::ForeignTypeItem => sidebar_foreign_type(cx, buffer, it),
-        _ => (),
+        _ => {}
     }
 
     // The sidebar is designed to display sibling functions, modules and
@@ -1746,22 +1747,24 @@ fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer) {
     // as much HTML as possible in order to allow non-JS-enabled browsers
     // to navigate the documentation (though slightly inefficiently).
 
-    buffer.write_str("<p class=\"location\">");
-    for (i, name) in cx.current.iter().take(parentlen).enumerate() {
-        if i > 0 {
-            buffer.write_str("::<wbr>");
+    if !it.is_mod() {
+        buffer.write_str("<p class=\"location\">Other items in<br>");
+        for (i, name) in cx.current.iter().take(parentlen).enumerate() {
+            if i > 0 {
+                buffer.write_str("::<wbr>");
+            }
+            write!(
+                buffer,
+                "<a href=\"{}index.html\">{}</a>",
+                &cx.root_path()[..(cx.current.len() - i - 1) * 3],
+                *name
+            );
         }
-        write!(
-            buffer,
-            "<a href=\"{}index.html\">{}</a>",
-            &cx.root_path()[..(cx.current.len() - i - 1) * 3],
-            *name
-        );
+        buffer.write_str("</p>");
     }
-    buffer.write_str("</p>");
 
     // Sidebar refers to the enclosing module, not this module.
-    let relpath = if it.is_mod() { "../" } else { "" };
+    let relpath = if it.is_mod() && parentlen != 0 { "./" } else { "" };
     write!(
         buffer,
         "<div id=\"sidebar-vars\" data-name=\"{name}\" data-ty=\"{ty}\" data-relpath=\"{path}\">\
@@ -1770,17 +1773,7 @@ fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer) {
         ty = it.type_(),
         path = relpath
     );
-
-    if parentlen == 0 {
-        write!(
-            buffer,
-            "<script defer src=\"{}sidebar-items{}.js\"></script>",
-            relpath, cx.shared.resource_suffix
-        );
-    } else {
-        write!(buffer, "<script defer src=\"{}sidebar-items.js\"></script>", relpath);
-    }
-
+    write!(buffer, "<script defer src=\"{}sidebar-items.js\"></script>", relpath);
     // Closes sidebar-elems div.
     buffer.write_str("</div>");
 }
@@ -2288,8 +2281,8 @@ fn sidebar_enum(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item, e: &clean:
     }
 }
 
-fn item_ty_to_strs(ty: &ItemType) -> (&'static str, &'static str) {
-    match *ty {
+fn item_ty_to_strs(ty: ItemType) -> (&'static str, &'static str) {
+    match ty {
         ItemType::ExternCrate | ItemType::Import => ("reexports", "Re-exports"),
         ItemType::Module => ("modules", "Modules"),
         ItemType::Struct => ("structs", "Structs"),
@@ -2321,10 +2314,14 @@ fn item_ty_to_strs(ty: &ItemType) -> (&'static str, &'static str) {
 fn sidebar_module(buf: &mut Buffer, items: &[clean::Item]) {
     let mut sidebar = String::new();
 
+    // Re-exports are handled a bit differently because they can be extern crates or imports.
     if items.iter().any(|it| {
-        it.type_() == ItemType::ExternCrate || (it.type_() == ItemType::Import && !it.is_stripped())
+        it.name.is_some()
+            && (it.type_() == ItemType::ExternCrate
+                || (it.type_() == ItemType::Import && !it.is_stripped()))
     }) {
-        sidebar.push_str("<li><a href=\"#reexports\">Re-exports</a></li>");
+        let (id, name) = item_ty_to_strs(ItemType::Import);
+        sidebar.push_str(&format!("<li><a href=\"#{}\">{}</a></li>", id, name));
     }
 
     // ordering taken from item_module, reorder, where it prioritized elements in a certain order
@@ -2351,13 +2348,9 @@ fn sidebar_module(buf: &mut Buffer, items: &[clean::Item]) {
         ItemType::ForeignType,
         ItemType::Keyword,
     ] {
-        if items.iter().any(|it| !it.is_stripped() && it.type_() == myty) {
-            let (short, name) = item_ty_to_strs(&myty);
-            sidebar.push_str(&format!(
-                "<li><a href=\"#{id}\">{name}</a></li>",
-                id = short,
-                name = name
-            ));
+        if items.iter().any(|it| !it.is_stripped() && it.type_() == myty && it.name.is_some()) {
+            let (id, name) = item_ty_to_strs(myty);
+            sidebar.push_str(&format!("<li><a href=\"#{}\">{}</a></li>", id, name));
         }
     }
 
diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs
@@ -263,7 +263,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
                 w.write_str("</table>");
             }
             curty = myty;
-            let (short, name) = item_ty_to_strs(&myty.unwrap());
+            let (short, name) = item_ty_to_strs(myty.unwrap());
             write!(
                 w,
                 "<h2 id=\"{id}\" class=\"section-header\">\
diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs
@@ -227,7 +227,6 @@ pub(super) fn write_shared(
     )?;
     write_minify("search.js", static_files::SEARCH_JS)?;
     write_minify("settings.js", static_files::SETTINGS_JS)?;
-    write_minify("sidebar-items.js", static_files::sidebar::ITEMS)?;
 
     if cx.shared.include_sources {
         write_minify("source-script.js", static_files::sidebar::SOURCE_SCRIPT)?;
diff --git a/src/librustdoc/html/static/main.js b/src/librustdoc/html/static/main.js
@@ -561,41 +561,40 @@ function hideThemeButtonState() {
         }
     }());
 
-    function addSidebarCrates(crates) {
-        // Draw a convenient sidebar of known crates if we have a listing
-        if (window.rootPath === "../" || window.rootPath === "./") {
-            var sidebar = document.getElementsByClassName("sidebar-elems")[0];
-            if (sidebar) {
-                var div = document.createElement("div");
-                div.className = "block crate";
-                div.innerHTML = "<h3>Crates</h3>";
-                var ul = document.createElement("ul");
-                div.appendChild(ul);
-
-                for (var i = 0; i < crates.length; ++i) {
-                    var klass = "crate";
-                    if (window.rootPath !== "./" && crates[i] === window.currentCrate) {
-                        klass += " current";
-                    }
-                    var link = document.createElement("a");
-                    link.href = window.rootPath + crates[i] + "/index.html";
-                    link.className = klass;
-                    link.textContent = crates[i];
-
-                    var li = document.createElement("li");
-                    li.appendChild(link);
-                    ul.appendChild(li);
-                }
-                sidebar.appendChild(div);
-            }
-        }
-    }
-
     // delayed sidebar rendering.
     window.initSidebarItems = function(items) {
         var sidebar = document.getElementsByClassName("sidebar-elems")[0];
         var current = window.sidebarCurrent;
 
+        function addSidebarCrates(crates) {
+            if (!hasClass(document.body, "crate")) {
+                // We only want to list crates on the crate page.
+                return;
+            }
+            // Draw a convenient sidebar of known crates if we have a listing
+            var div = document.createElement("div");
+            div.className = "block crate";
+            div.innerHTML = "<h3>Crates</h3>";
+            var ul = document.createElement("ul");
+            div.appendChild(ul);
+
+            for (var i = 0; i < crates.length; ++i) {
+                var klass = "crate";
+                if (window.rootPath !== "./" && crates[i] === window.currentCrate) {
+                    klass += " current";
+                }
+                var link = document.createElement("a");
+                link.href = window.rootPath + crates[i] + "/index.html";
+                link.className = klass;
+                link.textContent = crates[i];
+
+                var li = document.createElement("li");
+                li.appendChild(link);
+                ul.appendChild(li);
+            }
+            sidebar.appendChild(div);
+        }
+
         function block(shortty, longty) {
             var filtered = items[shortty];
             if (!filtered) {
@@ -634,28 +633,32 @@ function hideThemeButtonState() {
                 ul.appendChild(li);
             }
             div.appendChild(ul);
-            if (sidebar) {
-                sidebar.appendChild(div);
-            }
+            sidebar.appendChild(div);
         }
 
-        block("primitive", "Primitive Types");
-        block("mod", "Modules");
-        block("macro", "Macros");
-        block("struct", "Structs");
-        block("enum", "Enums");
-        block("union", "Unions");
-        block("constant", "Constants");
-        block("static", "Statics");
-        block("trait", "Traits");
-        block("fn", "Functions");
-        block("type", "Type Definitions");
-        block("foreigntype", "Foreign Types");
-        block("keyword", "Keywords");
-        block("traitalias", "Trait Aliases");
-
-        // `crates{version}.js` should always be loaded before this script, so we can use it safely.
-        addSidebarCrates(window.ALL_CRATES);
+        if (sidebar) {
+            var isModule = hasClass(document.body, "mod");
+            if (!isModule) {
+                block("primitive", "Primitive Types");
+                block("mod", "Modules");
+                block("macro", "Macros");
+                block("struct", "Structs");
+                block("enum", "Enums");
+                block("union", "Unions");
+                block("constant", "Constants");
+                block("static", "Statics");
+                block("trait", "Traits");
+                block("fn", "Functions");
+                block("type", "Type Definitions");
+                block("foreigntype", "Foreign Types");
+                block("keyword", "Keywords");
+                block("traitalias", "Trait Aliases");
+            }
+
+            // `crates{version}.js` should always be loaded before this script, so we can use
+            // it safely.
+            addSidebarCrates(window.ALL_CRATES);
+        }
     };
 
     window.register_implementors = function(imp) {
diff --git a/src/librustdoc/html/static/rustdoc.css b/src/librustdoc/html/static/rustdoc.css
@@ -311,9 +311,12 @@ nav.sub {
 	border: none;
 }
 
-.location a:first-child {
+.location a:first-of-type {
 	font-weight: 500;
 }
+.location a:hover {
+	text-decoration: underline;
+}
 
 .block {
 	padding: 0;
diff --git a/src/librustdoc/html/static/sidebar-items.js b/src/librustdoc/html/static/sidebar-items.js
diff --git a/src/librustdoc/html/static_files.rs b/src/librustdoc/html/static_files.rs
@@ -141,7 +141,4 @@ crate mod noto_sans_kr {
 crate mod sidebar {
     /// File script to handle sidebar.
     crate static SOURCE_SCRIPT: &str = include_str!("static/source-script.js");
-
-    /// Top Level sidebar items script which will load a sidebar without items.
-    crate static ITEMS: &str = include_str!("static/sidebar-items.js");
 }
diff --git a/src/test/rustdoc-gui/sidebar.goml b/src/test/rustdoc-gui/sidebar.goml
@@ -0,0 +1,56 @@
+goto: file://|DOC_PATH|/test_docs/index.html
+assert: (".sidebar > .location", "Crate test_docs")
+// In modules, we only have one "location" element.
+assert: (".sidebar .location", 1)
+assert: (".sidebar-elems > #all-types", "See all test_docs's items")
+// We check that we have the crates list and that the "current" on is "test_docs".
+assert: (".sidebar-elems > .crate > ul > li > a.current", "test_docs")
+// And we're also supposed to have the list of items in the current module.
+assert: (".sidebar-elems > .items > ul > li:nth-child(1)", "Modules")
+assert: (".sidebar-elems > .items > ul > li:nth-child(2)", "Structs")
+assert: (".sidebar-elems > .items > ul > li:nth-child(3)", "Enums")
+assert: (".sidebar-elems > .items > ul > li:nth-child(4)", "Traits")
+assert: (".sidebar-elems > .items > ul > li:nth-child(5)", "Functions")
+assert: (".sidebar-elems > .items > ul > li:nth-child(6)", "Keywords")
+assert: ("#structs + table td > a", "Foo")
+click: "#structs + table td > a"
+
+// PAGE: struct.Foo.html
+assert: (".sidebar .location", 2)
+// We check that there is no crate listed outside of the top level.
+assert-false: ".sidebar-elems > .crate"
+// We now go back to the crate page to click on the "lib2" crate link.
+goto: file://|DOC_PATH|/test_docs/index.html
+click: ".sidebar-elems > .crate > ul > li:first-child > a"
+
+// PAGE: lib2/index.html
+goto: file://|DOC_PATH|/lib2/index.html
+assert: (".sidebar > .location", "Crate lib2")
+// We check that we have the crates list and that the "current" on is now "lib2".
+assert: (".sidebar-elems > .crate > ul > li > a.current", "lib2")
+// We now go to the "foobar" function page.
+assert: (".sidebar-elems > .items > ul > li:nth-child(1)", "Modules")
+assert: (".sidebar-elems > .items > ul > li:nth-child(2)", "Functions")
+assert:	("#functions + table td > a", "foobar")
+click: "#functions + table td > a"
+
+// PAGE: fn.foobar.html
+// In items containing no items (like functions or constants) and in modules, we have one
+// "location" elements.
+assert: (".sidebar .location", 1)
+// There is a "<br>" tag between "in" and "lib2", but it doesn't count as a space.
+assert: (".sidebar .sidebar-elems .location", "Other items inlib2")
+// We check that we don't have the crate list.
+assert-false: ".sidebar-elems > .crate"
+
+goto: ./module/index.html
+assert: (".sidebar > .location", "Module module")
+// We check that we don't have the crate list.
+assert-false: ".sidebar-elems > .crate"
+
+goto: ./sub_module/sub_sub_module/index.html
+assert: (".sidebar > .location", "Module sub_sub_module")
+// We check that we don't have the crate list.
+assert-false: ".sidebar-elems > .crate"
+assert: (".sidebar-elems > .items > ul > li:nth-child(1)", "Functions")
+assert: ("#functions + table td > a", "foo")
diff --git a/src/test/rustdoc-gui/src/lib2.rs b/src/test/rustdoc-gui/src/lib2.rs
@@ -0,0 +1,11 @@
+pub mod module {
+    pub mod sub_module {
+        pub mod sub_sub_module {
+            pub fn foo() {}
+        }
+        pub fn bar() {}
+    }
+    pub fn whatever() {}
+}
+
+pub fn foobar() {}
diff --git a/src/test/rustdoc/keyword.rs b/src/test/rustdoc/keyword.rs
diff --git a/src/tools/rustdoc-gui/tester.js b/src/tools/rustdoc-gui/tester.js

Original file line number	Diff line number	Diff line change
`@@ -526,7 +526,6 @@ impl Item {`
`526`	`526`	`crate fn is_crate(&self) -> bool {`
`527`	`527`	`self.is_mod() && self.def_id.as_real().map_or(false, \|did\| did.index == CRATE_DEF_INDEX)`
`528`	`528`	`}`
`529`		`-`
`530`	`529`	`crate fn is_mod(&self) -> bool {`
`531`	`530`	`self.type_() == ItemType::Module`
`532`	`531`	`}`
Original file line number	Diff line number	Diff line change
`@@ -263,7 +263,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl`
`263`	`263`	`w.write_str("</table>");`
`264`	`264`	`}`
`265`	`265`	`curty = myty;`
`266`		`- let (short, name) = item_ty_to_strs(&myty.unwrap());`
	`266`	`+ let (short, name) = item_ty_to_strs(myty.unwrap());`
`267`	`267`	`write!(`
`268`	`268`	`w,`
`269`	`269`	`"<h2 id=\"{id}\" class=\"section-header\">\`
Original file line number	Diff line number	Diff line change
`@@ -311,9 +311,12 @@ nav.sub {`
`311`	`311`	`border: none;`
`312`	`312`	`}`
`313`	`313`
`314`		`-.location a:first-child {`
	`314`	`+.location a:first-of-type {`
`315`	`315`	`font-weight: 500;`
`316`	`316`	`}`
	`317`	`+.location a:hover {`
	`318`	`+ text-decoration: underline;`
	`319`	`+}`
`317`	`320`
`318`	`321`	`.block {`
`319`	`322`	`padding: 0;`
Original file line number	Diff line number	Diff line change
`@@ -141,7 +141,4 @@ crate mod noto_sans_kr {`
`141`	`141`	`crate mod sidebar {`
`142`	`142`	`/// File script to handle sidebar.`
`143`	`143`	`crate static SOURCE_SCRIPT: &str = include_str!("static/source-script.js");`
`144`		`-`
`145`		`- /// Top Level sidebar items script which will load a sidebar without items.`
`146`		`- crate static ITEMS: &str = include_str!("static/sidebar-items.js");`
`147`	`144`	`}`