From dcb7946110c55fe7195d33b4146729c7c3bd3407 Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Fri, 26 Jan 2018 21:40:28 +0100 Subject: [PATCH] Convert project to use RelativePath where appropriate. --- Cargo.toml | 1 + src/book/book.rs | 56 +++++++-------- src/book/mod.rs | 6 +- src/book/summary.rs | 30 ++++---- src/lib.rs | 5 +- src/preprocess/links.rs | 6 +- src/renderer/html_handlebars/hbs_renderer.rs | 74 +++++++++----------- src/utils/fs.rs | 44 +----------- 8 files changed, 86 insertions(+), 136 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d05d3737..45f0cca5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ tempdir = "0.3.4" itertools = "0.7" shlex = "0.1" toml-query = "0.6" +relative-path = { version = "0.3", features = ["serde"] } # Watch feature notify = { version = "4.0", optional = true } diff --git a/src/book/book.rs b/src/book/book.rs index adb49ade..dc634671 100644 --- a/src/book/book.rs +++ b/src/book/book.rs @@ -1,5 +1,5 @@ use std::fmt::{self, Display, Formatter}; -use std::path::{Path, PathBuf}; +use std::path::{Path}; use std::collections::VecDeque; use std::fs::{self, File}; use std::io::{Read, Write}; @@ -7,6 +7,7 @@ use std::io::{Read, Write}; use super::summary::{parse_summary, Link, SectionNumber, Summary, SummaryItem}; use config::BuildConfig; use errors::*; +use relative_path::RelativePathBuf; /// Load a book into memory from its `src/` directory. pub fn load_book>(src_dir: P, cfg: &BuildConfig) -> Result { @@ -39,7 +40,8 @@ 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); + let filename = link.location.to_path(src_dir); + if !filename.exists() { if let Some(parent) = filename.parent() { if !parent.exists() { @@ -150,13 +152,15 @@ pub struct Chapter { pub number: Option, /// Nested items. pub sub_items: Vec, - /// The chapter's location, relative to the `SUMMARY.md` file. - pub path: PathBuf, + /// The chapter's relative location. + pub path: RelativePathBuf, } impl Chapter { /// Create a new chapter with the provided content. - pub fn new>(name: &str, content: String, path: P) -> Chapter { + pub fn new>( + name: &str, content: String, path: P, + ) -> Chapter { Chapter { name: name.to_string(), content: content, @@ -201,24 +205,16 @@ fn load_chapter>(link: &Link, src_dir: P) -> Result { debug!("Loading {} ({})", link.name, link.location.display()); let src_dir = src_dir.as_ref(); - let location = if link.location.is_absolute() { - link.location.clone() - } else { - src_dir.join(&link.location) - }; + let location = &link.location; - let mut f = File::open(&location) + let mut f = File::open(location.to_path(src_dir)) .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 ch = Chapter::new(&link.name, content, stripped); + let mut ch = Chapter::new(&link.name, content, &link.location); ch.number = link.number.clone(); let sub_items = link.nested_items @@ -289,8 +285,9 @@ And here is some \ fn dummy_link() -> (Link, TempDir) { let temp = TempDir::new("book").unwrap(); - let chapter_path = temp.path().join("chapter_1.md"); - File::create(&chapter_path) + let chapter_path = RelativePathBuf::from("chapter_1.md"); + + File::create(&chapter_path.to_path(temp.path())) .unwrap() .write(DUMMY_SRC.as_bytes()) .unwrap(); @@ -304,9 +301,9 @@ And here is some \ fn nested_links() -> (Link, TempDir) { let (mut root, temp_dir) = dummy_link(); - let second_path = temp_dir.path().join("second.md"); + let second_path = RelativePathBuf::from("second.md"); - File::create(&second_path) + File::create(&second_path.to_path(&temp_dir)) .unwrap() .write_all("Hello World!".as_bytes()) .unwrap(); @@ -332,7 +329,7 @@ And here is some \ #[test] fn cant_load_a_nonexistent_chapter() { - let link = Link::new("Chapter 1", "/foo/bar/baz.md"); + let link = Link::new("Chapter 1", "foo/bar/baz.md"); let got = load_chapter(&link, ""); assert!(got.is_err()); @@ -346,14 +343,14 @@ 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: RelativePathBuf::from("second.md"), sub_items: Vec::new(), }; let should_be = BookItem::Chapter(Chapter { name: String::from("Chapter 1"), content: String::from(DUMMY_SRC), number: None, - path: PathBuf::from("chapter_1.md"), + path: RelativePathBuf::from("chapter_1.md"), sub_items: vec![ BookItem::Chapter(nested.clone()), BookItem::Separator, @@ -377,7 +374,7 @@ And here is some \ BookItem::Chapter(Chapter { name: String::from("Chapter 1"), content: String::from(DUMMY_SRC), - path: PathBuf::from("chapter_1.md"), + path: RelativePathBuf::from("chapter_1.md"), ..Default::default() }), ], @@ -416,7 +413,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: RelativePathBuf::from("Chapter_1/index.md"), sub_items: vec![ BookItem::Chapter(Chapter::new( "Hello World", @@ -463,7 +460,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: RelativePathBuf::from("Chapter_1/index.md"), sub_items: vec![ BookItem::Chapter(Chapter::new( "Hello World", @@ -497,7 +494,7 @@ And here is some \ numbered_chapters: vec![ SummaryItem::Link(Link { name: String::from("Empty"), - location: PathBuf::from(""), + location: RelativePathBuf::from(""), ..Default::default() }), ], @@ -511,14 +508,13 @@ And here is some \ #[test] fn cant_load_chapters_when_the_link_is_a_directory() { let (_, temp) = dummy_link(); - let dir = temp.path().join("nested"); - fs::create_dir(&dir).unwrap(); + fs::create_dir(temp.path().join("nested")).unwrap(); let summary = Summary { numbered_chapters: vec![ SummaryItem::Link(Link { name: String::from("nested"), - location: dir, + location: RelativePathBuf::from("nested"), ..Default::default() }), ], diff --git a/src/book/mod.rs b/src/book/mod.rs index 141b6cf7..3c3b9008 100644 --- a/src/book/mod.rs +++ b/src/book/mod.rs @@ -221,13 +221,13 @@ impl MDBook { for item in self.iter() { if let BookItem::Chapter(ref ch) = *item { - if !ch.path.as_os_str().is_empty() { - let path = self.source_dir().join(&ch.path); + if !ch.path.as_str().is_empty() { + let path = ch.path.to_path(self.source_dir()); let content = utils::fs::file_to_string(&path)?; info!("Testing file: {:?}", path); // write preprocessed file to tempdir - let path = temp_dir.path().join(&ch.path); + let path = ch.path.to_path(temp_dir.path()); let mut tmpf = utils::fs::create_file(&path)?; tmpf.write_all(content.as_bytes())?; diff --git a/src/book/summary.rs b/src/book/summary.rs index 700b4ce5..79725ab5 100644 --- a/src/book/summary.rs +++ b/src/book/summary.rs @@ -1,9 +1,9 @@ use std::fmt::{self, Display, Formatter}; use std::iter::FromIterator; use std::ops::{Deref, DerefMut}; -use std::path::{Path, PathBuf}; use memchr::{self, Memchr}; use pulldown_cmark::{self, Alignment, Event, Tag}; +use relative_path::{RelativePath, RelativePathBuf}; use errors::*; /// Parse the text from a `SUMMARY.md` file into a sort of "recipe" to be @@ -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: RelativePathBuf, /// The section number, if this chapter is in the numbered section. pub number: Option, /// Any nested items this chapter may contain. @@ -80,10 +80,10 @@ pub struct Link { impl Link { /// Create a new link with no nested items. - pub fn new, P: AsRef>(name: S, location: P) -> Link { + pub fn new, P: AsRef>(name: S, location: P) -> Link { Link { name: name.into(), - location: location.as_ref().to_path_buf(), + location: location.as_ref().to_relative_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: RelativePathBuf::new(), number: None, nested_items: Vec::new(), } @@ -277,7 +277,7 @@ impl<'a> SummaryParser<'a> { } else { Ok(Link { name: name, - location: PathBuf::from(href.to_string()), + location: RelativePathBuf::from(href.to_string()), number: None, nested_items: Vec::new(), }) @@ -617,12 +617,12 @@ mod tests { let should_be = vec![ SummaryItem::Link(Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: RelativePathBuf::from("./first.md"), ..Default::default() }), SummaryItem::Link(Link { name: String::from("Second"), - location: PathBuf::from("./second.md"), + location: RelativePathBuf::from("./second.md"), ..Default::default() }), ]; @@ -661,7 +661,7 @@ mod tests { let src = "[First](./first.md)"; let should_be = Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: RelativePathBuf::from("./first.md"), ..Default::default() }; @@ -682,7 +682,7 @@ mod tests { let src = "- [First](./first.md)\n"; let link = Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: RelativePathBuf::from("./first.md"), number: Some(SectionNumber(vec![1])), ..Default::default() }; @@ -703,12 +703,12 @@ mod tests { let should_be = vec![ SummaryItem::Link(Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: RelativePathBuf::from("./first.md"), number: Some(SectionNumber(vec![1])), nested_items: vec![ SummaryItem::Link(Link { name: String::from("Nested"), - location: PathBuf::from("./nested.md"), + location: RelativePathBuf::from("./nested.md"), number: Some(SectionNumber(vec![1, 1])), nested_items: Vec::new(), }), @@ -716,7 +716,7 @@ mod tests { }), SummaryItem::Link(Link { name: String::from("Second"), - location: PathBuf::from("./second.md"), + location: RelativePathBuf::from("./second.md"), number: Some(SectionNumber(vec![2])), nested_items: Vec::new(), }), @@ -740,13 +740,13 @@ mod tests { let should_be = vec![ SummaryItem::Link(Link { name: String::from("First"), - location: PathBuf::from("./first.md"), + location: RelativePathBuf::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: RelativePathBuf::from("./second.md"), number: Some(SectionNumber(vec![2])), nested_items: Vec::new(), }), diff --git a/src/lib.rs b/src/lib.rs index 559cec30..ee9e1e3f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -95,6 +95,7 @@ extern crate shlex; extern crate tempdir; extern crate toml; extern crate toml_query; +extern crate relative_path; #[cfg(test)] #[macro_use] @@ -114,7 +115,7 @@ pub use config::Config; /// The error types used through out this crate. pub mod errors { - use std::path::PathBuf; + use relative_path::RelativePathBuf; error_chain!{ foreign_links { @@ -142,7 +143,7 @@ pub mod errors { } /// The user tried to use a reserved filename. - ReservedFilenameError(filename: PathBuf) { + ReservedFilenameError(filename: RelativePathBuf) { description("Reserved Filename") display("{} is reserved for internal use", filename.display()) } diff --git a/src/preprocess/links.rs b/src/preprocess/links.rs index fda175cb..1657a3ad 100644 --- a/src/preprocess/links.rs +++ b/src/preprocess/links.rs @@ -31,10 +31,10 @@ impl Preprocessor for LinkPreprocessor { book.for_each_mut(|section: &mut BookItem| { if let BookItem::Chapter(ref mut ch) = *section { - let base = ch.path + let base = ch.path.to_path(&src_dir) .parent() - .map(|dir| src_dir.join(dir)) - .expect("All book items have a parent"); + .expect("All book items have a parent") + .to_owned(); let content = replace_all(&ch.content, base); ch.content = content; diff --git a/src/renderer/html_handlebars/hbs_renderer.rs b/src/renderer/html_handlebars/hbs_renderer.rs index 6535a4a3..c811cc76 100644 --- a/src/renderer/html_handlebars/hbs_renderer.rs +++ b/src/renderer/html_handlebars/hbs_renderer.rs @@ -15,6 +15,7 @@ use std::collections::BTreeMap; use std::collections::HashMap; use handlebars::Handlebars; +use relative_path::RelativePath; use serde_json; @@ -26,13 +27,13 @@ impl HtmlHandlebars { HtmlHandlebars } - fn write_file>( + fn write_file>( &self, build_dir: &Path, - filename: P, + path: P, content: &[u8], ) -> Result<()> { - let path = build_dir.join(filename); + let path = path.as_ref().to_path(build_dir); utils::fs::create_file(&path)? .write_all(content) @@ -41,8 +42,8 @@ impl HtmlHandlebars { fn render_item( &self, - item: &BookItem, - mut ctx: RenderItemContext, + item: &BookItem, + mut ctx: RenderItemContext, print_content: &mut String, ) -> Result<()> { // FIXME: This should be made DRY-er and rely less on mutable state @@ -50,15 +51,11 @@ impl HtmlHandlebars { BookItem::Chapter(ref ch) => { let content = ch.content.clone(); let content = utils::render_markdown(&content, ctx.html_config.curly_quotes); + print_content.push_str(&content); - // Update the context with data for this file - let path = ch.path - .to_str() - .chain_err(|| "Could not convert path to str")?; - // "print.html" is used for the print page. - if ch.path == Path::new("print.md") { + if ch.path == RelativePath::new("print.md") { bail!(ErrorKind::ReservedFilenameError(ch.path.clone())); }; @@ -72,23 +69,35 @@ impl HtmlHandlebars { title = ch.name.clone() + " - " + book_title; } - ctx.data.insert("path".to_owned(), json!(path)); + // NB: normalize would translate something like: `a/.././b/c/d` to + // `b/c/d`, if we then skip one and translate we get `../..`. + let path_to_root = ch.path + .normalize().components().skip(1).map(|_| "..").collect::>() + .join("/"); + + // TODO: remove trailing slash (and this block), it is only needed to have + // string-perfect backwards compatibility, but `` works the same without it. + let path_to_root = if !path_to_root.is_empty() { + format!("{}/", path_to_root) + } else { + path_to_root + }; + + ctx.data.insert("path".to_owned(), json!(ch.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))); + ctx.data.insert("path_to_root".to_owned(), json!(path_to_root)); // Render the handlebars template with the data debug!("Render template"); let rendered = ctx.handlebars.render("index", &ctx.data)?; - let filepath = Path::new(&ch.path).with_extension("html"); + let filepath = ch.path.with_extension("html"); + let rendered = self.post_process( rendered, - &normalize_path(filepath.to_str().ok_or_else(|| { - Error::from(format!("Bad file name: {}", filepath.display())) - })?), + filepath.as_str(), &ctx.html_config.playpen, ); @@ -112,8 +121,9 @@ impl HtmlHandlebars { let mut content = String::new(); - File::open(destination.join(&ch.path.with_extension("html")))? - .read_to_string(&mut content)?; + let path = ch.path.with_extension("html").to_path(destination); + + File::open(&path)?.read_to_string(&mut content)?; // This could cause a problem when someone displays // code containing @@ -125,10 +135,7 @@ impl HtmlHandlebars { self.write_file(destination, "index.html", content.as_bytes())?; - debug!( - "Creating index.html from {} ✓", - destination.join(&ch.path.with_extension("html")).display() - ); + debug!("Creating index.html from {} ✓", path.display()); Ok(()) } @@ -226,8 +233,7 @@ impl HtmlHandlebars { 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(), - json!(utils::fs::path_to_root(Path::new("print.md")))); + data.insert("path_to_root".to_owned(), json!("")); } fn register_hbs_helpers(&self, handlebars: &mut Handlebars, html_config: &HtmlConfig) { @@ -329,9 +335,7 @@ impl Renderer for HtmlHandlebars { let rendered = handlebars.render("index", &data)?; - let rendered = self.post_process(rendered, - "print.html", - &html_config.playpen); + let rendered = self.post_process(rendered, "print.html", &html_config.playpen); self.write_file(&destination, "print.html", &rendered.into_bytes())?; debug!("Creating print.html ✓"); @@ -428,10 +432,7 @@ fn make_data(root: &Path, book: &Book, config: &Config, html_config: &HtmlConfig } 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)); + chapter.insert("path".to_owned(), json!(ch.path)); } BookItem::Separator => { chapter.insert("spacer".to_owned(), json!("_spacer_")); @@ -624,13 +625,6 @@ struct RenderItemContext<'a> { html_config: HtmlConfig, } -pub fn normalize_path(path: &str) -> String { - use std::path::is_separator; - path.chars() - .map(|ch| if is_separator(ch) { '/' } else { ch }) - .collect::() -} - pub fn normalize_id(content: &str) -> String { content.chars() .filter_map(|ch| if ch.is_alphanumeric() || ch == '_' || ch == '-' { diff --git a/src/utils/fs.rs b/src/utils/fs.rs index 9189eeb2..cbcd8bbb 100644 --- a/src/utils/fs.rs +++ b/src/utils/fs.rs @@ -1,4 +1,4 @@ -use std::path::{Component, Path, PathBuf}; +use std::path::Path; use errors::*; use std::io::Read; use std::fs::{self, File}; @@ -16,48 +16,6 @@ pub fn file_to_string>(path: P) -> Result { Ok(content) } -/// Takes a path and returns a path containing just enough `../` to point to -/// the root of the given path. -/// -/// This is mostly interesting for a relative path to point back to the -/// directory from where the path starts. -/// -/// ```rust -/// # extern crate mdbook; -/// # -/// # use std::path::Path; -/// # use mdbook::utils::fs::path_to_root; -/// # -/// # fn main() { -/// let path = Path::new("some/relative/path"); -/// assert_eq!(path_to_root(path), "../../"); -/// # } -/// ``` -/// -/// **note:** it's not very fool-proof, if you find a situation where -/// it doesn't return the correct path. -/// Consider [submitting a new issue](https://github.com/rust-lang-nursery/mdBook/issues) -/// or a [pull-request](https://github.com/rust-lang-nursery/mdBook/pulls) to improve it. - -pub fn path_to_root>(path: P) -> String { - debug!("path_to_root"); - // Remove filename and add "../" for every directory - - path.into() - .parent() - .expect("") - .components() - .fold(String::new(), |mut s, c| { - match c { - Component::Normal(_) => s.push_str("../"), - _ => { - debug!("Other path component... {:?}", c); - } - } - s - }) -} - /// This function creates a file and returns it. But before creating the file /// it checks every directory in the path to see if it exists, /// and if it does not it will be created.