Rollup merge of #87288 - ijackson:rustdoc-theme, r=GuillaumeGomez

rustdoc: Restore --default-theme, etc, by restoring varname escaping

In #86157

    cd0f93193c
    Use Tera templates for rustdoc.

dropped the following transformation from the keys of the default settings element's `data-` attribute names:

    .map(|(k, v)| format!(r#" data-{}="{}""#, k.replace('-', "_"), Escape(v)))

The `Escape` part is indeed no longer needed, because Tera does that for us.  But the massaging of `-` to `_` is needed, for the (bizarre) reasons explained in the new comments.

I have tested that the default theme function works again for me.  I have also verified that passing (in shell syntax)

    '--default-theme="zork&"'

escapes the value in the HTML.

Closes #87263

Cargo.toml

@@ -40,6 +40,7 @@ members = [
 exclude = [
   "build",
   "compiler/rustc_codegen_cranelift",
+  "src/test/rustdoc-gui",
   # HACK(eddyb) This hardcodes the fact that our CI uses `/checkout/obj`.
   "obj",
   # The `x` binary is a thin wrapper that calls `x.py`, which initializes

src/bootstrap/test.rs

@@ -907,18 +907,27 @@ impl Step for RustdocGUI {
         // We remove existing folder to be sure there won't be artifacts remaining.
         let _ = fs::remove_dir_all(&out_dir);
 
-        let src_path = "src/test/rustdoc-gui/src";
+        let src_path = builder.build.src.join("src/test/rustdoc-gui/src");
         // We generate docs for the libraries present in the rustdoc-gui's src folder.
-        let mut cargo = Command::new(&builder.initial_cargo);
+        for entry in src_path.read_dir().expect("read_dir call failed") {
-        cargo
+            if let Ok(entry) = entry {
-            .arg("doc")
+                let path = entry.path();
-            .arg("--workspace")
+
-            .arg("--target-dir")
+                if !path.is_dir() {
-            .arg(&out_dir)
+                    continue;
-            .env("RUSTDOC", builder.rustdoc(self.compiler))
+                }
-            .env("RUSTC", builder.rustc(self.compiler))
+
-            .current_dir(&builder.build.src.join(src_path));
+                let mut cargo = Command::new(&builder.initial_cargo);
-        builder.run(&mut cargo);
+                cargo
+                    .arg("doc")
+                    .arg("--target-dir")
+                    .arg(&out_dir)
+                    .env("RUSTDOC", builder.rustdoc(self.compiler))
+                    .env("RUSTC", builder.rustc(self.compiler))
+                    .current_dir(path);
+                builder.run(&mut cargo);
+            }
+        }
 
         // We now run GUI tests.
         let mut command = Command::new(&nodejs);

src/librustdoc/config.rs

@@ -459,7 +459,31 @@ impl Options {
                 })
                 .collect(),
         ];
-        let default_settings = default_settings.into_iter().flatten().collect();
+        let default_settings = default_settings
+            .into_iter()
+            .flatten()
+            .map(
+                // The keys here become part of `data-` attribute names in the generated HTML.  The
+                // browser does a strange mapping when converting them into attributes on the
+                // `dataset` property on the DOM HTML Node:
+                //   https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/dataset
+                //
+                // The original key values we have are the same as the DOM storage API keys and the
+                // command line options, so contain `-`.  Our Javascript needs to be able to look
+                // these values up both in `dataset` and in the storage API, so it needs to be able
+                // to convert the names back and forth.  Despite doing this kebab-case to
+                // StudlyCaps transformation automatically, the JS DOM API does not provide a
+                // mechanism for doing the just transformation on a string.  So we want to avoid
+                // the StudlyCaps representation in the `dataset` property.
+                //
+                // We solve this by replacing all the `-`s with `_`s.  We do that here, when we
+                // generate the `data-` attributes, and in the JS, when we look them up.  (See
+                // `getSettingValue` in `storage.js.`) Converting `-` to `_` is simple in JS.
+                //
+                // The values will be HTML-escaped by the default Tera escaping.
+                |(k, v)| (k.replace('-', "_"), v),
+            )
+            .collect();
 
         let test_args = matches.opt_strs("test-args");
         let test_args: Vec<String> =

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

@@ -22,6 +22,8 @@ function getSettingValue(settingName) {
         return current;
     }
     if (settingsDataset !== null) {
+        // See the comment for `default_settings.into_iter()` etc. in
+        // `Options::from_matches` in `librustdoc/config.rs`.
         var def = settingsDataset[settingName.replace(/-/g,'_')];
         if (def !== undefined) {
             return def;

src/test/rustdoc-gui/default-settings.goml

@@ -0,0 +1,8 @@
+// This test ensures that the default settings are correctly applied.
+//
+// The "settings" crate uses "ayu" as default setting, which is what we will
+// check.
+goto: file://|DOC_PATH|/settings/index.html
+// Wait a bit to be sure the default theme is applied.
+wait-for: 1000
+assert-css: ("body", {"background-color": "rgb(15, 20, 25)"})

src/test/rustdoc-gui/src/Cargo.toml

@@ -1,6 +0,0 @@
-[workspace]
-members = [
-    "test_docs",
-    "lib2",
-    "implementors",
-]

src/test/rustdoc-gui/src/lib2/Cargo.lock

@@ -12,7 +12,3 @@ version = "0.1.0"
 dependencies = [
  "implementors",
 ]
-
-[[package]]
-name = "test_docs"
-version = "0.1.0"

src/test/rustdoc-gui/src/lib2/Cargo.toml

@@ -7,4 +7,4 @@ edition = "2018"
 path = "lib.rs"
 
 [dependencies]
-implementors = { path = "../implementors" }
+implementors = { path = "./implementors" }

src/test/rustdoc-gui/src/lib2/implementors/Cargo.lock

@@ -0,0 +1,7 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "implementors"
+version = "0.1.0"

src/test/rustdoc-gui/src/settings/.cargo/config.toml

@@ -0,0 +1,2 @@
+[build]
+rustdocflags = ["--default-theme", "ayu"]

src/test/rustdoc-gui/src/settings/Cargo.lock

@@ -0,0 +1,7 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "settings"
+version = "0.1.0"

src/test/rustdoc-gui/src/settings/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "settings"
+version = "0.1.0"
+edition = "2018"
+
+[lib]
+path = "lib.rs"

src/test/rustdoc-gui/src/settings/lib.rs

@@ -0,0 +1 @@
+pub fn foo() {}

src/test/rustdoc-gui/src/test_docs/Cargo.lock

@@ -0,0 +1,7 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "test_docs"
+version = "0.1.0"

src/test/rustdoc/default-theme.rs

@@ -0,0 +1,7 @@
+// compile-flags: --default-theme ayu
+
+// @has default_theme/index.html
+// @has - '//script[@id="default-settings"]/@data-theme' 'ayu'
+// @has - '//script[@id="default-settings"]/@data-use_system_theme' 'false'
+
+pub fn whatever() {}