From d6059388866db3aac458053013944f1603e1991b Mon Sep 17 00:00:00 2001 From: Mathieu David Date: Sat, 29 Feb 2020 17:55:45 +0100 Subject: [PATCH 1/2] Bring back draft chapters --- book-example/src/SUMMARY.md | 1 + book-example/src/format/summary.md | 18 ++- src/book/book.rs | 104 +++++++++------ src/book/mod.rs | 48 +++---- src/book/summary.rs | 77 ++++++----- src/preprocess/index.rs | 14 +- src/preprocess/links.rs | 15 ++- src/renderer/html_handlebars/hbs_renderer.rs | 132 ++++++++++--------- src/renderer/html_handlebars/search.rs | 14 +- src/renderer/markdown_renderer.rs | 8 +- 10 files changed, 258 insertions(+), 173 deletions(-) diff --git a/book-example/src/SUMMARY.md b/book-example/src/SUMMARY.md index 4aab90f3..3ae143fb 100644 --- a/book-example/src/SUMMARY.md +++ b/book-example/src/SUMMARY.md @@ -10,6 +10,7 @@ - [clean](cli/clean.md) - [Format](format/README.md) - [SUMMARY.md](format/summary.md) + - [Draft chapter]() - [Configuration](format/config.md) - [Theme](format/theme/README.md) - [index.hbs](format/theme/index-hbs.md) diff --git a/book-example/src/format/summary.md b/book-example/src/format/summary.md index 71aa723b..61a2c6ec 100644 --- a/book-example/src/format/summary.md +++ b/book-example/src/format/summary.md @@ -7,7 +7,7 @@ are. Without this file, there is no book. Even though `SUMMARY.md` is a markdown file, the formatting is very strict to allow for easy parsing. Let's see how you should format your `SUMMARY.md` file. -#### Allowed elements +#### Structure 1. ***Title*** It's common practice to begin with a title, generally # Summary. But it is not mandatory, the @@ -36,3 +36,19 @@ allow for easy parsing. Let's see how you should format your `SUMMARY.md` file. All other elements are unsupported and will be ignored at best or result in an error. + +#### Other elements + +- ***Separators*** In between chapters you can add a separator. In the HTML renderer + this will result in a line being rendered in the table of contents. A separator is + a line containing exclusively dashes and at least three of them: `---`. +- ***Draft chapters*** Draft chapters are chapters without a file and thus content. + The purpose of a draft chapter is to signal future chapters still to be written. + Or when still laying out the structure of the book to avoid creating the files + while you are still changing the structure of the book a lot. + Draft chapters will be rendered in the HTML renderer as disabled links in the table + of contents, as you can see for the next chapter in the table of contents on the left. + Draft chapters are written like normal chapters but without writing the path to the file + ```markdown + - [Draft chapter]() + ``` \ No newline at end of file diff --git a/src/book/book.rs b/src/book/book.rs index 6a31c9e8..781331a9 100644 --- a/src/book/book.rs +++ b/src/book/book.rs @@ -39,17 +39,19 @@ fn create_missing(src_dir: &Path, summary: &Summary) -> Result<()> { let next = items.pop().expect("already checked"); if let SummaryItem::Link(ref link) = *next { - let filename = src_dir.join(&link.location); - if !filename.exists() { - if let Some(parent) = filename.parent() { - if !parent.exists() { - fs::create_dir_all(parent)?; + if let Some(ref location) = link.location { + let filename = src_dir.join(location); + if !filename.exists() { + if let Some(parent) = filename.parent() { + if !parent.exists() { + fs::create_dir_all(parent)?; + } } - } - debug!("Creating missing file {}", filename.display()); + debug!("Creating missing file {}", filename.display()); - let mut f = File::create(&filename)?; - writeln!(f, "# {}", link.name)?; + let mut f = File::create(&filename)?; + writeln!(f, "# {}", link.name)?; + } } items.extend(&link.nested_items); @@ -152,7 +154,7 @@ pub struct Chapter { /// Nested items. pub sub_items: Vec, /// The chapter's location, relative to the `SUMMARY.md` file. - pub path: PathBuf, + pub path: Option, /// An ordered list of the names of each chapter above this one, in the hierarchy. pub parent_names: Vec, } @@ -168,11 +170,31 @@ impl Chapter { Chapter { name: name.to_string(), content, - path: path.into(), + path: Some(path.into()), parent_names, ..Default::default() } } + + /// Create a new draft chapter that is not attached to a source markdown file and has + /// thus no content. + pub fn new_draft(name: &str, parent_names: Vec) -> Self { + Chapter { + name: name.to_string(), + content: String::new(), + path: None, + parent_names, + ..Default::default() + } + } + + /// Check if the chapter is a draft chapter, meaning it has no path to a source markdown file + pub fn is_draft_chapter(&self) -> bool { + match self.path { + Some(_) => false, + None => true, + } + } } /// Use the provided `Summary` to load a `Book` from disk. @@ -202,7 +224,7 @@ pub(crate) fn load_book_from_disk>(summary: &Summary, src_dir: P) }) } -fn load_summary_item>( +fn load_summary_item + Clone>( item: &SummaryItem, src_dir: P, parent_names: Vec, @@ -215,40 +237,46 @@ fn load_summary_item>( } } -fn load_chapter>( +fn load_chapter + Clone>( link: &Link, src_dir: P, parent_names: Vec, ) -> Result { - debug!("Loading {} ({})", link.name, link.location.display()); - let src_dir = src_dir.as_ref(); + let mut ch = if let Some(ref link_location) = link.location { + debug!("Loading {} ({})", link.name, link_location.display()); + let src_dir = src_dir.as_ref(); - let location = if link.location.is_absolute() { - link.location.clone() + let location = if link_location.is_absolute() { + link_location.clone() + } else { + src_dir.join(link_location) + }; + + let mut f = File::open(&location) + .chain_err(|| format!("Chapter file not found, {}", link_location.display()))?; + + let mut content = String::new(); + f.read_to_string(&mut content) + .chain_err(|| format!("Unable to read \"{}\" ({})", link.name, location.display()))?; + + let stripped = location + .strip_prefix(&src_dir) + .expect("Chapters are always inside a book"); + + Chapter::new(&link.name, content, stripped, parent_names.clone()) } else { - src_dir.join(&link.location) + Chapter::new_draft(&link.name, parent_names.clone()) }; - let mut f = File::open(&location) - .chain_err(|| format!("Chapter file not found, {}", link.location.display()))?; - - let mut content = String::new(); - f.read_to_string(&mut content) - .chain_err(|| format!("Unable to read \"{}\" ({})", link.name, location.display()))?; - - let stripped = location - .strip_prefix(&src_dir) - .expect("Chapters are always inside a book"); - let mut sub_item_parents = parent_names.clone(); - let mut ch = Chapter::new(&link.name, content, stripped, parent_names); + ch.number = link.number.clone(); sub_item_parents.push(link.name.clone()); let sub_items = link .nested_items .iter() - .map(|i| load_summary_item(i, src_dir, sub_item_parents.clone())) + .map(|i| load_summary_item(i, src_dir.clone(), sub_item_parents.clone())) .collect::>>()?; ch.sub_items = sub_items; @@ -376,7 +404,7 @@ And here is some \ name: String::from("Nested Chapter 1"), content: String::from("Hello World!"), number: Some(SectionNumber(vec![1, 2])), - path: PathBuf::from("second.md"), + path: Some(PathBuf::from("second.md")), parent_names: vec![String::from("Chapter 1")], sub_items: Vec::new(), }; @@ -384,7 +412,7 @@ And here is some \ name: String::from("Chapter 1"), content: String::from(DUMMY_SRC), number: None, - path: PathBuf::from("chapter_1.md"), + path: Some(PathBuf::from("chapter_1.md")), parent_names: Vec::new(), sub_items: vec![ BookItem::Chapter(nested.clone()), @@ -408,7 +436,7 @@ And here is some \ sections: vec![BookItem::Chapter(Chapter { name: String::from("Chapter 1"), content: String::from(DUMMY_SRC), - path: PathBuf::from("chapter_1.md"), + path: Some(PathBuf::from("chapter_1.md")), ..Default::default() })], ..Default::default() @@ -448,7 +476,7 @@ And here is some \ name: String::from("Chapter 1"), content: String::from(DUMMY_SRC), number: None, - path: PathBuf::from("Chapter_1/index.md"), + path: Some(PathBuf::from("Chapter_1/index.md")), parent_names: Vec::new(), sub_items: vec![ BookItem::Chapter(Chapter::new( @@ -500,7 +528,7 @@ And here is some \ name: String::from("Chapter 1"), content: String::from(DUMMY_SRC), number: None, - path: PathBuf::from("Chapter_1/index.md"), + path: Some(PathBuf::from("Chapter_1/index.md")), parent_names: Vec::new(), sub_items: vec![ BookItem::Chapter(Chapter::new( @@ -537,7 +565,7 @@ And here is some \ let summary = Summary { numbered_chapters: vec![SummaryItem::Link(Link { name: String::from("Empty"), - location: PathBuf::from(""), + location: Some(PathBuf::from("")), ..Default::default() })], ..Default::default() @@ -556,7 +584,7 @@ And here is some \ let summary = Summary { numbered_chapters: vec![SummaryItem::Link(Link { name: String::from("nested"), - location: dir, + location: Some(dir), ..Default::default() })], ..Default::default() diff --git a/src/book/mod.rs b/src/book/mod.rs index c2effe0f..2da96659 100644 --- a/src/book/mod.rs +++ b/src/book/mod.rs @@ -251,36 +251,38 @@ impl MDBook { for item in book.iter() { if let BookItem::Chapter(ref ch) = *item { - if !ch.path.as_os_str().is_empty() { - let path = self.source_dir().join(&ch.path); - info!("Testing file: {:?}", path); + if let Some(ref chapter_path) = ch.path { + if !chapter_path.as_os_str().is_empty() { + let path = self.source_dir().join(&chapter_path); + info!("Testing file: {:?}", path); - // write preprocessed file to tempdir - let path = temp_dir.path().join(&ch.path); - let mut tmpf = utils::fs::create_file(&path)?; - tmpf.write_all(ch.content.as_bytes())?; + // write preprocessed file to tempdir + let path = temp_dir.path().join(&chapter_path); + let mut tmpf = utils::fs::create_file(&path)?; + tmpf.write_all(ch.content.as_bytes())?; - let mut cmd = Command::new("rustdoc"); - cmd.arg(&path).arg("--test").args(&library_args); + let mut cmd = Command::new("rustdoc"); + cmd.arg(&path).arg("--test").args(&library_args); - if let Some(edition) = self.config.rust.edition { - match edition { - RustEdition::E2015 => { - cmd.args(&["--edition", "2015"]); - } - RustEdition::E2018 => { - cmd.args(&["--edition", "2018"]); + if let Some(edition) = self.config.rust.edition { + match edition { + RustEdition::E2015 => { + cmd.args(&["--edition", "2015"]); + } + RustEdition::E2018 => { + cmd.args(&["--edition", "2018"]); + } } } - } - let output = cmd.output()?; + let output = cmd.output()?; - if !output.status.success() { - bail!(ErrorKind::Subprocess( - "Rustdoc returned an error".to_string(), - output - )); + if !output.status.success() { + bail!(ErrorKind::Subprocess( + "Rustdoc returned an error".to_string(), + output + )); + } } } } diff --git a/src/book/summary.rs b/src/book/summary.rs index d9d6bcb5..8fc9e8fc 100644 --- a/src/book/summary.rs +++ b/src/book/summary.rs @@ -71,7 +71,7 @@ pub struct Link { pub name: String, /// The location of the chapter's source file, taking the book's `src` /// directory as the root. - pub location: PathBuf, + pub location: Option, /// The section number, if this chapter is in the numbered section. pub number: Option, /// Any nested items this chapter may contain. @@ -83,7 +83,7 @@ impl Link { pub fn new, P: AsRef>(name: S, location: P) -> Link { Link { name: name.into(), - location: location.as_ref().to_path_buf(), + location: Some(location.as_ref().to_path_buf()), number: None, nested_items: Vec::new(), } @@ -94,7 +94,7 @@ impl Default for Link { fn default() -> Self { Link { name: String::new(), - location: PathBuf::new(), + location: Some(PathBuf::new()), number: None, nested_items: Vec::new(), } @@ -260,7 +260,7 @@ impl<'a> SummaryParser<'a> { } } Some(Event::Start(Tag::Link(_type, href, _title))) => { - let link = self.parse_link(href.to_string())?; + let link = self.parse_link(href.to_string()); items.push(SummaryItem::Link(link)); } Some(Event::Rule) => items.push(SummaryItem::Separator), @@ -272,19 +272,21 @@ impl<'a> SummaryParser<'a> { Ok(items) } - fn parse_link(&mut self, href: String) -> Result { + fn parse_link(&mut self, href: String) -> Link { let link_content = collect_events!(self.stream, end Tag::Link(..)); let name = stringify_events(link_content); - if href.is_empty() { - Err(self.parse_error("You can't have an empty link.")) + let path = if href.is_empty() { + None } else { - Ok(Link { - name, - location: PathBuf::from(href), - number: None, - nested_items: Vec::new(), - }) + Some(PathBuf::from(href)) + }; + + Link { + name, + location: path, + number: None, + nested_items: Vec::new(), } } @@ -410,7 +412,7 @@ impl<'a> SummaryParser<'a> { match self.next_event() { Some(Event::Start(Tag::Paragraph)) => continue, Some(Event::Start(Tag::Link(_type, href, _title))) => { - let mut link = self.parse_link(href.to_string())?; + let mut link = self.parse_link(href.to_string()); let mut number = parent.clone(); number.0.push(num_existing_items as u32 + 1); @@ -418,7 +420,10 @@ impl<'a> SummaryParser<'a> { "Found chapter: {} {} ({})", number, link.name, - link.location.display() + link.location + .as_ref() + .map(|p| p.to_str().unwrap_or("")) + .unwrap_or("[draft]") ); link.number = Some(number); @@ -589,12 +594,12 @@ mod tests { let should_be = vec![ SummaryItem::Link(Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: Some(PathBuf::from("./first.md")), ..Default::default() }), SummaryItem::Link(Link { name: String::from("Second"), - location: PathBuf::from("./second.md"), + location: Some(PathBuf::from("./second.md")), ..Default::default() }), ]; @@ -633,7 +638,7 @@ mod tests { let src = "[First](./first.md)"; let should_be = Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: Some(PathBuf::from("./first.md")), ..Default::default() }; @@ -645,7 +650,7 @@ mod tests { other => panic!("Unreachable, {:?}", other), }; - let got = parser.parse_link(href).unwrap(); + let got = parser.parse_link(href); assert_eq!(got, should_be); } @@ -654,7 +659,7 @@ mod tests { let src = "- [First](./first.md)\n"; let link = Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: Some(PathBuf::from("./first.md")), number: Some(SectionNumber(vec![1])), ..Default::default() }; @@ -675,18 +680,18 @@ mod tests { let should_be = vec![ SummaryItem::Link(Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: Some(PathBuf::from("./first.md")), number: Some(SectionNumber(vec![1])), nested_items: vec![SummaryItem::Link(Link { name: String::from("Nested"), - location: PathBuf::from("./nested.md"), + location: Some(PathBuf::from("./nested.md")), number: Some(SectionNumber(vec![1, 1])), nested_items: Vec::new(), })], }), SummaryItem::Link(Link { name: String::from("Second"), - location: PathBuf::from("./second.md"), + location: Some(PathBuf::from("./second.md")), number: Some(SectionNumber(vec![2])), nested_items: Vec::new(), }), @@ -707,13 +712,13 @@ mod tests { let should_be = vec![ SummaryItem::Link(Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: Some(PathBuf::from("./first.md")), number: Some(SectionNumber(vec![1])), nested_items: Vec::new(), }), SummaryItem::Link(Link { name: String::from("Second"), - location: PathBuf::from("./second.md"), + location: Some(PathBuf::from("./second.md")), number: Some(SectionNumber(vec![2])), nested_items: Vec::new(), }), @@ -737,13 +742,13 @@ mod tests { let should_be = vec![ SummaryItem::Link(Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: Some(PathBuf::from("./first.md")), number: Some(SectionNumber(vec![1])), nested_items: Vec::new(), }), SummaryItem::Link(Link { name: String::from("Second"), - location: PathBuf::from("./second.md"), + location: Some(PathBuf::from("./second.md")), number: Some(SectionNumber(vec![2])), nested_items: Vec::new(), }), @@ -758,13 +763,21 @@ mod tests { } #[test] - fn an_empty_link_location_is_an_error() { + fn an_empty_link_location_is_a_draft_chapter() { let src = "- [Empty]()\n"; let mut parser = SummaryParser::new(src); parser.stream.next(); let got = parser.parse_numbered(); - assert!(got.is_err()); + let should_be = vec![SummaryItem::Link(Link { + name: String::from("Empty"), + location: None, + number: Some(SectionNumber(vec![1])), + nested_items: Vec::new(), + })]; + + assert!(got.is_ok()); + assert_eq!(got.unwrap(), should_be); } /// Regression test for https://github.com/rust-lang/mdBook/issues/779 @@ -776,21 +789,21 @@ mod tests { let should_be = vec![ SummaryItem::Link(Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: Some(PathBuf::from("./first.md")), number: Some(SectionNumber(vec![1])), nested_items: Vec::new(), }), SummaryItem::Separator, SummaryItem::Link(Link { name: String::from("Second"), - location: PathBuf::from("./second.md"), + location: Some(PathBuf::from("./second.md")), number: Some(SectionNumber(vec![2])), nested_items: Vec::new(), }), SummaryItem::Separator, SummaryItem::Link(Link { name: String::from("Third"), - location: PathBuf::from("./third.md"), + location: Some(PathBuf::from("./third.md")), number: Some(SectionNumber(vec![3])), nested_items: Vec::new(), }), diff --git a/src/preprocess/index.rs b/src/preprocess/index.rs index f3256b81..fd60ad4d 100644 --- a/src/preprocess/index.rs +++ b/src/preprocess/index.rs @@ -29,13 +29,15 @@ impl Preprocessor for IndexPreprocessor { let source_dir = ctx.root.join(&ctx.config.book.src); book.for_each_mut(|section: &mut BookItem| { if let BookItem::Chapter(ref mut ch) = *section { - if is_readme_file(&ch.path) { - let index_md = source_dir.join(ch.path.with_file_name("index.md")); - if index_md.exists() { - warn_readme_name_conflict(&ch.path, &index_md); - } + if let Some(ref mut path) = ch.path { + if is_readme_file(&path) { + let mut index_md = source_dir.join(path.with_file_name("index.md")); + if index_md.exists() { + warn_readme_name_conflict(&path, &&mut index_md); + } - ch.path.set_file_name("index.md"); + path.set_file_name("index.md"); + } } } }); diff --git a/src/preprocess/links.rs b/src/preprocess/links.rs index 408873aa..496b6d14 100644 --- a/src/preprocess/links.rs +++ b/src/preprocess/links.rs @@ -45,14 +45,15 @@ impl Preprocessor for LinkPreprocessor { book.for_each_mut(|section: &mut BookItem| { if let BookItem::Chapter(ref mut ch) = *section { - let base = ch - .path - .parent() - .map(|dir| src_dir.join(dir)) - .expect("All book items have a parent"); + if let Some(ref chapter_path) = ch.path { + let base = chapter_path + .parent() + .map(|dir| src_dir.join(dir)) + .expect("All book items have a parent"); - let content = replace_all(&ch.content, base, &ch.path, 0); - ch.content = content; + let content = replace_all(&ch.content, base, chapter_path, 0); + ch.content = content; + } } }); diff --git a/src/renderer/html_handlebars/hbs_renderer.rs b/src/renderer/html_handlebars/hbs_renderer.rs index cea3a9fd..8a98ff1e 100644 --- a/src/renderer/html_handlebars/hbs_renderer.rs +++ b/src/renderer/html_handlebars/hbs_renderer.rs @@ -31,75 +31,80 @@ impl HtmlHandlebars { ) -> Result<()> { // FIXME: This should be made DRY-er and rely less on mutable state if let BookItem::Chapter(ref ch) = *item { - let content = ch.content.clone(); - let content = utils::render_markdown(&content, ctx.html_config.curly_quotes); + if let Some(ref path) = ch.path { + let content = ch.content.clone(); + let content = utils::render_markdown(&content, ctx.html_config.curly_quotes); - let fixed_content = utils::render_markdown_with_path( - &ch.content, - ctx.html_config.curly_quotes, - Some(&ch.path), - ); - print_content.push_str(&fixed_content); + let fixed_content = utils::render_markdown_with_path( + &ch.content, + ctx.html_config.curly_quotes, + Some(&path), + ); + print_content.push_str(&fixed_content); - // Update the context with data for this file - let path = ch - .path - .to_str() - .chain_err(|| "Could not convert path to str")?; - let filepath = Path::new(&ch.path).with_extension("html"); + // Update the context with data for this file + let ctx_path = path + .to_str() + .chain_err(|| "Could not convert path to str")?; + let filepath = Path::new(&ctx_path).with_extension("html"); - // "print.html" is used for the print page. - if ch.path == Path::new("print.md") { - bail!(ErrorKind::ReservedFilenameError(ch.path.clone())); - }; + // "print.html" is used for the print page. + if path == Path::new("print.md") { + bail!(ErrorKind::ReservedFilenameError(path.clone())); + }; - // Non-lexical lifetimes needed :'( - let title: String; - { - let book_title = ctx - .data - .get("book_title") - .and_then(serde_json::Value::as_str) - .unwrap_or(""); + // Non-lexical lifetimes needed :'( + let title: String; + { + let book_title = ctx + .data + .get("book_title") + .and_then(serde_json::Value::as_str) + .unwrap_or(""); - title = match book_title { - "" => ch.name.clone(), - _ => ch.name.clone() + " - " + book_title, + title = match book_title { + "" => ch.name.clone(), + _ => ch.name.clone() + " - " + book_title, + } } - } - ctx.data.insert("path".to_owned(), json!(path)); - 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(), - json!(utils::fs::path_to_root(&ch.path)), - ); - if let Some(ref section) = ch.number { - ctx.data - .insert("section".to_owned(), json!(section.to_string())); - } + ctx.data.insert("path".to_owned(), json!(path)); + 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(), + json!(utils::fs::path_to_root(&path)), + ); + if let Some(ref section) = ch.number { + ctx.data + .insert("section".to_owned(), json!(section.to_string())); + } - // Render the handlebars template with the data - debug!("Render template"); - let rendered = ctx.handlebars.render("index", &ctx.data)?; + // Render the handlebars template with the data + debug!("Render template"); + let rendered = ctx.handlebars.render("index", &ctx.data)?; - let rendered = self.post_process(rendered, &ctx.html_config.playpen, ctx.edition); + let rendered = self.post_process(rendered, &ctx.html_config.playpen, ctx.edition); - // Write to file - debug!("Creating {}", filepath.display()); - utils::fs::write_file(&ctx.destination, &filepath, rendered.as_bytes())?; + // Write to file + debug!("Creating {}", filepath.display()); + utils::fs::write_file(&ctx.destination, &filepath, rendered.as_bytes())?; - if ctx.is_index { - ctx.data.insert("path".to_owned(), json!("index.md")); - ctx.data.insert("path_to_root".to_owned(), json!("")); - ctx.data.insert("is_index".to_owned(), json!("true")); - let rendered_index = ctx.handlebars.render("index", &ctx.data)?; - let rendered_index = - self.post_process(rendered_index, &ctx.html_config.playpen, ctx.edition); - debug!("Creating index.html from {}", path); - utils::fs::write_file(&ctx.destination, "index.html", rendered_index.as_bytes())?; + if ctx.is_index { + ctx.data.insert("path".to_owned(), json!("index.md")); + ctx.data.insert("path_to_root".to_owned(), json!("")); + ctx.data.insert("is_index".to_owned(), json!("true")); + let rendered_index = ctx.handlebars.render("index", &ctx.data)?; + let rendered_index = + self.post_process(rendered_index, &ctx.html_config.playpen, ctx.edition); + debug!("Creating index.html from {}", ctx_path); + utils::fs::write_file( + &ctx.destination, + "index.html", + rendered_index.as_bytes(), + )?; + } } } @@ -526,11 +531,12 @@ fn make_data( ); chapter.insert("name".to_owned(), json!(ch.name)); - let path = ch - .path - .to_str() - .chain_err(|| "Could not convert path to str")?; - chapter.insert("path".to_owned(), json!(path)); + if let Some(ref path) = ch.path { + let p = path + .to_str() + .chain_err(|| "Could not convert path to str")?; + chapter.insert("path".to_owned(), json!(p)); + } } BookItem::Separator => { chapter.insert("spacer".to_owned(), json!("_spacer_")); diff --git a/src/renderer/html_handlebars/search.rs b/src/renderer/html_handlebars/search.rs index f184a592..9ce93fa2 100644 --- a/src/renderer/html_handlebars/search.rs +++ b/src/renderer/html_handlebars/search.rs @@ -71,11 +71,21 @@ fn render_item( item: &BookItem, ) -> Result<()> { let chapter = match *item { - BookItem::Chapter(ref ch) => ch, + BookItem::Chapter(ref ch) => { + if let Some(_) = ch.path { + ch + } else { + return Ok(()); + } + } _ => return Ok(()), }; - let filepath = Path::new(&chapter.path).with_extension("html"); + let chapter_path = chapter + .path + .as_ref() + .expect("Checked that path exists above"); + let filepath = Path::new(&chapter_path).with_extension("html"); let filepath = filepath .to_str() .chain_err(|| "Could not convert HTML path to str")?; diff --git a/src/renderer/markdown_renderer.rs b/src/renderer/markdown_renderer.rs index e59436e7..77a73812 100644 --- a/src/renderer/markdown_renderer.rs +++ b/src/renderer/markdown_renderer.rs @@ -34,7 +34,13 @@ impl Renderer for MarkdownRenderer { trace!("markdown render"); for item in book.iter() { if let BookItem::Chapter(ref ch) = *item { - utils::fs::write_file(&ctx.destination, &ch.path, ch.content.as_bytes())?; + if !ch.is_draft_chapter() { + utils::fs::write_file( + &ctx.destination, + &ch.path.as_ref().expect("Checked path exists before"), + ch.content.as_bytes(), + )?; + } } } From 43008ef2ef305c1eac377d3de82ed3a857ac4936 Mon Sep 17 00:00:00 2001 From: Mathieu David Date: Mon, 9 Mar 2020 22:34:28 +0100 Subject: [PATCH 2/2] fix issues from code review --- src/book/book.rs | 7 +- src/book/mod.rs | 55 ++++---- src/renderer/html_handlebars/hbs_renderer.rs | 134 +++++++++---------- src/renderer/html_handlebars/search.rs | 8 +- 4 files changed, 99 insertions(+), 105 deletions(-) diff --git a/src/book/book.rs b/src/book/book.rs index 781331a9..1fb9e94b 100644 --- a/src/book/book.rs +++ b/src/book/book.rs @@ -237,14 +237,15 @@ fn load_summary_item + Clone>( } } -fn load_chapter + Clone>( +fn load_chapter>( link: &Link, src_dir: P, parent_names: Vec, ) -> Result { + let src_dir = src_dir.as_ref(); + let mut ch = if let Some(ref link_location) = link.location { debug!("Loading {} ({})", link.name, link_location.display()); - let src_dir = src_dir.as_ref(); let location = if link_location.is_absolute() { link_location.clone() @@ -276,7 +277,7 @@ fn load_chapter + Clone>( let sub_items = link .nested_items .iter() - .map(|i| load_summary_item(i, src_dir.clone(), sub_item_parents.clone())) + .map(|i| load_summary_item(i, src_dir, sub_item_parents.clone())) .collect::>>()?; ch.sub_items = sub_items; diff --git a/src/book/mod.rs b/src/book/mod.rs index 2da96659..5711eb5e 100644 --- a/src/book/mod.rs +++ b/src/book/mod.rs @@ -251,40 +251,41 @@ impl MDBook { for item in book.iter() { if let BookItem::Chapter(ref ch) = *item { - if let Some(ref chapter_path) = ch.path { - if !chapter_path.as_os_str().is_empty() { - let path = self.source_dir().join(&chapter_path); - info!("Testing file: {:?}", path); + let chapter_path = match ch.path { + Some(ref path) if !path.as_os_str().is_empty() => path, + _ => continue, + }; - // write preprocessed file to tempdir - let path = temp_dir.path().join(&chapter_path); - let mut tmpf = utils::fs::create_file(&path)?; - tmpf.write_all(ch.content.as_bytes())?; + let path = self.source_dir().join(&chapter_path); + info!("Testing file: {:?}", path); - let mut cmd = Command::new("rustdoc"); - cmd.arg(&path).arg("--test").args(&library_args); + // write preprocessed file to tempdir + let path = temp_dir.path().join(&chapter_path); + let mut tmpf = utils::fs::create_file(&path)?; + tmpf.write_all(ch.content.as_bytes())?; - if let Some(edition) = self.config.rust.edition { - match edition { - RustEdition::E2015 => { - cmd.args(&["--edition", "2015"]); - } - RustEdition::E2018 => { - cmd.args(&["--edition", "2018"]); - } - } + let mut cmd = Command::new("rustdoc"); + cmd.arg(&path).arg("--test").args(&library_args); + + if let Some(edition) = self.config.rust.edition { + match edition { + RustEdition::E2015 => { + cmd.args(&["--edition", "2015"]); } - - let output = cmd.output()?; - - if !output.status.success() { - bail!(ErrorKind::Subprocess( - "Rustdoc returned an error".to_string(), - output - )); + RustEdition::E2018 => { + cmd.args(&["--edition", "2018"]); } } } + + let output = cmd.output()?; + + if !output.status.success() { + bail!(ErrorKind::Subprocess( + "Rustdoc returned an error".to_string(), + output + )); + } } } Ok(()) diff --git a/src/renderer/html_handlebars/hbs_renderer.rs b/src/renderer/html_handlebars/hbs_renderer.rs index 8a98ff1e..5335247d 100644 --- a/src/renderer/html_handlebars/hbs_renderer.rs +++ b/src/renderer/html_handlebars/hbs_renderer.rs @@ -30,84 +30,82 @@ impl HtmlHandlebars { print_content: &mut String, ) -> Result<()> { // FIXME: This should be made DRY-er and rely less on mutable state - if let BookItem::Chapter(ref ch) = *item { - if let Some(ref path) = ch.path { - let content = ch.content.clone(); - let content = utils::render_markdown(&content, ctx.html_config.curly_quotes); - let fixed_content = utils::render_markdown_with_path( - &ch.content, - ctx.html_config.curly_quotes, - Some(&path), - ); - print_content.push_str(&fixed_content); + let (ch, path) = match item { + BookItem::Chapter(ch) if !ch.is_draft_chapter() => (ch, ch.path.as_ref().unwrap()), + _ => return Ok(()), + }; - // Update the context with data for this file - let ctx_path = path - .to_str() - .chain_err(|| "Could not convert path to str")?; - let filepath = Path::new(&ctx_path).with_extension("html"); + let content = ch.content.clone(); + let content = utils::render_markdown(&content, ctx.html_config.curly_quotes); - // "print.html" is used for the print page. - if path == Path::new("print.md") { - bail!(ErrorKind::ReservedFilenameError(path.clone())); - }; + let fixed_content = utils::render_markdown_with_path( + &ch.content, + ctx.html_config.curly_quotes, + Some(&path), + ); + print_content.push_str(&fixed_content); - // Non-lexical lifetimes needed :'( - let title: String; - { - let book_title = ctx - .data - .get("book_title") - .and_then(serde_json::Value::as_str) - .unwrap_or(""); + // Update the context with data for this file + let ctx_path = path + .to_str() + .chain_err(|| "Could not convert path to str")?; + let filepath = Path::new(&ctx_path).with_extension("html"); - title = match book_title { - "" => ch.name.clone(), - _ => ch.name.clone() + " - " + book_title, - } - } + // "print.html" is used for the print page. + if path == Path::new("print.md") { + bail!(ErrorKind::ReservedFilenameError(path.clone())); + }; - ctx.data.insert("path".to_owned(), json!(path)); - 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(), - json!(utils::fs::path_to_root(&path)), - ); - if let Some(ref section) = ch.number { - ctx.data - .insert("section".to_owned(), json!(section.to_string())); - } + // Non-lexical lifetimes needed :'( + let title: String; + { + let book_title = ctx + .data + .get("book_title") + .and_then(serde_json::Value::as_str) + .unwrap_or(""); - // Render the handlebars template with the data - debug!("Render template"); - let rendered = ctx.handlebars.render("index", &ctx.data)?; - - let rendered = self.post_process(rendered, &ctx.html_config.playpen, ctx.edition); - - // Write to file - debug!("Creating {}", filepath.display()); - utils::fs::write_file(&ctx.destination, &filepath, rendered.as_bytes())?; - - if ctx.is_index { - ctx.data.insert("path".to_owned(), json!("index.md")); - ctx.data.insert("path_to_root".to_owned(), json!("")); - ctx.data.insert("is_index".to_owned(), json!("true")); - let rendered_index = ctx.handlebars.render("index", &ctx.data)?; - let rendered_index = - self.post_process(rendered_index, &ctx.html_config.playpen, ctx.edition); - debug!("Creating index.html from {}", ctx_path); - utils::fs::write_file( - &ctx.destination, - "index.html", - rendered_index.as_bytes(), - )?; - } + title = match book_title { + "" => ch.name.clone(), + _ => ch.name.clone() + " - " + book_title, } } + ctx.data.insert("path".to_owned(), json!(path)); + 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(), + json!(utils::fs::path_to_root(&path)), + ); + if let Some(ref section) = ch.number { + ctx.data + .insert("section".to_owned(), json!(section.to_string())); + } + + // Render the handlebars template with the data + debug!("Render template"); + let rendered = ctx.handlebars.render("index", &ctx.data)?; + + let rendered = self.post_process(rendered, &ctx.html_config.playpen, ctx.edition); + + // Write to file + debug!("Creating {}", filepath.display()); + utils::fs::write_file(&ctx.destination, &filepath, rendered.as_bytes())?; + + if ctx.is_index { + ctx.data.insert("path".to_owned(), json!("index.md")); + ctx.data.insert("path_to_root".to_owned(), json!("")); + ctx.data.insert("is_index".to_owned(), json!("true")); + let rendered_index = ctx.handlebars.render("index", &ctx.data)?; + let rendered_index = + self.post_process(rendered_index, &ctx.html_config.playpen, ctx.edition); + debug!("Creating index.html from {}", ctx_path); + utils::fs::write_file(&ctx.destination, "index.html", rendered_index.as_bytes())?; + } + Ok(()) } diff --git a/src/renderer/html_handlebars/search.rs b/src/renderer/html_handlebars/search.rs index 9ce93fa2..c1ac7062 100644 --- a/src/renderer/html_handlebars/search.rs +++ b/src/renderer/html_handlebars/search.rs @@ -71,13 +71,7 @@ fn render_item( item: &BookItem, ) -> Result<()> { let chapter = match *item { - BookItem::Chapter(ref ch) => { - if let Some(_) = ch.path { - ch - } else { - return Ok(()); - } - } + BookItem::Chapter(ref ch) if !ch.is_draft_chapter() => ch, _ => return Ok(()), };