From c3309634449ccb021f5d78bfaeeb6211e093c2ac Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Mon, 22 Nov 2021 23:23:58 -0800
Subject: [PATCH] Simplify rendering of stylesheet links into HTML

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.
---
 src/librustdoc/config.rs                   |  2 +-
 src/librustdoc/error.rs                    |  5 ++-
 src/librustdoc/html/layout.rs              | 22 ++++---------
 src/librustdoc/html/render/context.rs      | 18 ++++++----
 src/librustdoc/html/render/mod.rs          | 38 +++++++++-------------
 src/librustdoc/html/render/write_shared.rs |  4 +--
 src/librustdoc/html/templates/page.html    | 10 +++++-
 7 files changed, 49 insertions(+), 50 deletions(-)

diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs
index 493aa56fce6ef..6523d0cf3fc89 100644
--- a/src/librustdoc/config.rs
+++ b/src/librustdoc/config.rs
@@ -556,7 +556,7 @@ impl Options {
                     ))
                     .emit();
                 }
-                themes.push(StylePath { path: theme_file, disabled: true });
+                themes.push(StylePath { path: theme_file });
             }
         }
 
diff --git a/src/librustdoc/error.rs b/src/librustdoc/error.rs
index 82d0002b98b18..8eadbf63f33d9 100644
--- a/src/librustdoc/error.rs
+++ b/src/librustdoc/error.rs
@@ -39,7 +39,10 @@ macro_rules! try_none {
         match $e {
             Some(e) => e,
             None => {
-                return Err(Error::new(io::Error::new(io::ErrorKind::Other, "not found"), $file));
+                return Err(<crate::error::Error as crate::docfs::PathError>::new(
+                    io::Error::new(io::ErrorKind::Other, "not found"),
+                    $file,
+                ));
             }
         }
     }};
diff --git a/src/librustdoc/html/layout.rs b/src/librustdoc/html/layout.rs
index 71d7cc1a09dce..03ad784ef77c5 100644
--- a/src/librustdoc/html/layout.rs
+++ b/src/librustdoc/html/layout.rs
@@ -2,8 +2,8 @@ use std::path::PathBuf;
 
 use rustc_data_structures::fx::FxHashMap;
 
+use crate::error::Error;
 use crate::externalfiles::ExternalHtml;
-use crate::html::escape::Escape;
 use crate::html::format::{Buffer, Print};
 use crate::html::render::{ensure_trailing_slash, StylePath};
 
@@ -50,7 +50,7 @@ struct PageLayout<'a> {
     static_root_path: &'a str,
     page: &'a Page<'a>,
     layout: &'a Layout,
-    style_files: String,
+    themes: Vec<String>,
     sidebar: String,
     content: String,
     krate_with_trailing_slash: String,
@@ -66,26 +66,18 @@ crate fn render<T: Print, S: Print>(
 ) -> String {
     let static_root_path = page.get_static_root_path();
     let krate_with_trailing_slash = ensure_trailing_slash(&layout.krate).to_string();
-    let style_files = style_files
+    let themes = style_files
         .iter()
-        .filter_map(|t| t.path.file_stem().map(|stem| (stem, t.disabled)))
-        .filter_map(|t| t.0.to_str().map(|path| (path, t.1)))
-        .map(|t| {
-            format!(
-                r#"<link rel="stylesheet" type="text/css" href="{}.css" {} {}>"#,
-                Escape(&format!("{}{}{}", static_root_path, t.0, page.resource_suffix)),
-                if t.1 { "disabled" } else { "" },
-                if t.0 == "light" { "id=\"themeStyle\"" } else { "" }
-            )
-        })
-        .collect::<String>();
+        .map(StylePath::basename)
+        .collect::<Result<_, Error>>()
+        .unwrap_or_default();
     let content = Buffer::html().to_display(t); // Note: This must happen before making the sidebar.
     let sidebar = Buffer::html().to_display(sidebar);
     let teractx = tera::Context::from_serialize(PageLayout {
         static_root_path,
         page,
         layout,
-        style_files,
+        themes,
         sidebar,
         content,
         krate_with_trailing_slash,
diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs
index 069862efde640..365d959ad9f3b 100644
--- a/src/librustdoc/html/render/context.rs
+++ b/src/librustdoc/html/render/context.rs
@@ -504,9 +504,9 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
         // by the browser as the theme stylesheet. The theme system (hackily) works by
         // changing the href to this stylesheet. All other themes are disabled to
         // prevent rule conflicts
-        scx.style_files.push(StylePath { path: PathBuf::from("light.css"), disabled: false });
-        scx.style_files.push(StylePath { path: PathBuf::from("dark.css"), disabled: true });
-        scx.style_files.push(StylePath { path: PathBuf::from("ayu.css"), disabled: true });
+        scx.style_files.push(StylePath { path: PathBuf::from("light.css") });
+        scx.style_files.push(StylePath { path: PathBuf::from("dark.css") });
+        scx.style_files.push(StylePath { path: PathBuf::from("ayu.css") });
 
         let dst = output;
         scx.ensure_dir(&dst)?;
@@ -596,9 +596,13 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
         page.description = "Settings of Rustdoc";
         page.root_path = "./";
 
-        let mut style_files = self.shared.style_files.clone();
         let sidebar = "<h2 class=\"location\">Settings</h2><div class=\"sidebar-elems\"></div>";
-        style_files.push(StylePath { path: PathBuf::from("settings.css"), disabled: false });
+        let theme_names: Vec<String> = self
+            .shared
+            .style_files
+            .iter()
+            .map(StylePath::basename)
+            .collect::<Result<_, Error>>()?;
         let v = layout::render(
             &self.shared.templates,
             &self.shared.layout,
@@ -607,9 +611,9 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             settings(
                 self.shared.static_root_path.as_deref().unwrap_or("./"),
                 &self.shared.resource_suffix,
-                &self.shared.style_files,
+                theme_names,
             )?,
-            &style_files,
+            &self.shared.style_files,
         );
         self.shared.fs.write(settings_file, v)?;
         if let Some(ref redirections) = self.shared.redirections {
diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
index dd592de41bd6c..b761a6dec13ed 100644
--- a/src/librustdoc/html/render/mod.rs
+++ b/src/librustdoc/html/render/mod.rs
@@ -64,7 +64,6 @@ use serde::ser::SerializeSeq;
 use serde::{Serialize, Serializer};
 
 use crate::clean::{self, ItemId, RenderedLink, SelfTy};
-use crate::docfs::PathError;
 use crate::error::Error;
 use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
@@ -173,8 +172,12 @@ impl Serialize for TypeWithKind {
 crate struct StylePath {
     /// The path to the theme
     crate path: PathBuf,
-    /// What the `disabled` attribute should be set to in the HTML tag
-    crate disabled: bool,
+}
+
+impl StylePath {
+    pub fn basename(&self) -> Result<String, Error> {
+        Ok(try_none!(try_none!(self.path.file_stem(), &self.path).to_str(), &self.path).to_string())
+    }
 }
 
 fn write_srclink(cx: &Context<'_>, item: &clean::Item, buf: &mut Buffer) {
@@ -353,7 +356,7 @@ enum Setting {
         js_data_name: &'static str,
         description: &'static str,
         default_value: &'static str,
-        options: Vec<(String, String)>,
+        options: Vec<String>,
     },
 }
 
@@ -393,10 +396,9 @@ impl Setting {
                 options
                     .iter()
                     .map(|opt| format!(
-                        "<option value=\"{}\" {}>{}</option>",
-                        opt.0,
-                        if opt.0 == default_value { "selected" } else { "" },
-                        opt.1,
+                        "<option value=\"{name}\" {}>{name}</option>",
+                        if opt == default_value { "selected" } else { "" },
+                        name = opt,
                     ))
                     .collect::<String>(),
                 root_path,
@@ -421,18 +423,7 @@ impl<T: Into<Setting>> From<(&'static str, Vec<T>)> for Setting {
     }
 }
 
-fn settings(root_path: &str, suffix: &str, themes: &[StylePath]) -> Result<String, Error> {
-    let theme_names: Vec<(String, String)> = themes
-        .iter()
-        .map(|entry| {
-            let theme =
-                try_none!(try_none!(entry.path.file_stem(), &entry.path).to_str(), &entry.path)
-                    .to_string();
-
-            Ok((theme.clone(), theme))
-        })
-        .collect::<Result<_, Error>>()?;
-
+fn settings(root_path: &str, suffix: &str, theme_names: Vec<String>) -> Result<String, Error> {
     // (id, explanation, default value)
     let settings: &[Setting] = &[
         (
@@ -469,10 +460,11 @@ fn settings(root_path: &str, suffix: &str, themes: &[StylePath]) -> Result<Strin
             <span class=\"in-band\">Rustdoc settings</span>\
         </h1>\
         <div class=\"settings\">{}</div>\
-        <script src=\"{}settings{}.js\"></script>",
+        <link rel=\"stylesheet\" href=\"{root_path}settings{suffix}.css\">\
+        <script src=\"{root_path}settings{suffix}.js\"></script>",
         settings.iter().map(|s| s.display(root_path, suffix)).collect::<String>(),
-        root_path,
-        suffix
+        root_path = root_path,
+        suffix = suffix
     ))
 }
 
diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs
index 2d3b2490677e9..6b0f8dad4c501 100644
--- a/src/librustdoc/html/render/write_shared.rs
+++ b/src/librustdoc/html/render/write_shared.rs
@@ -228,12 +228,12 @@ pub(super) fn write_shared(
     let mut themes: FxHashSet<String> = FxHashSet::default();
 
     for entry in &cx.shared.style_files {
-        let theme = try_none!(try_none!(entry.path.file_stem(), &entry.path).to_str(), &entry.path);
+        let theme = entry.basename()?;
         let extension =
             try_none!(try_none!(entry.path.extension(), &entry.path).to_str(), &entry.path);
 
         // Handle the official themes
-        match theme {
+        match theme.as_str() {
             "light" => write_minify("light.css", static_files::themes::LIGHT, cx, options)?,
             "dark" => write_minify("dark.css", static_files::themes::DARK, cx, options)?,
             "ayu" => write_minify("ayu.css", static_files::themes::AYU, cx, options)?,
diff --git a/src/librustdoc/html/templates/page.html b/src/librustdoc/html/templates/page.html
index 9fafea6914524..d4d39cae8e802 100644
--- a/src/librustdoc/html/templates/page.html
+++ b/src/librustdoc/html/templates/page.html
@@ -12,7 +12,15 @@
     <link rel="stylesheet" type="text/css" {# -#}
           href="{{static_root_path | safe}}rustdoc{{page.resource_suffix}}.css" {# -#}
           id="mainThemeStyle"> {#- -#}
-    {{- style_files | safe -}}
+    {%- for theme in themes -%}
+        <link rel="stylesheet" type="text/css" {# -#}
+        {%- if theme == "light" -%}
+            id="themeStyle"
+        {%- else -%}
+            disabled
+        {%- endif -%}
+            href="{{static_root_path | safe}}{{theme}}{{page.resource_suffix}}.css"> {#- -#}
+    {%- endfor -%}
     <script id="default-settings" {# -#}
       {% for k, v in layout.default_settings %}
         data-{{k}}="{{v}}"