Merge pull request #696 from mattico/fix-theme-dir

Fix default theme dir selection

src/renderer/html_handlebars/hbs_renderer.rs

@@ -1,9 +1,9 @@
 use book::{Book, BookItem, Chapter};
 use config::{Config, HtmlConfig, Playpen};
 use errors::*;
-use renderer::{RenderContext, Renderer};
 use renderer::html_handlebars::helpers;
-use theme::{self, Theme, playpen_editor};
+use renderer::{RenderContext, Renderer};
+use theme::{self, playpen_editor, Theme};
 use utils;
 
 use std::collections::BTreeMap;
@@ -41,9 +41,9 @@ impl HtmlHandlebars {
                 let path = ch.path
                     .to_str()
                     .chain_err(|| "Could not convert path to str")?;
-                let filepath = Path::new(&ch.path)
+                let filepath = Path::new(&ch.path).with_extension("html");
-                    .with_extension("html");
+                let filepathstr = filepath
-                let filepathstr = filepath.to_str()
+                    .to_str()
                     .chain_err(|| "Could not convert HTML path to str")?;
                 let filepathstr = utils::fs::normalize_path(filepathstr);
 
@@ -56,9 +56,9 @@ impl HtmlHandlebars {
                 let title: String;
                 {
                     let book_title = ctx.data
-                                        .get("book_title")
+                        .get("book_title")
-                                        .and_then(serde_json::Value::as_str)
+                        .and_then(serde_json::Value::as_str)
-                                        .unwrap_or("");
+                        .unwrap_or("");
                     title = ch.name.clone() + " - " + book_title;
                 }
 
@@ -66,18 +66,16 @@ impl HtmlHandlebars {
                 ctx.data.insert("content".to_owned(), json!(content));
                 ctx.data.insert("chapter_title".to_owned(), json!(ch.name));
                 ctx.data.insert("title".to_owned(), json!(title));
-                ctx.data.insert("path_to_root".to_owned(),
+                ctx.data.insert(
-                                json!(utils::fs::path_to_root(&ch.path)));
+                    "path_to_root".to_owned(),
+                    json!(utils::fs::path_to_root(&ch.path)),
+                );
 
                 // Render the handlebars template with the data
                 debug!("Render template");
                 let rendered = ctx.handlebars.render("index", &ctx.data)?;
 
-                let rendered = self.post_process(
+                let rendered = self.post_process(rendered, &filepathstr, &ctx.html_config.playpen);
-                    rendered,
-                    &filepathstr,
-                    &ctx.html_config.playpen,
-                );
 
                 // Write to file
                 debug!("Creating {} ✓", filepathstr);
@@ -105,10 +103,11 @@ impl HtmlHandlebars {
         // This could cause a problem when someone displays
         // code containing <base href=...>
         // on the front page, however this case should be very very rare...
-        content = content.lines()
+        content = content
-                         .filter(|line| !line.contains("<base href="))
+            .lines()
-                         .collect::<Vec<&str>>()
+            .filter(|line| !line.contains("<base href="))
-                         .join("\n");
+            .collect::<Vec<&str>>()
+            .join("\n");
 
         utils::fs::write_file(destination, "index.html", content.as_bytes())?;
 
@@ -121,11 +120,7 @@ impl HtmlHandlebars {
     }
 
     #[cfg_attr(feature = "cargo-clippy", allow(let_and_return))]
-    fn post_process(&self,
+    fn post_process(&self, rendered: String, filepath: &str, playpen_config: &Playpen) -> String {
-                    rendered: String,
-                    filepath: &str,
-                    playpen_config: &Playpen)
-                    -> String {
         let rendered = build_header_links(&rendered, filepath);
         let rendered = fix_anchor_links(&rendered, filepath);
         let rendered = fix_code_blocks(&rendered);
@@ -198,7 +193,8 @@ impl HtmlHandlebars {
             write_file(destination, "ace.js", playpen_editor::ACE_JS)?;
             write_file(destination, "mode-rust.js", playpen_editor::MODE_RUST_JS)?;
             write_file(destination, "theme-dawn.js", playpen_editor::THEME_DAWN_JS)?;
-            write_file(destination,
+            write_file(
+                destination,
                 "theme-tomorrow_night.js",
                 playpen_editor::THEME_TOMORROW_NIGHT_JS,
             )?;
@@ -208,28 +204,42 @@ impl HtmlHandlebars {
     }
 
     /// Update the context with data for this file
-    fn configure_print_version(&self,
+    fn configure_print_version(
-                               data: &mut serde_json::Map<String, serde_json::Value>,
+        &self,
-                               print_content: &str) {
+        data: &mut serde_json::Map<String, serde_json::Value>,
+        print_content: &str,
+    ) {
         // Make sure that the Print chapter does not display the title from
         // the last rendered chapter by removing it from its context
         data.remove("title");
         data.insert("is_print".to_owned(), json!(true));
         data.insert("path".to_owned(), json!("print.md"));
         data.insert("content".to_owned(), json!(print_content));
-        data.insert("path_to_root".to_owned(),
+        data.insert(
-                    json!(utils::fs::path_to_root(Path::new("print.md"))));
+            "path_to_root".to_owned(),
+            json!(utils::fs::path_to_root(Path::new("print.md"))),
+        );
     }
 
     fn register_hbs_helpers(&self, handlebars: &mut Handlebars, html_config: &HtmlConfig) {
-        handlebars.register_helper("toc", Box::new(helpers::toc::RenderToc {no_section_label: html_config.no_section_label}));
+        handlebars.register_helper(
+            "toc",
+            Box::new(helpers::toc::RenderToc {
+                no_section_label: html_config.no_section_label,
+            }),
+        );
         handlebars.register_helper("previous", Box::new(helpers::navigation::previous));
         handlebars.register_helper("next", Box::new(helpers::navigation::next));
     }
 
     /// Copy across any additional CSS and JavaScript files which the book
     /// has been configured to use.
-    fn copy_additional_css_and_js(&self, html: &HtmlConfig, root: &Path, destination: &Path) -> Result<()> {
+    fn copy_additional_css_and_js(
+        &self,
+        html: &HtmlConfig,
+        root: &Path,
+        destination: &Path,
+    ) -> Result<()> {
         let custom_files = html.additional_css.iter().chain(html.additional_js.iter());
 
         debug!("Copying additional CSS and JS");
@@ -260,6 +270,25 @@ impl HtmlHandlebars {
     }
 }
 
+// TODO(mattico): Remove some time after the 0.1.8 release
+fn maybe_wrong_theme_dir(dir: &Path) -> Result<bool> {
+    fn entry_is_maybe_book_file(entry: fs::DirEntry) -> Result<bool> {
+        Ok(entry.file_type()?.is_file()
+            && entry.path().extension().map_or(false, |ext| ext == "md"))
+    }
+
+    if dir.is_dir() {
+        for entry in fs::read_dir(dir)? {
+            if entry_is_maybe_book_file(entry?).unwrap_or(false) {
+                return Ok(false);
+            }
+        }
+        Ok(true)
+    } else {
+        Ok(false)
+    }
+}
+
 impl Renderer for HtmlHandlebars {
     fn name(&self) -> &str {
         "html"
@@ -276,9 +305,19 @@ impl Renderer for HtmlHandlebars {
 
         let theme_dir = match html_config.theme {
             Some(ref theme) => theme.to_path_buf(),
-            None => src_dir.join("theme"),
+            None => ctx.root.join("theme"),
         };
 
+        if html_config.theme.is_none()
+            && maybe_wrong_theme_dir(&src_dir.join("theme")).unwrap_or(false)
+        {
+            warn!(
+                "Previous versions of mdBook erroneously accepted `./src/theme` as an automatic \
+                 theme directory"
+            );
+            warn!("Please move your theme files to `./theme` for them to continue being used");
+        }
+
         let theme = theme::Theme::new(theme_dir);
 
         debug!("Register the index handlebars template");
@@ -307,9 +346,7 @@ impl Renderer for HtmlHandlebars {
                 is_index: is_index,
                 html_config: html_config.clone(),
             };
-            self.render_item(item,
+            self.render_item(item, ctx, &mut print_content)?;
-                             ctx,
-                             &mut print_content)?;
             is_index = false;
         }
 
@@ -323,9 +360,7 @@ impl Renderer for HtmlHandlebars {
         debug!("Render template");
         let rendered = handlebars.render("index", &data)?;
 
-        let rendered = self.post_process(rendered,
+        let rendered = self.post_process(rendered, "print.html", &html_config.playpen);
-                                         "print.html",
-                                         &html_config.playpen);
 
         utils::fs::write_file(&destination, "print.html", &rendered.into_bytes())?;
         debug!("Creating print.html ✓");
@@ -347,14 +382,25 @@ impl Renderer for HtmlHandlebars {
     }
 }
 
-fn make_data(root: &Path, book: &Book, config: &Config, html_config: &HtmlConfig) -> Result<serde_json::Map<String, serde_json::Value>> {
+fn make_data(
+    root: &Path,
+    book: &Book,
+    config: &Config,
+    html_config: &HtmlConfig,
+) -> Result<serde_json::Map<String, serde_json::Value>> {
     trace!("make_data");
     let html = config.html_config().unwrap_or_default();
 
     let mut data = serde_json::Map::new();
     data.insert("language".to_owned(), json!("en"));
-    data.insert("book_title".to_owned(), json!(config.book.title.clone().unwrap_or_default()));
+    data.insert(
-    data.insert("description".to_owned(), json!(config.book.description.clone().unwrap_or_default()));
+        "book_title".to_owned(),
+        json!(config.book.title.clone().unwrap_or_default()),
+    );
+    data.insert(
+        "description".to_owned(),
+        json!(config.book.description.clone().unwrap_or_default()),
+    );
     data.insert("favicon".to_owned(), json!("favicon.png"));
     if let Some(ref livereload) = html_config.livereload_url {
         data.insert("livereload".to_owned(), json!(livereload));
@@ -374,13 +420,8 @@ fn make_data(root: &Path, book: &Book, config: &Config, html_config: &HtmlConfig
         let mut css = Vec::new();
         for style in &html.additional_css {
             match style.strip_prefix(root) {
-                Ok(p) => {
+                Ok(p) => css.push(p.to_str().expect("Could not convert to str")),
-                    css.push(p.to_str().expect("Could not convert to str"))
+                Err(_) => css.push(style.to_str().expect("Could not convert to str")),
-                },
-                Err(_) => {
-                    css.push(style.to_str()
-                                  .expect("Could not convert to str"))
-                }
             }
         }
         data.insert("additional_css".to_owned(), json!(css));
@@ -392,12 +433,13 @@ fn make_data(root: &Path, book: &Book, config: &Config, html_config: &HtmlConfig
         for script in &html.additional_js {
             match script.strip_prefix(root) {
                 Ok(p) => js.push(p.to_str().expect("Could not convert to str")),
-                Err(_) => {
+                Err(_) => js.push(
-                    js.push(script.file_name()
+                    script
-                                  .expect("File has a file name")
+                        .file_name()
-                                  .to_str()
+                        .expect("File has a file name")
-                                  .expect("Could not convert to str"))
+                        .to_str()
-                }
+                        .expect("Could not convert to str"),
+                ),
             }
         }
         data.insert("additional_js".to_owned(), json!(js));
@@ -415,8 +457,10 @@ fn make_data(root: &Path, book: &Book, config: &Config, html_config: &HtmlConfig
         }
     } else if search.is_some() {
         warn!("mdBook compiled without search support, ignoring `output.html.search` table");
-        warn!("please reinstall with `cargo install mdbook --force --features search`\
+        warn!(
-            to use the search feature")
+            "please reinstall with `cargo install mdbook --force --features search`to use the \
+             search feature"
+        )
     }
 
     let mut chapters = vec![];
@@ -457,22 +501,25 @@ fn build_header_links(html: &str, filepath: &str) -> String {
     let regex = Regex::new(r"<h(\d)>(.*?)</h\d>").unwrap();
     let mut id_counter = HashMap::new();
 
-    regex.replace_all(html, |caps: &Captures| {
+    regex
-        let level = caps[1].parse()
+        .replace_all(html, |caps: &Captures| {
-                           .expect("Regex should ensure we only ever get numbers here");
+            let level = caps[1]
+                .parse()
+                .expect("Regex should ensure we only ever get numbers here");
 
-        wrap_header_with_link(level, &caps[2], &mut id_counter, filepath)
+            wrap_header_with_link(level, &caps[2], &mut id_counter, filepath)
-    })
+        })
-         .into_owned()
+        .into_owned()
 }
 
 /// Wraps a single header tag with a link, making sure each tag gets its own
 /// unique ID by appending an auto-incremented number (if necessary).
-fn wrap_header_with_link(level: usize,
+fn wrap_header_with_link(
-                         content: &str,
+    level: usize,
-                         id_counter: &mut HashMap<String, usize>,
+    content: &str,
-                         filepath: &str)
+    id_counter: &mut HashMap<String, usize>,
-                         -> String {
+    filepath: &str,
+) -> String {
     let raw_id = utils::id_from_content(content);
 
     let id_count = id_counter.entry(raw_id.clone()).or_insert(0);
@@ -498,21 +545,23 @@ fn wrap_header_with_link(level: usize,
 // that in a very inelegant way
 fn fix_anchor_links(html: &str, filepath: &str) -> String {
     let regex = Regex::new(r##"<a([^>]+)href="#([^"]+)"([^>]*)>"##).unwrap();
-    regex.replace_all(html, |caps: &Captures| {
+    regex
-        let before = &caps[1];
+        .replace_all(html, |caps: &Captures| {
-        let anchor = &caps[2];
+            let before = &caps[1];
-        let after = &caps[3];
+            let anchor = &caps[2];
+            let after = &caps[3];
 
-        format!("<a{before}href=\"{filepath}#{anchor}\"{after}>",
+            format!(
+                "<a{before}href=\"{filepath}#{anchor}\"{after}>",
                 before = before,
                 filepath = filepath,
                 anchor = anchor,
-                after = after)
+                after = after
-    })
+            )
-         .into_owned()
+        })
+        .into_owned()
 }
 
-
 // The rust book uses annotations for rustdoc to test code snippets,
 // like the following:
 // ```rust,should_panic
@@ -523,51 +572,54 @@ fn fix_anchor_links(html: &str, filepath: &str) -> String {
 // This function replaces all commas by spaces in the code block classes
 fn fix_code_blocks(html: &str) -> String {
     let regex = Regex::new(r##"<code([^>]+)class="([^"]+)"([^>]*)>"##).unwrap();
-    regex.replace_all(html, |caps: &Captures| {
+    regex
-        let before = &caps[1];
+        .replace_all(html, |caps: &Captures| {
-        let classes = &caps[2].replace(",", " ");
+            let before = &caps[1];
-        let after = &caps[3];
+            let classes = &caps[2].replace(",", " ");
+            let after = &caps[3];
 
-        format!(r#"<code{before}class="{classes}"{after}>"#,
+            format!(
+                r#"<code{before}class="{classes}"{after}>"#,
                 before = before,
                 classes = classes,
-                after = after)
+                after = after
-    }).into_owned()
+            )
+        })
+        .into_owned()
 }
 
 fn add_playpen_pre(html: &str, playpen_config: &Playpen) -> String {
     let regex = Regex::new(r##"((?s)<code[^>]?class="([^"]+)".*?>(.*?)</code>)"##).unwrap();
-    regex.replace_all(html, |caps: &Captures| {
+    regex
-        let text = &caps[1];
+        .replace_all(html, |caps: &Captures| {
-        let classes = &caps[2];
+            let text = &caps[1];
-        let code = &caps[3];
+            let classes = &caps[2];
+            let code = &caps[3];
 
-        if (classes.contains("language-rust") && !classes.contains("ignore")) ||
+            if (classes.contains("language-rust") && !classes.contains("ignore"))
-            classes.contains("mdbook-runnable")
+                || classes.contains("mdbook-runnable")
-        {
-            // wrap the contents in an external pre block
-            if playpen_config.editable && classes.contains("editable") ||
-                text.contains("fn main") || text.contains("quick_main!")
             {
-                format!("<pre class=\"playpen\">{}</pre>", text)
+                // wrap the contents in an external pre block
-            } else {
+                if playpen_config.editable && classes.contains("editable")
-                // we need to inject our own main
+                    || text.contains("fn main") || text.contains("quick_main!")
-                let (attrs, code) = partition_source(code);
+                {
+                    format!("<pre class=\"playpen\">{}</pre>", text)
+                } else {
+                    // we need to inject our own main
+                    let (attrs, code) = partition_source(code);
 
-                format!("<pre class=\"playpen\"><code class=\"{}\">\n# \
+                    format!(
-                         #![allow(unused_variables)]\n\
+                        "<pre class=\"playpen\"><code class=\"{}\">\n# \
-                         {}#fn main() {{\n\
+                         #![allow(unused_variables)]\n{}#fn main() {{\n{}#}}</code></pre>",
-                         {}\
+                        classes, attrs, code
-                         #}}</code></pre>",
+                    )
-                        classes,
+                }
-                        attrs,
+            } else {
-                        code)
+                // not language-rust, so no-op
+                text.to_owned()
             }
-        } else {
+        })
-            // not language-rust, so no-op
+        .into_owned()
-            text.to_owned()
-        }
-    }).into_owned()
 }
 
 fn partition_source(s: &str) -> (String, String) {

tests/rendered_output.rs

@@ -18,7 +18,7 @@ use select::document::Document;
 use select::predicate::{Class, Name, Predicate};
 use tempfile::Builder as TempFileBuilder;
 use mdbook::errors::*;
-use mdbook::utils::fs::file_to_string;
+use mdbook::utils::fs::{file_to_string, write_file};
 use mdbook::config::Config;
 use mdbook::MDBook;
 
@@ -370,6 +370,24 @@ fn by_default_mdbook_use_index_preprocessor_to_convert_readme_to_index() {
     assert_doesnt_contain_strings(&second_index, &unexpected_strings);
 }
 
+#[test]
+fn theme_dir_overrides_work_correctly() {
+    let book_dir = dummy_book::new_copy_of_example_book().unwrap();
+    let book_dir = book_dir.path();
+    let theme_dir = book_dir.join("theme");
+
+    let mut index = ::mdbook::theme::INDEX.to_vec();
+    index.extend_from_slice(b"\n<!-- This is a modified index.hbs! -->");
+
+    write_file(&theme_dir, "index.hbs", &index).unwrap();
+
+    let md = MDBook::load(book_dir).unwrap();
+    md.build().unwrap();
+
+    let built_index = book_dir.join("book").join("index.html");
+    dummy_book::assert_contains_strings(built_index, &["This is a modified index.hbs!"]);
+}
+
 #[cfg(feature = "search")]
 mod search {
     extern crate serde_json;