Merge pull request #1266 from ehuss/favicon-no-default

Don't copy default favicon if at least one is overridden.
rust-lang · Jul 3, 2020 · 649f355 · 649f355
2 parents 8432df1 + b0c6f2d
commit 649f355
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 14 deletions.
diff --git a/src/renderer/html_handlebars/hbs_renderer.rs b/src/renderer/html_handlebars/hbs_renderer.rs
@@ -192,8 +192,12 @@ impl HtmlHandlebars {
         write_file(destination, "css/chrome.css", &theme.chrome_css)?;
         write_file(destination, "css/print.css", &theme.print_css)?;
         write_file(destination, "css/variables.css", &theme.variables_css)?;
-        write_file(destination, "favicon.png", &theme.favicon_png)?;
-        write_file(destination, "favicon.svg", &theme.favicon_svg)?;
+        if let Some(contents) = &theme.favicon_png {
+            write_file(destination, "favicon.png", &contents)?;
+        }
+        if let Some(contents) = &theme.favicon_svg {
+            write_file(destination, "favicon.svg", &contents)?;
+        }
         write_file(destination, "highlight.css", &theme.highlight_css)?;
         write_file(destination, "tomorrow-night.css", &theme.tomorrow_night_css)?;
         write_file(destination, "ayu-highlight.css", &theme.ayu_highlight_css)?;

diff --git a/src/theme/mod.rs b/src/theme/mod.rs
@@ -54,8 +54,8 @@ pub struct Theme {
     pub general_css: Vec<u8>,
     pub print_css: Vec<u8>,
     pub variables_css: Vec<u8>,
-    pub favicon_png: Vec<u8>,
-    pub favicon_svg: Vec<u8>,
+    pub favicon_png: Option<Vec<u8>>,
+    pub favicon_svg: Option<Vec<u8>>,
     pub js: Vec<u8>,
     pub highlight_css: Vec<u8>,
     pub tomorrow_night_css: Vec<u8>,
@@ -91,8 +91,6 @@ impl Theme {
                     theme_dir.join("css/variables.css"),
                     &mut theme.variables_css,
                 ),
-                (theme_dir.join("favicon.png"), &mut theme.favicon_png),
-                (theme_dir.join("favicon.svg"), &mut theme.favicon_svg),
                 (theme_dir.join("highlight.js"), &mut theme.highlight_js),
                 (theme_dir.join("clipboard.min.js"), &mut theme.clipboard_js),
                 (theme_dir.join("highlight.css"), &mut theme.highlight_css),
@@ -106,13 +104,36 @@ impl Theme {
                 ),
             ];
 
-            for (filename, dest) in files {
+            let load_with_warn = |filename: &Path, dest| {
                 if !filename.exists() {
-                    continue;
+                    // Don't warn if the file doesn't exist.
+                    return false;
                 }
-
-                if let Err(e) = load_file_contents(&filename, dest) {
+                if let Err(e) = load_file_contents(filename, dest) {
                     warn!("Couldn't load custom file, {}: {}", filename.display(), e);
+                    false
+                } else {
+                    true
+                }
+            };
+
+            for (filename, dest) in files {
+                load_with_warn(&filename, dest);
+            }
+
+            // If the user overrides one favicon, but not the other, do not
+            // copy the default for the other.
+            let favicon_png = &mut theme.favicon_png.as_mut().unwrap();
+            let png = load_with_warn(&theme_dir.join("favicon.png"), favicon_png);
+            let favicon_svg = &mut theme.favicon_svg.as_mut().unwrap();
+            let svg = load_with_warn(&theme_dir.join("favicon.svg"), favicon_svg);
+            match (png, svg) {
+                (true, true) | (false, false) => {}
+                (true, false) => {
+                    theme.favicon_svg = None;
+                }
+                (false, true) => {
+                    theme.favicon_png = None;
                 }
             }
         }
@@ -132,8 +153,8 @@ impl Default for Theme {
             general_css: GENERAL_CSS.to_owned(),
             print_css: PRINT_CSS.to_owned(),
             variables_css: VARIABLES_CSS.to_owned(),
-            favicon_png: FAVICON_PNG.to_owned(),
-            favicon_svg: FAVICON_SVG.to_owned(),
+            favicon_png: Some(FAVICON_PNG.to_owned()),
+            favicon_svg: Some(FAVICON_SVG.to_owned()),
             js: JS.to_owned(),
             highlight_css: HIGHLIGHT_CSS.to_owned(),
             tomorrow_night_css: TOMORROW_NIGHT_CSS.to_owned(),
@@ -219,8 +240,8 @@ mod tests {
             general_css: Vec::new(),
             print_css: Vec::new(),
             variables_css: Vec::new(),
-            favicon_png: Vec::new(),
-            favicon_svg: Vec::new(),
+            favicon_png: Some(Vec::new()),
+            favicon_svg: Some(Vec::new()),
             js: Vec::new(),
             highlight_css: Vec::new(),
             tomorrow_night_css: Vec::new(),
@@ -231,4 +252,19 @@ mod tests {
 
         assert_eq!(got, empty);
     }
+
+    #[test]
+    fn favicon_override() {
+        let temp = TempFileBuilder::new().prefix("mdbook-").tempdir().unwrap();
+        fs::write(temp.path().join("favicon.png"), "1234").unwrap();
+        let got = Theme::new(temp.path());
+        assert_eq!(got.favicon_png.as_ref().unwrap(), b"1234");
+        assert_eq!(got.favicon_svg, None);
+
+        let temp = TempFileBuilder::new().prefix("mdbook-").tempdir().unwrap();
+        fs::write(temp.path().join("favicon.svg"), "4567").unwrap();
+        let got = Theme::new(temp.path());
+        assert_eq!(got.favicon_png, None);
+        assert_eq!(got.favicon_svg.as_ref().unwrap(), b"4567");
+    }
 }