Replace navigation helpers with objects

This replaces the `{{#previous}}` and `{{#next}}` handelbars helpers
with simple objects that contain the previous and next values. These
helpers have been a bit fussy to work with and have caused issues in the
past. This drops a large amount of somewhat fragile code with something
that is a bit simpler.

Additionally, this switches the previous/next arrows to use an `{{#if}}`
instead CSS trickery which may help with upcoming changes to
font-awesome.

crates/mdbook-html/front-end/css/chrome.css

@@ -186,10 +186,6 @@ html:not(.js) .left-buttons button {
     left: var(--page-padding);
 }
 
-/* Use the correct buttons for RTL layouts*/
-[dir=rtl] .previous i.fa-angle-left:before {content:"\f105";}
-[dir=rtl] .next i.fa-angle-right:before { content:"\f104"; }
-
 @media only screen and (max-width: 1080px) {
     .nav-wide-wrapper { display: none; }
     .nav-wrapper { display: block; }

crates/mdbook-html/front-end/templates/index.hbs

@@ -221,17 +221,25 @@
 
                     <nav class="nav-wrapper" aria-label="Page navigation">
                         <!-- Mobile navigation buttons -->
-                        {{#previous}}
+                        {{#if previous}}
-                            <a rel="prev" href="{{ path_to_root }}{{link}}" class="mobile-nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
+                            <a rel="prev" href="{{ path_to_root }}{{previous.link}}" class="mobile-nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
-                                <i class="fa fa-angle-left"></i>
+                                {{#if (eq ../text_direction "rtl")}}
-                            </a>
-                        {{/previous}}
-
-                        {{#next}}
-                            <a rel="next prefetch" href="{{ path_to_root }}{{link}}" class="mobile-nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
                                 <i class="fa fa-angle-right"></i>
+                                {{else}}
+                                <i class="fa fa-angle-left"></i>
+                                {{/if}}
                             </a>
-                        {{/next}}
+                        {{/if}}
+
+                        {{#if next}}
+                            <a rel="next prefetch" href="{{ path_to_root }}{{next.link}}" class="mobile-nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
+                                {{#if (eq ../text_direction "rtl")}}
+                                <i class="fa fa-angle-left"></i>
+                                {{else}}
+                                <i class="fa fa-angle-right"></i>
+                                {{/if}}
+                            </a>
+                        {{/if}}
 
                         <div style="clear: both"></div>
                     </nav>
@@ -239,17 +247,25 @@
             </div>
 
             <nav class="nav-wide-wrapper" aria-label="Page navigation">
-                {{#previous}}
+                {{#if previous}}
-                    <a rel="prev" href="{{ path_to_root }}{{link}}" class="nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
+                    <a rel="prev" href="{{ path_to_root }}{{previous.link}}" class="nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
-                        <i class="fa fa-angle-left"></i>
+                        {{#if (eq ../text_direction "rtl")}}
-                    </a>
-                {{/previous}}
-
-                {{#next}}
-                    <a rel="next prefetch" href="{{ path_to_root }}{{link}}" class="nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
                         <i class="fa fa-angle-right"></i>
+                        {{else}}
+                        <i class="fa fa-angle-left"></i>
+                        {{/if}}
                     </a>
-                {{/next}}
+                {{/if}}
+
+                {{#if next}}
+                    <a rel="next prefetch" href="{{ path_to_root }}{{next.link}}" class="nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
+                        {{#if (eq text_direction "rtl")}}
+                        <i class="fa fa-angle-left"></i>
+                        {{else}}
+                        <i class="fa fa-angle-right"></i>
+                        {{/if}}
+                    </a>
+                {{/if}}
             </nav>
 
         </div>

crates/mdbook-html/src/html_handlebars/hbs_renderer.rs

@@ -4,7 +4,7 @@ use crate::theme::Theme;
 use anyhow::{Context, Result, bail};
 use handlebars::Handlebars;
 use log::{debug, info, trace, warn};
-use mdbook_core::book::{Book, BookItem};
+use mdbook_core::book::{Book, BookItem, Chapter};
 use mdbook_core::config::{BookConfig, Code, Config, HtmlConfig, Playground, RustEdition};
 use mdbook_core::utils;
 use mdbook_core::utils::fs::get_404_output_file;
@@ -30,18 +30,17 @@ impl HtmlHandlebars {
         HtmlHandlebars
     }
 
-    fn render_item(
+    fn render_chapter(
         &self,
-        item: &BookItem,
+        ch: &Chapter,
-        mut ctx: RenderItemContext<'_>,
+        prev_ch: Option<&Chapter>,
+        next_ch: Option<&Chapter>,
+        mut ctx: RenderChapterContext<'_>,
         print_content: &mut String,
     ) -> Result<()> {
         // FIXME: This should be made DRY-er and rely less on mutable state
 
-        let (ch, path) = match item {
+        let path = ch.path.as_ref().unwrap();
-            BookItem::Chapter(ch) if !ch.is_draft_chapter() => (ch, ch.path.as_ref().unwrap()),
-            _ => return Ok(()),
-        };
 
         if let Some(ref edit_url_template) = ctx.html_config.edit_url_template {
             let full_path = ctx.book_config.src.to_str().unwrap_or_default().to_owned()
@@ -61,7 +60,7 @@ impl HtmlHandlebars {
 
         let fixed_content =
             render_markdown_with_path(&ch.content, ctx.html_config.smart_punctuation, Some(path));
-        if !ctx.is_index && ctx.html_config.print.page_break {
+        if prev_ch.is_some() && ctx.html_config.print.page_break {
             // Add page break between chapters
             // See https://developer.mozilla.org/en-US/docs/Web/CSS/break-before and https://developer.mozilla.org/en-US/docs/Web/CSS/page-break-before
             // Add both two CSS properties because of the compatibility issue
@@ -116,6 +115,25 @@ impl HtmlHandlebars {
             );
         }
 
+        let mut nav = |name: &str, ch: Option<&Chapter>| {
+            let Some(ch) = ch else { return };
+            let path = ch
+                .path
+                .as_ref()
+                .unwrap()
+                .with_extension("html")
+                .to_str()
+                .unwrap()
+                .replace('\\', "//");
+            let obj = json!( {
+                "title": ch.name,
+                "link": path,
+            });
+            ctx.data.insert(name.to_string(), obj);
+        };
+        nav("previous", prev_ch);
+        nav("next", next_ch);
+
         // Render the handlebars template with the data
         debug!("Render template");
         let rendered = ctx.handlebars.render("index", &ctx.data)?;
@@ -131,7 +149,7 @@ impl HtmlHandlebars {
         debug!("Creating {}", filepath.display());
         utils::fs::write_file(&ctx.destination, &filepath, rendered.as_bytes())?;
 
-        if ctx.is_index {
+        if prev_ch.is_none() {
             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));
@@ -253,8 +271,6 @@ impl HtmlHandlebars {
                 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));
         // TODO: remove theme_option in 0.5, it is not needed.
         handlebars.register_helper("theme_option", Box::new(helpers::theme::theme_option));
     }
@@ -442,21 +458,26 @@ impl Renderer for HtmlHandlebars {
             utils::fs::write_file(destination, "CNAME", format!("{cname}\n").as_bytes())?;
         }
 
-        let mut is_index = true;
+        let chapters: Vec<_> = book
-        for item in book.iter() {
+            .iter()
-            let ctx = RenderItemContext {
+            .filter_map(|item| match item {
+                BookItem::Chapter(ch) if !ch.is_draft_chapter() => Some(ch),
+                _ => None,
+            })
+            .collect();
+        for (i, ch) in chapters.iter().enumerate() {
+            let previous = (i != 0).then(|| chapters[i - 1]);
+            let next = (i != chapters.len() - 1).then(|| chapters[i + 1]);
+            let ctx = RenderChapterContext {
                 handlebars: &handlebars,
                 destination: destination.to_path_buf(),
                 data: data.clone(),
-                is_index,
                 book_config: book_config.clone(),
                 html_config: html_config.clone(),
                 edition: ctx.config.rust.edition,
                 chapter_titles: &ctx.chapter_titles,
             };
-            self.render_item(item, ctx, &mut print_content)?;
+            self.render_chapter(ch, previous, next, ctx, &mut print_content)?;
-            // Only the first non-draft chapter item should be treated as the "index"
-            is_index &= !matches!(item, BookItem::Chapter(ch) if !ch.is_draft_chapter());
         }
 
         // Render 404 page
@@ -927,11 +948,10 @@ fn partition_source(s: &str) -> (String, String) {
     (before, after)
 }
 
-struct RenderItemContext<'a> {
+struct RenderChapterContext<'a> {
     handlebars: &'a Handlebars<'a>,
     destination: PathBuf,
     data: serde_json::Map<String, serde_json::Value>,
-    is_index: bool,
     book_config: BookConfig,
     html_config: HtmlConfig,
     edition: Option<RustEdition>,

crates/mdbook-html/src/html_handlebars/helpers/mod.rs

@@ -1,4 +1,3 @@
-pub(crate) mod navigation;
 pub(crate) mod resources;
 pub(crate) mod theme;
 pub(crate) mod toc;

crates/mdbook-html/src/html_handlebars/helpers/navigation.rs

@@ -1,302 +0,0 @@
-use std::collections::BTreeMap;
-use std::path::Path;
-
-use handlebars::{
-    Context, Handlebars, Helper, Output, RenderContext, RenderError, RenderErrorReason, Renderable,
-};
-
-use log::{debug, trace};
-use mdbook_core::utils;
-use serde_json::json;
-
-type StringMap = BTreeMap<String, String>;
-
-/// Target for `find_chapter`.
-enum Target {
-    Previous,
-    Next,
-}
-
-impl Target {
-    /// Returns target if found.
-    fn find(
-        &self,
-        base_path: &str,
-        current_path: &str,
-        current_item: &StringMap,
-        previous_item: &StringMap,
-    ) -> Result<Option<StringMap>, RenderError> {
-        match *self {
-            Target::Next => {
-                let previous_path = previous_item.get("path").ok_or_else(|| {
-                    RenderErrorReason::Other("No path found for chapter in JSON data".to_owned())
-                })?;
-
-                if previous_path == base_path {
-                    return Ok(Some(current_item.clone()));
-                }
-            }
-
-            Target::Previous => {
-                if current_path == base_path {
-                    return Ok(Some(previous_item.clone()));
-                }
-            }
-        }
-
-        Ok(None)
-    }
-}
-
-fn find_chapter(
-    ctx: &Context,
-    rc: &mut RenderContext<'_, '_>,
-    target: Target,
-) -> Result<Option<StringMap>, RenderError> {
-    debug!("Get data from context");
-
-    let chapters = rc.evaluate(ctx, "@root/chapters").and_then(|c| {
-        serde_json::value::from_value::<Vec<StringMap>>(c.as_json().clone()).map_err(|_| {
-            RenderErrorReason::Other("Could not decode the JSON data".to_owned()).into()
-        })
-    })?;
-
-    let base_path = rc
-        .evaluate(ctx, "@root/path")?
-        .as_json()
-        .as_str()
-        .ok_or_else(|| {
-            RenderErrorReason::Other("Type error for `path`, string expected".to_owned())
-        })?
-        .replace('\"', "");
-
-    if !rc.evaluate(ctx, "@root/is_index")?.is_missing() {
-        // Special case for index.md which may be a synthetic page.
-        // Target::find won't match because there is no page with the path
-        // "index.md" (unless there really is an index.md in SUMMARY.md).
-        match target {
-            Target::Previous => return Ok(None),
-            Target::Next => match chapters
-                .iter()
-                .filter(|chapter| {
-                    // Skip things like "spacer"
-                    chapter.contains_key("path")
-                })
-                .nth(1)
-            {
-                Some(chapter) => return Ok(Some(chapter.clone())),
-                None => return Ok(None),
-            },
-        }
-    }
-
-    let mut previous: Option<StringMap> = None;
-
-    debug!("Search for chapter");
-
-    for item in chapters {
-        match item.get("path") {
-            Some(path) if !path.is_empty() => {
-                if let Some(previous) = previous {
-                    if let Some(item) = target.find(&base_path, path, &item, &previous)? {
-                        return Ok(Some(item));
-                    }
-                }
-
-                previous = Some(item);
-            }
-            _ => continue,
-        }
-    }
-
-    Ok(None)
-}
-
-fn render(
-    _h: &Helper<'_>,
-    r: &Handlebars<'_>,
-    ctx: &Context,
-    rc: &mut RenderContext<'_, '_>,
-    out: &mut dyn Output,
-    chapter: &StringMap,
-) -> Result<(), RenderError> {
-    trace!("Creating BTreeMap to inject in context");
-
-    let mut context = BTreeMap::new();
-    let base_path = rc
-        .evaluate(ctx, "@root/path")?
-        .as_json()
-        .as_str()
-        .ok_or_else(|| {
-            RenderErrorReason::Other("Type error for `path`, string expected".to_owned())
-        })?
-        .replace('\"', "");
-
-    context.insert(
-        "path_to_root".to_owned(),
-        json!(utils::fs::path_to_root(base_path)),
-    );
-
-    chapter
-        .get("name")
-        .ok_or_else(|| {
-            RenderErrorReason::Other("No title found for chapter in JSON data".to_owned())
-        })
-        .map(|name| context.insert("title".to_owned(), json!(name)))?;
-
-    chapter
-        .get("path")
-        .ok_or_else(|| {
-            RenderErrorReason::Other("No path found for chapter in JSON data".to_owned())
-        })
-        .and_then(|p| {
-            Path::new(p)
-                .with_extension("html")
-                .to_str()
-                .ok_or_else(|| {
-                    RenderErrorReason::Other("Link could not be converted to str".to_owned())
-                })
-                .map(|p| context.insert("link".to_owned(), json!(p.replace('\\', "/"))))
-        })?;
-
-    trace!("Render template");
-
-    let t = _h
-        .template()
-        .ok_or_else(|| RenderErrorReason::Other("Error with the handlebars template".to_owned()))?;
-    let local_ctx = Context::wraps(&context)?;
-    let mut local_rc = rc.clone();
-    t.render(r, &local_ctx, &mut local_rc, out)
-}
-
-pub(crate) fn previous(
-    _h: &Helper<'_>,
-    r: &Handlebars<'_>,
-    ctx: &Context,
-    rc: &mut RenderContext<'_, '_>,
-    out: &mut dyn Output,
-) -> Result<(), RenderError> {
-    trace!("previous (handlebars helper)");
-
-    if let Some(previous) = find_chapter(ctx, rc, Target::Previous)? {
-        render(_h, r, ctx, rc, out, &previous)?;
-    }
-
-    Ok(())
-}
-
-pub(crate) fn next(
-    _h: &Helper<'_>,
-    r: &Handlebars<'_>,
-    ctx: &Context,
-    rc: &mut RenderContext<'_, '_>,
-    out: &mut dyn Output,
-) -> Result<(), RenderError> {
-    trace!("next (handlebars helper)");
-
-    if let Some(next) = find_chapter(ctx, rc, Target::Next)? {
-        render(_h, r, ctx, rc, out, &next)?;
-    }
-
-    Ok(())
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    static TEMPLATE: &str =
-        "{{#previous}}{{title}}: {{link}}{{/previous}}|{{#next}}{{title}}: {{link}}{{/next}}";
-
-    #[test]
-    fn test_next_previous() {
-        let data = json!({
-           "name": "two",
-           "path": "two.path",
-           "chapters": [
-              {
-                 "name": "one",
-                 "path": "one.path"
-              },
-              {
-                 "name": "two",
-                 "path": "two.path",
-              },
-              {
-                 "name": "three",
-                 "path": "three.path"
-              }
-           ]
-        });
-
-        let mut h = Handlebars::new();
-        h.register_helper("previous", Box::new(previous));
-        h.register_helper("next", Box::new(next));
-
-        assert_eq!(
-            h.render_template(TEMPLATE, &data).unwrap(),
-            "one: one.html|three: three.html"
-        );
-    }
-
-    #[test]
-    fn test_first() {
-        let data = json!({
-           "name": "one",
-           "path": "one.path",
-           "chapters": [
-              {
-                 "name": "one",
-                 "path": "one.path"
-              },
-              {
-                 "name": "two",
-                 "path": "two.path",
-              },
-              {
-                 "name": "three",
-                 "path": "three.path"
-              }
-           ]
-        });
-
-        let mut h = Handlebars::new();
-        h.register_helper("previous", Box::new(previous));
-        h.register_helper("next", Box::new(next));
-
-        assert_eq!(
-            h.render_template(TEMPLATE, &data).unwrap(),
-            "|two: two.html"
-        );
-    }
-    #[test]
-    fn test_last() {
-        let data = json!({
-           "name": "three",
-           "path": "three.path",
-           "chapters": [
-              {
-                 "name": "one",
-                 "path": "one.path"
-              },
-              {
-                 "name": "two",
-                 "path": "two.path",
-              },
-              {
-                 "name": "three",
-                 "path": "three.path"
-              }
-           ]
-        });
-
-        let mut h = Handlebars::new();
-        h.register_helper("previous", Box::new(previous));
-        h.register_helper("next", Box::new(next));
-
-        assert_eq!(
-            h.render_template(TEMPLATE, &data).unwrap(),
-            "two: two.html|"
-        );
-    }
-}

guide/src/format/theme/index-hbs.md

@@ -30,7 +30,7 @@ Here is a list of the properties that are exposed:
   to the root of the book from the current file. Since the original directory
   structure is maintained, it is useful to prepend relative links with this
   `path_to_root`.
-
+- ***previous*** and ***next*** These are objects used for linking to the previous and next chapter. They contain the properties `title` and `link` of the corresponding chapter.
 - ***chapters*** Is an array of dictionaries of the form
   ```json
   {"section": "1.2.1", "name": "name of this chapter", "path": "dir/markdown.md"}
@@ -43,7 +43,7 @@ Here is a list of the properties that are exposed:
 In addition to the properties you can access, there are some handlebars helpers
 at your disposal.
 
-### 1. toc
+### toc
 
 The toc helper is used like this
 
@@ -77,30 +77,7 @@ var chapters = {{chapters}};
 </script>
 ```
 
-### 2. previous / next
+### resource
-
-The previous and next helpers expose a `link` and `title` property to the
-previous and next chapters.
-
-They are used like this
-
-```handlebars
-{{#previous}}
-    <a href="{{link}}" class="nav-chapters previous">
-        <i class="fa fa-angle-left"></i> {{title}}
-    </a>
-{{/previous}}
-```
-
-The inner html will only be rendered if the previous / next chapter exists.
-Of course the inner html can be changed to your liking.
-
-------
-
-*If you would like other properties or helpers exposed, please [create a new
-issue](https://github.com/rust-lang/mdBook/issues)*
-
-### 3. resource
 
 The path to a static file.
 It implicitly includes `path_to_root`,