rustdoc: Call `initSidebarItems` in root module of crate #85304

Stupremee · 2021-05-14T20:48:24Z

Resolves #85301

jsha · 2021-05-14T21:00:44Z

src/librustdoc/html/render/mod.rs

    if parentlen == 0 {
-        // There is no sidebar-items.js beyond the crate root path
-        // FIXME maybe dynamic crate loading can be merged here
+        write!(buffer, "<script defer src=\"top-level-sidebar-items.js\"></script>",);
    } else {
        write!(buffer, "<script defer src=\"{path}sidebar-items.js\"></script>", path = relpath);
    }


I think what we really want here, rather than generating a file with a different name, is to generate a file named sidebar-items.js in the doc root, and unconditionally write a script tag that loads it. So these two if branches would get collapsed into one.

That's better yeah

jsha · 2021-05-14T21:01:32Z

src/librustdoc/html/render/context.rs

            let js_dst = self.dst.join("sidebar-items.js");
            let v = format!("initSidebarItems({});", serde_json::to_string(&items).unwrap());
            scx.fs.write(&js_dst, &v)?;
+
+            let js_dst = self.dst.join("top-level-sidebar-items.js");
+            scx.fs.write(&js_dst, "initSidebarItems({});")?;


I'm a little confused... reading the code above this: if the added code works to add a file at the top level, there should already be a sidebar-items.js at the top level.

the top-level does not mean the top-level of the generated documentation, but is just an alternative name to distinguish between initSidebarItems({}) and initSidebarItems({ /* some items */})

jsha · 2021-05-14T21:06:33Z

Specifically by "top level" I'm thinking of e.g. https://doc.rust-lang.org/std/. But I'm now realizing there's also a per-module index.html that we need to fix up, and that's what you've fixed up so far. Am I on the right track here?

And we need to generate a separate top-level-sidebar-items.js because for the per-module index.html, we specifically don't want to list all the extra stuff that's in the real sidebar-items.js?

Stupremee · 2021-05-14T21:14:13Z

No, the index.html that changes, is now the root-module, aka the crate itself (target/doc/mycrate/index.html). https://doc.rust-lang.org/std/ is just a smart redirect of the web server and is the same as https://doc.rust-lang.org/std/index.html. Inside other modules we do not need to care, because inside a module the crates are not even shown, instead all items and stuff.

I think the new sidebar-items.js can also be put into the root of the documentation (target/doc/sidebar-items.js) because the root modules are created at target/doc/std so adding a ../ to the path will lead to target/doc/sidebar-items.js. This would also only create one file per doc build instead of putting a new file into every crate.

Stupremee · 2021-05-14T21:20:55Z

I pushed the changes that I meant in my last message. Maybe that helps to understand.

jsha · 2021-05-14T23:04:21Z

This is looking good! But when I build locally and visit file:///home/jsha/rust/rust/build/x86_64-unknown-linux-gnu/doc/std/index.html, I get a JS console error:

Loading failed for the <script> with source “file:///home/jsha/rust/rust/build/x86_64-unknown-linux-gnu/doc/sidebar-items.js”.

And indeed that file doesn't exist. But build/x86_64-unknown-linux-gnu/doc/sidebar-items1.54.0.js does exist.

Stupremee · 2021-05-15T07:35:46Z

Does write_minify can append a toolchain version to the file? ~~I can't reproduce it locally but that was my guess~~ when building std I can reproduce

Stupremee · 2021-05-15T08:27:38Z

Apparently, it does. Should the file just be written as an unversichert file or adjust the script tag to include the version?

Nemo157 · 2021-05-15T08:30:18Z

It needs the version number if it's a cross-crate shared file (mostly for docs.rs and doc.rust-lang.org purposes).

Stupremee · 2021-05-15T09:37:04Z

Ah, that makes sense. Do you think the sidebar-items.js should be toolchain specific? I would assume yes because then it can change between rustdoc versions without breaking existing docs, right?

GuillaumeGomez · 2021-05-15T15:27:47Z

Strangely, your fix looks very different than the one I wrote in 4e22b32

jsha · 2021-05-15T16:31:01Z

I think sidebar-items.js should not have a version number at the crate root, because it doesn't have a version number in other places, and it would confusing.

@GuillaumeGomez sorry I didn't realize there was already a fix in progress. I think we should plan on landing this one to fix the regression since it's much smaller.

jsha · 2021-05-15T17:07:15Z

Looks good to me in a local test. Thanks for working so quickly to fix.

@bors r+

bors · 2021-05-15T17:07:17Z

📌 Commit b8c546862c4277b2a3259f662dda712626c2e9b0 has been approved by jsha

Nemo157 · 2021-05-15T17:14:21Z

If it's going to be unversioned, then it needs to be not in the shared files, otherwise docs.rs will have issues if the JS API ever changes. docs.rs can handle unversioned unshared files at the root of the doc-folder now, ~~but I don't think doc.rust-lang.org can just by looking at the existing docs it hosts~~ EDIT: Actually, it can, it fully segregates the files.

jsha · 2021-05-15T17:17:45Z

@bors r-

Stupremee · 2021-05-15T17:18:06Z

Alright. Then I will just revert my last commit!

Stupremee · 2021-05-15T17:20:45Z

I messed up the rebase. Whoops

Stupremee · 2021-05-15T18:20:50Z

It does not add a new file at every module 🤔
Only one shared sidebar-items.js which is loaded by the root module.
The sidebar-items.js file that is added in this PR, is just a way to call initSidebarItems at the root level, by loading it using a <script> tag

Nemo157 · 2021-05-15T18:21:59Z

This approach adds a single new shared file, not even per-crate. This seems by far the simplest minimal change I can see to get backported onto beta to fix it there. It may be possible to simplify it as part of #84834 or other fixes in nightly, but we need to get something that can be backported first.

GuillaumeGomez · 2021-05-15T18:22:54Z

I can make #84834 much shorter by only putting back the crate list if you want in another PR. But I'm strongly against adding a new unneeded file.

Stupremee · 2021-05-15T18:24:59Z

I can also change this PR to use another fix. I just thought this is an easy way to call the initSidebarItems function and it would be fine.

GuillaumeGomez · 2021-05-15T18:26:26Z

@Stupremee: Sorry that you're in the middle of this. :-/ I should have included @jsha on my other PR's discussion, that would have prevented all the problems in here...

Nemo157 · 2021-05-15T18:30:05Z

The only way I can see to do this without a new file (or some more major refactoring of how main.js generates the sidebar) is by adding an inline-script.

Testing #84834 locally it has an additional change in behaviour that it shows children instead of siblings in the sidebar, that is how it manages to not add a new file because it looks one level deeper on all module's sidebars.

GuillaumeGomez · 2021-05-15T18:35:15Z

Yes, this is what I said, I have two changes in my PR. I can extract the fix for the crate listing in another PR to keep the one under debate if you want.

Stupremee · 2021-05-15T18:39:08Z

@Stupremee: Sorry that you're in the middle of this. :-/

No problem!

Yes, this is what I said, I have two changes in my PR. I can extract the fix for the crate listing in another PR to keep the one under debate if you want.

That works too and might be the fastest solution.

Nemo157 · 2021-05-15T18:46:17Z

We discussed on discord and decided to go ahead with this as a first fix.

@bors r+

bors · 2021-05-15T18:46:18Z

📌 Commit aaf0ff8 has been approved by Nemo157

bors · 2021-05-16T07:33:02Z

⌛ Testing commit aaf0ff8 with merge 94ecdfd...

bors · 2021-05-16T09:49:06Z

☀️ Test successful - checks-actions
Approved by: Nemo157
Pushing 94ecdfd to master...

apiraino · 2021-05-20T15:35:21Z

Beta backport approved as per compiler team on Zulip

@rustbot label +beta-accepted

…ulacrum [beta] backports * Backport 1.52.1 release notes rust-lang#85404 * remove InPlaceIterable marker from Peekable due to unsoundness rust-lang#85340 * rustdoc: Call initSidebarItems in root module of crate rust-lang#85304 * Update LLVM submodule rust-lang#85236 * Do not ICE on invalid const param rust-lang#84913 * Disallows #![feature(no_coverage)] on stable and beta (using standard crate-level gating) rust-lang#84871 * Ensure TLS destructors run before thread joins in SGX rust-lang#84409

rust-highfive assigned jsha May 14, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 14, 2021

Stupremee force-pushed the crates-in-sidebar-in-root branch from 9545f7a to 2203840 Compare May 14, 2021 20:52

jsha reviewed May 14, 2021

View reviewed changes

Stupremee added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-js Area: Rustdoc's JS front-end labels May 14, 2021

jsha mentioned this pull request May 15, 2021

cargo doc no longer generates a nav list of all dependencies in the sidebar #85301

Closed

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 15, 2021

This comment has been minimized.

Sign in to view

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 15, 2021

Stupremee force-pushed the crates-in-sidebar-in-root branch from b8c5468 to 7c4f3f4 Compare May 15, 2021 17:19

Call initSidebarItems in root module of crate

aaf0ff8

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 15, 2021

Nemo157 added beta-nominated Nominated for backporting to the compiler in the beta channel. I-nominated labels May 15, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label May 16, 2021

bors merged commit 94ecdfd into rust-lang:master May 16, 2021

rustbot added this to the 1.54.0 milestone May 16, 2021

This was referenced May 16, 2021

Generate links to definition in rustdoc source code pages #84176

Merged

Sidebar unification #84834

Merged

Stupremee deleted the crates-in-sidebar-in-root branch May 20, 2021 06:19

rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label May 20, 2021

Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 22, 2021

Mark-Simulacrum modified the milestones: 1.54.0, 1.53.0 May 22, 2021

Mark-Simulacrum mentioned this pull request May 22, 2021

[beta] backports #85588

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: Call `initSidebarItems` in root module of crate #85304

rustdoc: Call `initSidebarItems` in root module of crate #85304

Stupremee commented May 14, 2021

jsha May 14, 2021

Stupremee May 14, 2021

jsha May 14, 2021

Stupremee May 14, 2021

jsha commented May 14, 2021

Stupremee commented May 14, 2021 •

edited

Loading

Stupremee commented May 14, 2021

jsha commented May 14, 2021

Stupremee commented May 15, 2021 •

edited

Loading

Stupremee commented May 15, 2021

Nemo157 commented May 15, 2021 •

edited

Loading

Stupremee commented May 15, 2021

GuillaumeGomez commented May 15, 2021

jsha commented May 15, 2021

jsha commented May 15, 2021

bors commented May 15, 2021

Nemo157 commented May 15, 2021 •

edited

Loading

This comment has been minimized.

jsha commented May 15, 2021

Stupremee commented May 15, 2021

Stupremee commented May 15, 2021

Stupremee commented May 15, 2021 •

edited

Loading

Nemo157 commented May 15, 2021

GuillaumeGomez commented May 15, 2021

Stupremee commented May 15, 2021

GuillaumeGomez commented May 15, 2021

Nemo157 commented May 15, 2021 •

edited

Loading

GuillaumeGomez commented May 15, 2021

Stupremee commented May 15, 2021

Nemo157 commented May 15, 2021

bors commented May 15, 2021

bors commented May 16, 2021

bors commented May 16, 2021

apiraino commented May 20, 2021

rustdoc: Call initSidebarItems in root module of crate #85304

rustdoc: Call initSidebarItems in root module of crate #85304

Conversation

Stupremee commented May 14, 2021

jsha May 14, 2021

Choose a reason for hiding this comment

Stupremee May 14, 2021

Choose a reason for hiding this comment

jsha May 14, 2021

Choose a reason for hiding this comment

Stupremee May 14, 2021

Choose a reason for hiding this comment

jsha commented May 14, 2021

Stupremee commented May 14, 2021 • edited Loading

Stupremee commented May 14, 2021

jsha commented May 14, 2021

Stupremee commented May 15, 2021 • edited Loading

Stupremee commented May 15, 2021

Nemo157 commented May 15, 2021 • edited Loading

Stupremee commented May 15, 2021

GuillaumeGomez commented May 15, 2021

jsha commented May 15, 2021

jsha commented May 15, 2021

bors commented May 15, 2021

Nemo157 commented May 15, 2021 • edited Loading

This comment has been minimized.

jsha commented May 15, 2021

Stupremee commented May 15, 2021

Stupremee commented May 15, 2021

Stupremee commented May 15, 2021 • edited Loading

Nemo157 commented May 15, 2021

GuillaumeGomez commented May 15, 2021

Stupremee commented May 15, 2021

GuillaumeGomez commented May 15, 2021

Nemo157 commented May 15, 2021 • edited Loading

GuillaumeGomez commented May 15, 2021

Stupremee commented May 15, 2021

Nemo157 commented May 15, 2021

bors commented May 15, 2021

bors commented May 16, 2021

bors commented May 16, 2021

apiraino commented May 20, 2021

rustdoc: Call `initSidebarItems` in root module of crate #85304

rustdoc: Call `initSidebarItems` in root module of crate #85304

Stupremee commented May 14, 2021 •

edited

Loading

Stupremee commented May 15, 2021 •

edited

Loading

Nemo157 commented May 15, 2021 •

edited

Loading

Nemo157 commented May 15, 2021 •

edited

Loading

Stupremee commented May 15, 2021 •

edited

Loading

Nemo157 commented May 15, 2021 •

edited

Loading