rustdoc: Consolidate static-file replacement mechanism #91062

jsha · 2021-11-19T23:56:23Z

There were a few places in rustdoc where we would take static JS or CSS and rewrite it at doc generation time to insert values. This consolidates all the CSS instances into one CSS file and replaces the JS examples with data- attributes on the rustdoc-vars div.

Demo https://rustdoc.crud.net/jsha/static-file-replace/test_docs/

r? @GuillaumeGomez

rust-highfive · 2021-11-19T23:56:25Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

jyn514 · 2021-11-20T06:19:05Z

src/librustdoc/html/render/write_shared.rs

+        cx,
+        options,
+    )?;
+
    // FIXME: this should probably not be a toolchain file since it depends on `--theme`.


You can remove this FIXME now I think :)

jyn514 · 2021-11-20T06:21:54Z

src/librustdoc/html/render/write_shared.rs

+        cx,
+        options,
+    )?;
+
    // FIXME: this should probably not be a toolchain file since it depends on `--theme`.
    // But it seems a shame to copy it over and over when it's almost always the same.
    // Maybe we can change the representation to move this out of main.js?
    write_minify(


Hmm, I think this should be using cx.write_minify(InvocationSpecific), since it can vary from crate to crate. Unfortunately we don't have a good way of testing this before it hits nightly right now.

The only thing that is invocation specific is the list of themes, right? What if I put those in a data- attribute in the HTML instead? The list is quite short, so duplication is not a big concern.

That would be great! Thanks :)

GuillaumeGomez · 2021-11-20T12:29:23Z

src/librustdoc/html/static/js/storage.js

@@ -1,5 +1,8 @@
 // From rust:
 /* global resourcesSuffix */
+var resourcesSuffix = /* AUTOREPLACE: */"RESOURCE_SUFFIX";


I'm a bit worried about this: if we generate docs with two different rustdoc version and/or different suffix, it means the older version will use newer files, which might break the display.

storage is stored as a versioned file (e.g. storage1.58.0.js), so it would load the other versioned files that match it.

That is, if you use suffix, otherwise you'll simply overwrite.

I don't think it's possible for rustdoc alone to prevent conflicting versions with our current setup. If you think that should be a goal, maybe open an issue or MCP but I don't think it makes sense to hold specific parts of rustdoc to a higher standard than others.

The more I think about it, the more I think it would be cleaner to put each of these three variables in a data- attribute in the HTML, like I proposed in #91062 (comment). The big advantage there is that when you're editing JS you can copy it directly into place without waiting to rebuild rustdoc. I've run into this a number of times just recently.

I don't think it's possible for rustdoc alone to prevent conflicting versions with our current setup. If you think that should be a goal, maybe open an issue or MCP but I don't think it makes sense to hold specific parts of rustdoc to a higher standard than others.

It is working currently because the links are set directly in the HTML, not in the JS which is regenerated. This is my biggest issue with this change, moving "hard-coded" HTML content into JS.

@jsha Yes, I think that could work. The added size would be negligible as well.

jsha · 2021-11-24T05:37:33Z

Alright, I switched this up so that main.js isn't rewritten at all - instead, the variables we want are set in the rustdoc-vars div.

One big change along the way: I revamped how stylesheets links are rendered into the HTML (c330963). I think this cleans things up a lot. It also made it much nicer to be able to generate a list of themes in layout.rs.

Also I changed storage.js to not rely on resource_suffix, because rustdoc-vars aren't yet available at the time storage.js runs.

…eGomez Fix toggle-click-deadspace rustdoc-gui test In rust-lang#91103 I introduced a rustdoc-gui test for clicks on toggles. I introduced some documentation on a method in lib2/struct.Foo.html so there would be something to toggle, but accidentally left the test checking test_docs/struct.Foo.html. That caused the test to reliably fail. I'm not sure how that test got past GitHub Actions and bors, but it's manifesting in test failures at rust-lang#91062 (comment) and rust-lang#91170 (comment). This fixes by pointing at the right file. r? `@GuillaumeGomez`

jyn514

LGTM but I want to give Guillaume a chance to review. Thanks for working on this :)

GuillaumeGomez · 2021-11-24T15:03:57Z

It looks good to me as well, thanks! You'll need to rebase to make the CI happy. ;)

r=jyn514,GuillaumeGomez once done.

We carry around a list of stylesheets that can carry two different types of thing: 1. Internal stylesheets specific to a page type (only for settings) 2. Themes In this change I move the link generation for settings.css into settings(), so Context.style_files is reserved just for themes. We had two places where we extracted a base theme name from a list of StylePaths. I consolidated that code to be a method on StylePath. I moved generation of link tags for stylesheets into the page.html template. With that change, I made the template responsible for special handling of light.css (making it the default theme) and of the other themes (marking them disabled). That allowed getting rid of the `disabled` field on StylePath.

We had been injecting the list of themes and the rustdoc version into main.js by rewriting it at doc generation time. By avoiding this rewrite, we can make it easier to edit main.js without regenerating all the docs. Added a more convenient accessor for rustdoc-vars. Changed storage.js to not rely on resourcesSuffix. It could in theory use rustdoc-vars, but because rustdoc-vars is at the end of the HTML, it's not available when storage.js runs (very early in page load).

jsha · 2021-11-27T04:06:38Z

@bors r=jyn514,GuillaumeGomez

bors · 2021-11-27T04:06:39Z

📌 Commit d9afca5 has been approved by jyn514,GuillaumeGomez

…GuillaumeGomez rustdoc: Consolidate static-file replacement mechanism There were a few places in rustdoc where we would take static JS or CSS and rewrite it at doc generation time to insert values. This consolidates all the CSS instances into one CSS file and replaces the JS examples with data- attributes on the rustdoc-vars div. Demo https://rustdoc.crud.net/jsha/static-file-replace/test_docs/ r? `@GuillaumeGomez`

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#83791 (Weaken guarantee around advancing underlying iterators in zip) - rust-lang#90995 (Document non-guarantees for Hash) - rust-lang#91057 (Expand `available_parallelism` docs in anticipation of cgroup quota support) - rust-lang#91062 (rustdoc: Consolidate static-file replacement mechanism) - rust-lang#91208 (Account for incorrect `where T::Assoc = Ty` bound) - rust-lang#91266 (Use non-generic inner function for pointer formatting) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Folyd · 2021-12-07T02:43:35Z

Ohhh, this PR broke my extension huhu/rust-search-extension#134. 😅
Rust search extension relies on the search-index.js. I hope we can keep the search-index.js loading steadily. Would you mind CC me if the search-index.js loading method has changed? Thanks. ❤️ @jsha @GuillaumeGomez

jyn514 · 2021-12-07T05:14:05Z

@Folyd you can set up an automatic ping in the highfive bot if you like - here's an example: https://github.com/rust-lang/highfive/blob/6e2c21639aaeafaeae423b244d353247c507d46a/highfive/configs/rust-lang/rust.json#L134

@Folyd

Ping @Folyd if some changes occurred in librustdoc's HTML/CSS/JS. Related context: rust-lang/rust#91062 (comment)

jsha added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: rustdoc UI (generated HTML) labels Nov 19, 2021

rust-highfive assigned GuillaumeGomez Nov 19, 2021

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

This comment has been minimized.

Sign in to view

jyn514 changed the title ~~Consolidate static-file replacement mechanism~~ rustdoc: Consolidate static-file replacement mechanism Nov 20, 2021

jyn514 reviewed Nov 20, 2021

View reviewed changes

GuillaumeGomez reviewed Nov 20, 2021

View reviewed changes

jsha force-pushed the static-file-replace branch from b424fd8 to 7a5cd2b Compare November 24, 2021 05:29

This comment has been minimized.

Sign in to view

jsha force-pushed the static-file-replace branch from 7a5cd2b to 276ea81 Compare November 24, 2021 06:26

This comment has been minimized.

Sign in to view

jsha force-pushed the static-file-replace branch from 276ea81 to 57e6fd7 Compare November 24, 2021 07:47

jsha mentioned this pull request Nov 24, 2021

Fix toggle-click-deadspace rustdoc-gui test #91171

Merged

jyn514 approved these changes Nov 24, 2021

View reviewed changes

GuillaumeGomez approved these changes Nov 24, 2021

View reviewed changes

jyn514 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 24, 2021

jsha added 3 commits November 24, 2021 19:41

Consistentize the system for image URLs in CSS.

d9afca5

jsha force-pushed the static-file-replace branch from 57e6fd7 to d9afca5 Compare November 25, 2021 04:05

bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 27, 2021

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 27, 2021

This was referenced Nov 27, 2021

Rollup of 5 pull requests #91287

Closed

Rollup of 6 pull requests #91288

Merged

bors merged commit 55f8b5f into rust-lang:master Nov 27, 2021

rustbot added this to the 1.59.0 milestone Nov 27, 2021

camelid mentioned this pull request Nov 28, 2021

rustdoc: Header color has regressed #91304

Closed

jsha deleted the static-file-replace branch November 28, 2021 04:52

Folyd added a commit to Folyd/highfive that referenced this pull request Dec 7, 2021

Add new reviewer to librustdoc static files

747b4c4

Ping @Folyd if some changes occurred in librustdoc's HTML/CSS/JS. Related context: rust-lang/rust#91062 (comment)

Folyd mentioned this pull request Dec 7, 2021

Add new reviewer to librustdoc static files rust-lang/highfive#367

Merged

yvt mentioned this pull request Aug 19, 2022

Generate SVG work for both light and dark theme yvt/svgbobdoc#6

Open

syphar mentioned this pull request Dec 28, 2022

rustdoc referenced search.js and settings.js not via static-root-path rust-lang/docs.rs#1979

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: Consolidate static-file replacement mechanism #91062

rustdoc: Consolidate static-file replacement mechanism #91062

jsha commented Nov 19, 2021 •

edited

Loading

rust-highfive commented Nov 19, 2021

This comment has been minimized.

jyn514 Nov 20, 2021

jyn514 Nov 20, 2021

jsha Nov 20, 2021

jyn514 Nov 20, 2021

GuillaumeGomez Nov 20, 2021

jsha Nov 21, 2021

GuillaumeGomez Nov 21, 2021

jyn514 Nov 21, 2021

jsha Nov 21, 2021

GuillaumeGomez Nov 21, 2021

jsha commented Nov 24, 2021

This comment has been minimized.

This comment has been minimized.

jyn514 left a comment

GuillaumeGomez commented Nov 24, 2021

jsha commented Nov 27, 2021

bors commented Nov 27, 2021

Folyd commented Dec 7, 2021

jyn514 commented Dec 7, 2021

rustdoc: Consolidate static-file replacement mechanism #91062

rustdoc: Consolidate static-file replacement mechanism #91062

Conversation

jsha commented Nov 19, 2021 • edited Loading

rust-highfive commented Nov 19, 2021

This comment has been minimized.

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

jsha commented Nov 24, 2021

This comment has been minimized.

This comment has been minimized.

jyn514 left a comment

Choose a reason for hiding this comment

GuillaumeGomez commented Nov 24, 2021

jsha commented Nov 27, 2021

bors commented Nov 27, 2021

Folyd commented Dec 7, 2021

jyn514 commented Dec 7, 2021

jsha commented Nov 19, 2021 •

edited

Loading