Auto merge of #54733 - GuillaumeGomez:stabilize-rustdoc-theme, r=olli…

…e27,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.)

src/doc/rustdoc/src/command-line-arguments.md

@@ -355,7 +355,38 @@ $ rustdoc src/lib.rs --edition 2018
 $ rustdoc --test src/lib.rs --edition 2018
 ```
 
-This flag allows rustdoc to treat your rust code as the given edition. It will compile doctests with
+This flag allows `rustdoc` to treat your rust code as the given edition. It will compile doctests with
 the given edition as well. As with `rustc`, the default edition that `rustdoc` will use is `2015`
 (the first edition).
 
+## `--theme`: add a theme to the documentation output
+
+Using this flag looks like this:
+
+```bash
+$ rustdoc src/lib.rs --theme /path/to/your/custom-theme.css
+```
+
+`rustdoc`'s default output includes two themes: `light` (the default) and
+`dark`. This flag allows you to add custom themes to the output. Giving a CSS
+file to this flag adds it to your documentation as an additional theme choice.
+The theme's name is determined by its filename; a theme file named
+`custom-theme.css` will add a theme named `custom-theme` to the documentation.
+
+## `--check-theme`: verify custom themes against the default theme
+
+Using this flag looks like this:
+
+```bash
+$ rustdoc --check-theme /path/to/your/custom-theme.css
+```
+
+While `rustdoc`'s HTML output is more-or-less consistent between versions, there
+is no guarantee that a theme file will have the same effect. The `--theme` flag
+will still allow you to add the theme to your documentation, but to ensure that
+your theme works as expected, you can use this flag to verify that it implements
+the same CSS rules as the official `light` theme.
+
+`--check-theme` is a separate mode in `rustdoc`. When `rustdoc` sees the
+`--check-theme` flag, it discards all other flags and only performs the CSS rule
+comparison operation.

src/doc/rustdoc/src/unstable-features.md

@@ -294,30 +294,6 @@ some consideration for their stability, and names that end in a number). Giving
 `rustdoc` will disable this sorting and instead make it print the items in the order they appear in
 the source.
 
-### `--themes`: provide additional themes
-
-Using this flag looks like this:
-
-```bash
-$ rustdoc src/lib.rs -Z unstable-options --themes theme.css
-```
-
-Giving this flag to `rustdoc` will make it copy your theme into the generated crate docs and enable
-it in the theme selector. Note that `rustdoc` will reject your theme file if it doesn't style
-everything the "light" theme does. See `--theme-checker` below for details.
-
-### `--theme-checker`: verify theme CSS for validity
-
-Using this flag looks like this:
-
-```bash
-$ rustdoc -Z unstable-options --theme-checker theme.css
-```
-
-Before including your theme in crate docs, `rustdoc` will compare all the CSS rules it contains
-against the "light" theme included by default. Using this flag will allow you to see which rules are
-missing if `rustdoc` rejects your theme.
-
 ### `--resource-suffix`: modifying the name of CSS/JavaScript in crate docs
 
 Using this flag looks like this:

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);
             }
         }

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| {

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()
     )?;

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.

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")
         }),

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);

src/test/run-make-fulldeps/rustdoc-themes/Makefile

@@ -0,0 +1,10 @@
+-include ../tools.mk
+
+# Test that rustdoc will properly load in a theme file and display it in the theme selector.
+
+OUTPUT_DIR := "$(TMPDIR)/rustdoc-themes"
+
+all:
+	cp $(S)/src/librustdoc/html/static/themes/light.css $(TMPDIR)/test.css
+	$(RUSTDOC) -o $(OUTPUT_DIR) foo.rs --theme $(TMPDIR)/test.css
+	$(HTMLDOCCK) $(OUTPUT_DIR) foo.rs

src/test/run-make-fulldeps/rustdoc-themes/foo.rs

@@ -0,0 +1,4 @@
+// @has test.css
+// @has foo/struct.Foo.html
+// @has - '//link[@rel="stylesheet"]/@href' '../test.css'
+pub struct Foo;

src/test/run-make-fulldeps/tools.mk

@@ -15,7 +15,7 @@ RUSTC := $(RUSTC) -Clinker=$(RUSTC_LINKER)
 RUSTDOC := $(RUSTDOC) -Clinker=$(RUSTC_LINKER)
 endif
 #CC := $(CC) -L $(TMPDIR)
-HTMLDOCCK := $(PYTHON) $(S)/src/etc/htmldocck.py
+HTMLDOCCK := '$(PYTHON)' '$(S)/src/etc/htmldocck.py'
 CGREP := "$(S)/src/etc/cat-and-grep.sh"
 
 # This is the name of the binary we will generate and run; use this

src/tools/rustdoc-themes/main.rs

@@ -38,9 +38,11 @@ fn main() {
         eprintln!("No theme found in \"{}\"...", themes_folder);
         exit(1);
     }
+    let arg_name = "--check-theme".to_owned();
     let status = Command::new(rustdoc_bin)
-                        .args(&["-Z", "unstable-options", "--theme-checker"])
-                        .args(&themes)
+                        .args(&themes.iter()
+                                     .flat_map(|t| vec![&arg_name, t].into_iter())
+                                     .collect::<Vec<_>>())
                         .status()
                         .expect("failed to execute child");
     if !status.success() {