Support anchors in SUMMARY (#1173)

* Parse anchors in `SUMMARY.md`

* Support sub-links in SUMMARY with html renderer

* Add some tests for anchors

5 files changed, 84 insertions, 21 deletions

diff --git a/src/book/book.rs b/src/book/book.rs
index 6a31c9e8..1a7ba447 100644
--- a/src/book/book.rs
+++ b/src/book/book.rs
@@ -153,6 +153,8 @@ pub struct Chapter {
     pub sub_items: Vec<BookItem>,
     /// The chapter's location, relative to the `SUMMARY.md` file.
     pub path: PathBuf,
+    /// An optional anchor in the original link.
+    pub anchor: Option<String>,
     /// An ordered list of the names of each chapter above this one, in the hierarchy.
     pub parent_names: Vec<String>,
 }
@@ -243,6 +245,7 @@ fn load_chapter<P: AsRef<Path>>(
     let mut sub_item_parents = parent_names.clone();
     let mut ch = Chapter::new(&link.name, content, stripped, parent_names);
     ch.number = link.number.clone();
+    ch.anchor = link.anchor.clone();
 
     sub_item_parents.push(link.name.clone());
     let sub_items = link
@@ -320,7 +323,7 @@ And here is some \
             .write_all(DUMMY_SRC.as_bytes())
             .unwrap();
 
-        let link = Link::new("Chapter 1", chapter_path);
+        let link = Link::new("Chapter 1", chapter_path, None);
 
         (link, temp)
     }
@@ -336,7 +339,7 @@ And here is some \
             .write_all(b"Hello World!")
             .unwrap();
 
-        let mut second = Link::new("Nested Chapter 1", &second_path);
+        let mut second = Link::new("Nested Chapter 1", &second_path, None);
         second.number = Some(SectionNumber(vec![1, 2]));
 
         root.nested_items.push(second.clone().into());
@@ -362,7 +365,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", None);
 
         let got = load_chapter(&link, "", Vec::new());
         assert!(got.is_err());
@@ -379,6 +382,7 @@ And here is some \
             path: PathBuf::from("second.md"),
             parent_names: vec![String::from("Chapter 1")],
             sub_items: Vec::new(),
+            anchor: None,
         };
         let should_be = BookItem::Chapter(Chapter {
             name: String::from("Chapter 1"),
@@ -391,6 +395,7 @@ And here is some \
                 BookItem::Separator,
                 BookItem::Chapter(nested.clone()),
             ],
+            anchor: None,
         });
 
         let got = load_summary_item(&SummaryItem::Link(root), temp.path(), Vec::new()).unwrap();
@@ -465,6 +470,7 @@ And here is some \
                             Vec::new(),
                         )),
                     ],
+                    anchor: None,
                 }),
                 BookItem::Separator,
             ],
@@ -517,6 +523,7 @@ And here is some \
                             Vec::new(),
                         )),
                     ],
+                    anchor: None,
                 }),
                 BookItem::Separator,
             ],
diff --git a/src/book/summary.rs b/src/book/summary.rs
index 1e130537..253de5f7 100644
--- a/src/book/summary.rs
+++ b/src/book/summary.rs
@@ -72,6 +72,8 @@ pub struct Link {
     /// The location of the chapter's source file, taking the book's `src`
     /// directory as the root.
     pub location: PathBuf,
+    /// An optional anchor in the original link.
+    pub anchor: Option<String>,
     /// The section number, if this chapter is in the numbered section.
     pub number: Option<SectionNumber>,
     /// Any nested items this chapter may contain.
@@ -80,10 +82,15 @@ pub struct Link {
 
 impl Link {
     /// Create a new link with no nested items.
-    pub fn new<S: Into<String>, P: AsRef<Path>>(name: S, location: P) -> Link {
+    pub fn new<S: Into<String>, P: AsRef<Path>>(
+        name: S,
+        location: P,
+        anchor: Option<String>,
+    ) -> Link {
         Link {
             name: name.into(),
             location: location.as_ref().to_path_buf(),
+            anchor,
             number: None,
             nested_items: Vec::new(),
         }
@@ -95,6 +102,7 @@ impl Default for Link {
         Link {
             name: String::new(),
             location: PathBuf::new(),
+            anchor: None,
             number: None,
             nested_items: Vec::new(),
         }
@@ -276,15 +284,18 @@ impl<'a> SummaryParser<'a> {
         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."))
-        } else {
-            Ok(Link {
+        let mut split = href.splitn(2, '#');
+        let (href, anchor) = (split.next(), split.next());
+
+        match href {
+            Some(href) if !href.is_empty() => Ok(Link {
                 name,
                 location: PathBuf::from(href.to_string()),
+                anchor: anchor.map(String::from),
                 number: None,
                 nested_items: Vec::new(),
-            })
+            }),
+            _ => Err(self.parse_error("You can't have an empty link.")),
         }
     }
 
@@ -676,10 +687,12 @@ mod tests {
             SummaryItem::Link(Link {
                 name: String::from("First"),
                 location: PathBuf::from("./first.md"),
+                anchor: None,
                 number: Some(SectionNumber(vec![1])),
                 nested_items: vec![SummaryItem::Link(Link {
                     name: String::from("Nested"),
                     location: PathBuf::from("./nested.md"),
+                    anchor: None,
                     number: Some(SectionNumber(vec![1, 1])),
                     nested_items: Vec::new(),
                 })],
@@ -687,6 +700,7 @@ mod tests {
             SummaryItem::Link(Link {
                 name: String::from("Second"),
                 location: PathBuf::from("./second.md"),
+                anchor: None,
                 number: Some(SectionNumber(vec![2])),
                 nested_items: Vec::new(),
             }),
@@ -708,12 +722,14 @@ mod tests {
             SummaryItem::Link(Link {
                 name: String::from("First"),
                 location: PathBuf::from("./first.md"),
+                anchor: None,
                 number: Some(SectionNumber(vec![1])),
                 nested_items: Vec::new(),
             }),
             SummaryItem::Link(Link {
                 name: String::from("Second"),
                 location: PathBuf::from("./second.md"),
+                anchor: None,
                 number: Some(SectionNumber(vec![2])),
                 nested_items: Vec::new(),
             }),
@@ -727,6 +743,26 @@ mod tests {
         assert_eq!(got, should_be);
     }
 
+    #[test]
+    fn parse_anchors() {
+        let src = "- [Link to anchor](./page.md#Foo)";
+
+        let should_be = vec![SummaryItem::Link(Link {
+            name: String::from("Link to anchor"),
+            location: PathBuf::from("./page.md"),
+            anchor: Some("Foo".to_string()),
+            number: Some(SectionNumber(vec![1])),
+            nested_items: Vec::new(),
+        })];
+
+        let mut parser = SummaryParser::new(src);
+        let _ = parser.stream.next();
+
+        let got = parser.parse_numbered().unwrap();
+
+        assert_eq!(got, should_be);
+    }
+
     /// This test ensures the book will continue to pass because it breaks the
     /// `SUMMARY.md` up using level 2 headers ([example]).
     ///
@@ -738,12 +774,14 @@ mod tests {
             SummaryItem::Link(Link {
                 name: String::from("First"),
                 location: PathBuf::from("./first.md"),
+                anchor: None,
                 number: Some(SectionNumber(vec![1])),
                 nested_items: Vec::new(),
             }),
             SummaryItem::Link(Link {
                 name: String::from("Second"),
                 location: PathBuf::from("./second.md"),
+                anchor: None,
                 number: Some(SectionNumber(vec![2])),
                 nested_items: Vec::new(),
             }),
@@ -759,12 +797,13 @@ mod tests {
 
     #[test]
     fn an_empty_link_location_is_an_error() {
-        let src = "- [Empty]()\n";
-        let mut parser = SummaryParser::new(src);
-        parser.stream.next();
+        for src in &["- [Empty]()\n", "- [Empty](#Foo)\n"] {
+            let mut parser = SummaryParser::new(src);
+            parser.stream.next();
 
-        let got = parser.parse_numbered();
-        assert!(got.is_err());
+            let got = parser.parse_numbered();
+            assert!(got.is_err());
+        }
     }
 
     /// Regression test for https://github.com/rust-lang/mdBook/issues/779
@@ -779,6 +818,7 @@ mod tests {
                 location: PathBuf::from("./first.md"),
                 number: Some(SectionNumber(vec![1])),
                 nested_items: Vec::new(),
+                anchor: None,
             }),
             SummaryItem::Separator,
             SummaryItem::Link(Link {
@@ -786,11 +826,13 @@ mod tests {
                 location: PathBuf::from("./second.md"),
                 number: Some(SectionNumber(vec![2])),
                 nested_items: Vec::new(),
+                anchor: None,
             }),
             SummaryItem::Separator,
             SummaryItem::Link(Link {
                 name: String::from("Third"),
                 location: PathBuf::from("./third.md"),
+                anchor: None,
                 number: Some(SectionNumber(vec![3])),
                 nested_items: Vec::new(),
             }),
diff --git a/src/renderer/html_handlebars/hbs_renderer.rs b/src/renderer/html_handlebars/hbs_renderer.rs
index bf27ec34..665222f2 100644
--- a/src/renderer/html_handlebars/hbs_renderer.rs
+++ b/src/renderer/html_handlebars/hbs_renderer.rs
@@ -31,8 +31,7 @@ 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);
+            let content = utils::render_markdown(&ch.content, ctx.html_config.curly_quotes);
 
             let fixed_content = utils::render_markdown_with_path(
                 &ch.content,
@@ -81,6 +80,10 @@ impl HtmlHandlebars {
                     .insert("section".to_owned(), json!(section.to_string()));
             }
 
+            if let Some(anchor) = &ch.anchor {
+                ctx.data.insert("anchor".to_owned(), json!(anchor));
+            }
+
             // Render the handlebars template with the data
             debug!("Render template");
             let rendered = ctx.handlebars.render("index", &ctx.data)?;
@@ -520,6 +523,10 @@ fn make_data(
                     .to_str()
                     .chain_err(|| "Could not convert path to str")?;
                 chapter.insert("path".to_owned(), json!(path));
+
+                if let Some(anchor) = &ch.anchor {
+                    chapter.insert("anchor".to_owned(), json!(anchor));
+                }
             }
             BookItem::Separator => {
                 chapter.insert("spacer".to_owned(), json!("_spacer_"));
diff --git a/src/renderer/html_handlebars/helpers/navigation.rs b/src/renderer/html_handlebars/helpers/navigation.rs
index 9d9ea76d..3cd0bbf1 100644
--- a/src/renderer/html_handlebars/helpers/navigation.rs
+++ b/src/renderer/html_handlebars/helpers/navigation.rs
@@ -72,8 +72,8 @@ fn find_chapter(
             Target::Next => match chapters
                 .iter()
                 .filter(|chapter| {
-                    // Skip things like "spacer"
-                    chapter.contains_key("path")
+                    // Skip things like "spacer" or sub-links
+                    chapter.contains_key("path") && !chapter.contains_key("anchor")
                 })
                 .skip(1)
                 .next()
@@ -90,7 +90,7 @@ fn find_chapter(
 
     for item in chapters {
         match item.get("path") {
-            Some(path) if !path.is_empty() => {
+            Some(path) if !path.is_empty() && item.get("anchor").is_none() => {
                 if let Some(previous) = previous {
                     if let Some(item) = target.find(&base_path, &path, &item, &previous)? {
                         return Ok(Some(item));
diff --git a/src/renderer/html_handlebars/helpers/toc.rs b/src/renderer/html_handlebars/helpers/toc.rs
index b77c7e94..e7c07740 100644
--- a/src/renderer/html_handlebars/helpers/toc.rs
+++ b/src/renderer/html_handlebars/helpers/toc.rs
@@ -111,7 +111,7 @@ impl HelperDef for RenderToc {
                 if !path.is_empty() {
                     out.write("<a href=\"")?;
 
-                    let tmp = Path::new(item.get("path").expect("Error: path should be Some(_)"))
+                    let tmp = Path::new(path)
                         .with_extension("html")
                         .to_str()
                         .unwrap()
@@ -121,9 +121,16 @@ impl HelperDef for RenderToc {
                     // Add link
                     out.write(&utils::fs::path_to_root(&current_path))?;
                     out.write(&tmp)?;
+
+                    let anchor = item.get("anchor");
+                    if let Some(anchor) = anchor {
+                        out.write("#")?;
+                        out.write(anchor)?;
+                    }
+
                     out.write("\"")?;
 
-                    if path == &current_path {
+                    if anchor.is_none() && path == &current_path {
                         out.write(" class=\"active\"")?;
                     }