Auto merge of #54733 - GuillaumeGomez:stabilize-rustdoc-theme, r=ollie27,Dylan-DPC

Stabilize rustdoc theme options

Closes #54730

This PR stabilizes the `--themes` (now `--theme`) and `--theme-checker` (now `--check-theme`) options, for allowing users to add custom themes to their documentation.

Rustdoc includes two themes by default: `light` and `dark`. Using the `--theme` option, you can give rustdoc a CSS file to include as an extra theme for that render. Themes are named after the CSS file used, so using `--theme /path/to/your/custom-theme.css` will add a theme called `custom-theme` to the documentation.

Even though the CLI flag to add a theme is getting stabilized, there's no guarantee that a theme file will always have the same effect on documentation generated with future versions of rustdoc. To aid in ensuring that a theme will work, the flag `--check-theme` is also available, which compares the CSS rules defined by a custom theme against the ones used in the `light` theme. If the `light` theme defines a CSS rule that the custom theme does not, rustdoc will report an error. (Rustdoc also performs this check for themes given to `--theme`, but only reports a warning when a difference is found.)

6 files changed, 37 insertions, 24 deletions

diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs
index 3e1c57182b9..cdb1a1f6997 100644
--- a/src/librustdoc/config.rs
+++ b/src/librustdoc/config.rs
@@ -1,4 +1,5 @@
 use std::collections::BTreeMap;
+use std::ffi::OsStr;
 use std::fmt;
 use std::path::PathBuf;
 
@@ -281,12 +282,12 @@ impl Options {
         // check for deprecated options
         check_deprecated_options(&matches, &diag);
 
-        let to_check = matches.opt_strs("theme-checker");
+        let to_check = matches.opt_strs("check-theme");
         if !to_check.is_empty() {
             let paths = theme::load_css_paths(static_files::themes::LIGHT.as_bytes());
             let mut errors = 0;
 
-            println!("rustdoc: [theme-checker] Starting tests!");
+            println!("rustdoc: [check-theme] Starting tests! (Ignoring all other arguments)");
             for theme_file in to_check.iter() {
                 print!(" - Checking \"{}\"...", theme_file);
                 let (success, differences) = theme::test_theme_against(theme_file, &paths, &diag);
@@ -357,23 +358,35 @@ impl Options {
         }
 
         let mut themes = Vec::new();
-        if matches.opt_present("themes") {
+        if matches.opt_present("theme") {
             let paths = theme::load_css_paths(static_files::themes::LIGHT.as_bytes());
 
-            for (theme_file, theme_s) in matches.opt_strs("themes")
+            for (theme_file, theme_s) in matches.opt_strs("theme")
                                                 .iter()
                                                 .map(|s| (PathBuf::from(&s), s.to_owned())) {
                 if !theme_file.is_file() {
-                    diag.struct_err("option --themes arguments must all be files").emit();
+                    diag.struct_err(&format!("invalid argument: \"{}\"", theme_s))
+                        .help("arguments to --theme must be files")
+                        .emit();
                     return Err(1);
                 }
-                let (success, ret) = theme::test_theme_against(&theme_file, &paths, &diag);
-                if !success || !ret.is_empty() {
-                    diag.struct_err(&format!("invalid theme: \"{}\"", theme_s))
-                        .help("check what's wrong with the --theme-checker option")
+                if theme_file.extension() != Some(OsStr::new("css")) {
+                    diag.struct_err(&format!("invalid argument: \"{}\"", theme_s))
                         .emit();
                     return Err(1);
                 }
+                let (success, ret) = theme::test_theme_against(&theme_file, &paths, &diag);
+                if !success {
+                    diag.struct_err(&format!("error loading theme file: \"{}\"", theme_s)).emit();
+                    return Err(1);
+                } else if !ret.is_empty() {
+                    diag.struct_warn(&format!("theme file \"{}\" is missing CSS rules from the \
+                                               default theme", theme_s))
+                        .warn("the theme may appear incorrect when loaded")
+                        .help(&format!("to see what rules are missing, call `rustdoc \
+                                        --check-theme \"{}\"`", theme_s))
+                        .emit();
+                }
                 themes.push(theme_file);
             }
         }
diff --git a/src/librustdoc/html/layout.rs b/src/librustdoc/html/layout.rs
index 697dee0216e..8249e69e9a7 100644
--- a/src/librustdoc/html/layout.rs
+++ b/src/librustdoc/html/layout.rs
@@ -1,6 +1,7 @@
 use std::path::PathBuf;
 
 use crate::externalfiles::ExternalHtml;
+use crate::html::escape::Escape;
 use crate::html::render::ensure_trailing_slash;
 use crate::html::format::{Buffer, Print};
 
@@ -166,10 +167,11 @@ pub fn render<T: Print, S: Print>(
     themes = themes.iter()
                    .filter_map(|t| t.file_stem())
                    .filter_map(|t| t.to_str())
-                   .map(|t| format!(r#"<link rel="stylesheet" type="text/css" href="{}{}{}.css">"#,
-                                    static_root_path,
-                                    t,
-                                    page.resource_suffix))
+                   .map(|t| format!(r#"<link rel="stylesheet" type="text/css" href="{}.css">"#,
+                                    Escape(&format!("{}{}{}",
+                                                    static_root_path,
+                                                    t,
+                                                    page.resource_suffix))))
                    .collect::<String>(),
     suffix=page.resource_suffix,
     static_extra_scripts=page.static_extra_scripts.iter().map(|e| {
diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs
index 9a87bcc10db..1207c5e3bc5 100644
--- a/src/librustdoc/html/render.rs
+++ b/src/librustdoc/html/render.rs
@@ -633,19 +633,16 @@ function handleThemeButtonsBlur(e) {{
 
 themePicker.onclick = switchThemeButtonState;
 themePicker.onblur = handleThemeButtonsBlur;
-[{}].forEach(function(item) {{
+{}.forEach(function(item) {{
     var but = document.createElement('button');
-    but.innerHTML = item;
+    but.textContent = item;
     but.onclick = function(el) {{
         switchTheme(currentTheme, mainTheme, item, true);
     }};
     but.onblur = handleThemeButtonsBlur;
     themes.appendChild(but);
 }});"#,
-                 themes.iter()
-                       .map(|s| format!("\"{}\"", s))
-                       .collect::<Vec<String>>()
-                       .join(","));
+                 as_json(&themes));
     write(cx.dst.join(&format!("theme{}.js", cx.shared.resource_suffix)),
           theme_js.as_bytes()
     )?;
diff --git a/src/librustdoc/html/static_files.rs b/src/librustdoc/html/static_files.rs
index 3a2c24b1a96..9fc1d76185f 100644
--- a/src/librustdoc/html/static_files.rs
+++ b/src/librustdoc/html/static_files.rs
@@ -59,7 +59,7 @@ pub static RUST_FAVICON: &'static [u8] = include_bytes!("static/favicon.ico");
 /// The built-in themes given to every documentation site.
 pub mod themes {
     /// The "light" theme, selected by default when no setting is available. Used as the basis for
-    /// the `--theme-checker` functionality.
+    /// the `--check-theme` functionality.
     pub static LIGHT: &'static str = include_str!("static/themes/light.css");
 
     /// The "dark" theme.
diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs
index 8bfaf98f086..277475f6fff 100644
--- a/src/librustdoc/lib.rs
+++ b/src/librustdoc/lib.rs
@@ -252,13 +252,13 @@ fn opts() -> Vec<RustcOptGroup> {
             o.optflag("", "sort-modules-by-appearance", "sort modules by where they appear in the \
                                                          program, rather than alphabetically")
         }),
-        unstable("themes", |o| {
-            o.optmulti("", "themes",
+        stable("theme", |o| {
+            o.optmulti("", "theme",
                        "additional themes which will be added to the generated docs",
                        "FILES")
         }),
-        unstable("theme-checker", |o| {
-            o.optmulti("", "theme-checker",
+        stable("check-theme", |o| {
+            o.optmulti("", "check-theme",
                        "check if given theme is valid",
                        "FILES")
         }),
diff --git a/src/librustdoc/theme.rs b/src/librustdoc/theme.rs
index 7037a146c50..1be85f4a91d 100644
--- a/src/librustdoc/theme.rs
+++ b/src/librustdoc/theme.rs
@@ -273,6 +273,7 @@ pub fn test_theme_against<P: AsRef<Path>>(
     diag: &Handler,
 ) -> (bool, Vec<String>) {
     let data = try_something!(fs::read(f), diag, (false, vec![]));
+
     let paths = load_css_paths(&data);
     let mut ret = vec![];
     get_differences(against, &paths, &mut ret);