From f0435ac0ee247a3ee30594916b957233ae6ad825 Mon Sep 17 00:00:00 2001 From: Jacob Wahlgren Date: Fri, 6 Oct 2017 23:37:20 +0200 Subject: [PATCH 1/4] Refactor navigation helpers --- .../html_handlebars/helpers/navigation.rs | 230 ++++++++---------- 1 file changed, 99 insertions(+), 131 deletions(-) diff --git a/src/renderer/html_handlebars/helpers/navigation.rs b/src/renderer/html_handlebars/helpers/navigation.rs index 235c3949b8..b2df7afdbd 100644 --- a/src/renderer/html_handlebars/helpers/navigation.rs +++ b/src/renderer/html_handlebars/helpers/navigation.rs @@ -5,165 +5,133 @@ use serde_json; use handlebars::{Context, Handlebars, Helper, RenderContext, RenderError, Renderable}; -// Handlebars helper for navigation +type StringMap = BTreeMap; -pub fn previous(_h: &Helper, r: &Handlebars, rc: &mut RenderContext) -> Result<(), RenderError> { - debug!("[fn]: previous (handlebars helper)"); +/// Target for `find_chapter`. +enum Target { + Previous, + Next, +} + +impl Target { + /// Returns target if found. + fn find(&self, + base_path: &String, + current_path: &String, + current_item: &StringMap, + previous_item: StringMap, + ) -> Result, RenderError> { + match self { + &Target::Next => { + let previous_path = previous_item.get("path").ok_or_else(|| { + RenderError::new("No path found for chapter in JSON data") + })?; + + if previous_path == base_path { + return Ok(Some(current_item.clone())); + } + }, + + &Target::Previous => { + if current_path == base_path { + return Ok(Some(previous_item)); + } + } + } + + Ok(None) + } +} +fn find_chapter( + rc: &mut RenderContext, + target: Target +) -> Result, RenderError> { debug!("[*]: Get data from context"); + let chapters = rc.evaluate_absolute("chapters").and_then(|c| { - serde_json::value::from_value::>>(c.clone()) + serde_json::value::from_value::>(c.clone()) .map_err(|_| RenderError::new("Could not decode the JSON data")) })?; - let current = rc.evaluate_absolute("path")? - .as_str() - .ok_or_else(|| RenderError::new("Type error for `path`, string expected"))? - .replace("\"", ""); + let base_path = rc.evaluate_absolute("path")? + .as_str() + .ok_or_else(|| RenderError::new("Type error for `path`, string expected"))? + .replace("\"", ""); + + let mut previous: Option = None; - let mut previous: Option> = None; + debug!("[*]: Search for chapter"); - debug!("[*]: Search for current Chapter"); - // Search for current chapter and return previous entry for item in chapters { match item.get("path") { Some(path) if !path.is_empty() => { - if path == ¤t { - debug!("[*]: Found current chapter"); - if let Some(previous) = previous { - debug!("[*]: Creating BTreeMap to inject in context"); - // Create new BTreeMap to extend the context: 'title' and 'link' - let mut previous_chapter = BTreeMap::new(); - - // Chapter title - previous.get("name") - .ok_or_else(|| { - RenderError::new("No title found for chapter in \ - JSON data") - }) - .and_then(|n| { - previous_chapter.insert("title".to_owned(), json!(n)); - Ok(()) - })?; - - - // Chapter link - previous.get("path") - .ok_or_else(|| { - RenderError::new("No path found for chapter in \ - JSON data") - }) - .and_then(|p| { - Path::new(p).with_extension("html") - .to_str() - .ok_or_else(|| { - RenderError::new("Link could not be \ - converted to str") - }) - .and_then(|p| { - previous_chapter - .insert("link".to_owned(), json!(p.replace("\\", "/"))); - Ok(()) - }) - })?; - - - debug!("[*]: Render template"); - // Render template - _h.template() - .ok_or_else(|| RenderError::new("Error with the handlebars template")) - .and_then(|t| { - let mut local_rc = rc.with_context(Context::wraps(&previous_chapter)?); - t.render(r, &mut local_rc) - })?; + if let Some(previous) = previous { + if let Some(item) = target.find(&base_path, &path, &item, previous)? { + return Ok(Some(item)); } - break; - } else { - previous = Some(item.clone()); } + + previous = Some(item.clone()); } _ => continue, } } + Ok(None) +} + +fn render( + _h: &Helper, + r: &Handlebars, + rc: &mut RenderContext, + chapter: &StringMap, +) -> Result<(), RenderError> { + debug!("[*]: Creating BTreeMap to inject in context"); + + let mut context = BTreeMap::new(); + + chapter.get("name") + .ok_or_else(|| RenderError::new("No title found for chapter in JSON data")) + .map(|name| context.insert("title".to_owned(), json!(name)))?; + + chapter.get("path") + .ok_or_else(|| RenderError::new("No path found for chapter in JSON data")) + .and_then(|p| { + Path::new(p).with_extension("html") + .to_str() + .ok_or_else(|| RenderError::new("Link could not be converted to str")) + .map(|p| context.insert("link".to_owned(), json!(p.replace("\\", "/")))) + })?; + + debug!("[*]: Render template"); + + _h.template() + .ok_or_else(|| RenderError::new("Error with the handlebars template")) + .and_then(|t| { + let mut local_rc = rc.with_context(Context::wraps(&context)?); + t.render(r, &mut local_rc) + })?; + Ok(()) } +pub fn previous(_h: &Helper, r: &Handlebars, rc: &mut RenderContext) -> Result<(), RenderError> { + debug!("[fn]: previous (handlebars helper)"); + if let Some(previous) = find_chapter(rc, Target::Previous)? { + render(_h, r, rc, &previous)?; + } + Ok(()) +} pub fn next(_h: &Helper, r: &Handlebars, rc: &mut RenderContext) -> Result<(), RenderError> { debug!("[fn]: next (handlebars helper)"); - debug!("[*]: Get data from context"); - let chapters = rc.evaluate_absolute("chapters").and_then(|c| { - serde_json::value::from_value::>>(c.clone()) - .map_err(|_| RenderError::new("Could not decode the JSON data")) - })?; - let current = rc.evaluate_absolute("path")? - .as_str() - .ok_or_else(|| RenderError::new("Type error for `path`, string expected"))? - .replace("\"", ""); - - let mut previous: Option> = None; - - debug!("[*]: Search for current Chapter"); - // Search for current chapter and return previous entry - for item in chapters { - match item.get("path") { - Some(path) if !path.is_empty() => { - if let Some(previous) = previous { - let previous_path = previous.get("path").ok_or_else(|| { - RenderError::new("No path found for chapter in JSON data") - })?; - - if previous_path == ¤t { - debug!("[*]: Found current chapter"); - debug!("[*]: Creating BTreeMap to inject in context"); - // Create new BTreeMap to extend the context: 'title' and 'link' - let mut next_chapter = BTreeMap::new(); - - item.get("name") - .ok_or_else(|| { - RenderError::new("No title found for chapter in JSON \ - data") - }) - .and_then(|n| { - next_chapter.insert("title".to_owned(), json!(n)); - Ok(()) - })?; - - Path::new(path).with_extension("html") - .to_str() - .ok_or_else(|| { - RenderError::new("Link could not converted \ - to str") - }) - .and_then(|l| { - debug!("[*]: Inserting link: {:?}", l); - // Hack for windows who tends to use `\` as separator instead of `/` - next_chapter.insert("link".to_owned(), json!(l.replace("\\", "/"))); - Ok(()) - })?; - - debug!("[*]: Render template"); - - // Render template - _h.template() - .ok_or_else(|| RenderError::new("Error with the handlebars template")) - .and_then(|t| { - let mut local_rc = rc.with_context(Context::wraps(&next_chapter)?); - t.render(r, &mut local_rc) - })?; - break; - } - } - - previous = Some(item.clone()); - } - - _ => continue, - } + if let Some(next) = find_chapter(rc, Target::Next)? { + render(_h, r, rc, &next)?; } + Ok(()) } From 31982bc8764c0456230b9ca8e1b98ce40fed2030 Mon Sep 17 00:00:00 2001 From: Jacob Wahlgren Date: Mon, 13 Nov 2017 15:48:13 +0100 Subject: [PATCH 2/4] Target::find: take previous_item by reference This makes more sense for find as an interface, though it causes a second clone in some cases. Maybe rustc is smart here? --- src/renderer/html_handlebars/helpers/navigation.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderer/html_handlebars/helpers/navigation.rs b/src/renderer/html_handlebars/helpers/navigation.rs index b2df7afdbd..10e383ca90 100644 --- a/src/renderer/html_handlebars/helpers/navigation.rs +++ b/src/renderer/html_handlebars/helpers/navigation.rs @@ -19,7 +19,7 @@ impl Target { base_path: &String, current_path: &String, current_item: &StringMap, - previous_item: StringMap, + previous_item: &StringMap, ) -> Result, RenderError> { match self { &Target::Next => { @@ -34,7 +34,7 @@ impl Target { &Target::Previous => { if current_path == base_path { - return Ok(Some(previous_item)); + return Ok(Some(previous_item.clone())); } } } @@ -67,7 +67,7 @@ fn find_chapter( 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)? { + if let Some(item) = target.find(&base_path, &path, &item, &previous)? { return Ok(Some(item)); } } From 7ffb4abee7384b43fac189d8975ebdeeb63c3f33 Mon Sep 17 00:00:00 2001 From: Jacob Wahlgren Date: Mon, 13 Nov 2017 21:11:51 +0100 Subject: [PATCH 3/4] Test next and previous navigation helpers --- .../html_handlebars/helpers/navigation.rs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/renderer/html_handlebars/helpers/navigation.rs b/src/renderer/html_handlebars/helpers/navigation.rs index 10e383ca90..57c9828b23 100644 --- a/src/renderer/html_handlebars/helpers/navigation.rs +++ b/src/renderer/html_handlebars/helpers/navigation.rs @@ -135,3 +135,42 @@ pub fn next(_h: &Helper, r: &Handlebars, rc: &mut RenderContext) -> Result<(), R Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_next_previous() { + let template = " + {{#previous}}{{title}}: {{link}}{{/previous}} + {{#next}}{{title}}: {{link}}{{/next}}"; + + 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.template_render(template, &data).unwrap(), " + one: one.html + three: three.html"); + } +} From 6ca14cc805b02404df9b5e13a0a91a0d464f5dc7 Mon Sep 17 00:00:00 2001 From: Jacob Wahlgren Date: Mon, 13 Nov 2017 22:39:33 +0100 Subject: [PATCH 4/4] Add more next/previous tests --- .../html_handlebars/helpers/navigation.rs | 72 +++++++++++++++++-- 1 file changed, 65 insertions(+), 7 deletions(-) diff --git a/src/renderer/html_handlebars/helpers/navigation.rs b/src/renderer/html_handlebars/helpers/navigation.rs index 57c9828b23..77316a1178 100644 --- a/src/renderer/html_handlebars/helpers/navigation.rs +++ b/src/renderer/html_handlebars/helpers/navigation.rs @@ -140,12 +140,11 @@ pub fn next(_h: &Helper, r: &Handlebars, rc: &mut RenderContext) -> Result<(), R mod tests { use super::*; + static TEMPLATE: &'static str = + "{{#previous}}{{title}}: {{link}}{{/previous}}|{{#next}}{{title}}: {{link}}{{/next}}"; + #[test] fn test_next_previous() { - let template = " - {{#previous}}{{title}}: {{link}}{{/previous}} - {{#next}}{{title}}: {{link}}{{/next}}"; - let data = json!({ "name": "two", "path": "two.path", @@ -169,8 +168,67 @@ mod tests { h.register_helper("previous", Box::new(previous)); h.register_helper("next", Box::new(next)); - assert_eq!(h.template_render(template, &data).unwrap(), " - one: one.html - three: three.html"); + assert_eq!( + h.template_render(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.template_render(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.template_render(TEMPLATE, &data).unwrap(), + "two: two.html|"); } }