From c069bfdafadf38cb39fd3b5d81298460b8052d3a Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Wed, 22 Aug 2018 17:34:32 +0100 Subject: [PATCH 1/5] Rework summary handling. Push summary handling into Markdown parsing, identifying the presence of one by giving its length in the rendered markup. Hopefully a better fix for #376. --- components/content/src/page.rs | 19 +++++----------- components/content/src/section.rs | 4 ++-- components/rendering/src/lib.rs | 2 +- components/rendering/src/markdown.rs | 33 ++++++++++++++++++++-------- 4 files changed, 33 insertions(+), 25 deletions(-) diff --git a/components/content/src/page.rs b/components/content/src/page.rs index 3a0bf066..4162d6d3 100644 --- a/components/content/src/page.rs +++ b/components/content/src/page.rs @@ -185,19 +185,12 @@ impl Page { context.tera_context.add("page", self); - let res = render_content( - &self.raw_content.replacen("", "", 1), - &context, - ).chain_err(|| format!("Failed to render content of {}", self.file.path.display()))?; - self.content = res.0; - self.toc = res.1; - if self.raw_content.contains("") { - self.summary = Some({ - let summary = self.raw_content.splitn(2, "").collect::>()[0]; - render_content(summary, &context) - .chain_err(|| format!("Failed to render content of {}", self.file.path.display()))?.0 - }) - } + let res = render_content(&self.raw_content, &context) + .chain_err(|| format!("Failed to render content of {}", self.file.path.display()))?; + + self.summary = res.summary_len.map(|l| res.body[0..l].to_owned()); + self.content = res.body; + self.toc = res.toc; Ok(()) } diff --git a/components/content/src/section.rs b/components/content/src/section.rs index dcff1f5c..07ddde2b 100644 --- a/components/content/src/section.rs +++ b/components/content/src/section.rs @@ -137,8 +137,8 @@ impl Section { let res = render_content(&self.raw_content, &context) .chain_err(|| format!("Failed to render content of {}", self.file.path.display()))?; - self.content = res.0; - self.toc = res.1; + self.content = res.body; + self.toc = res.toc; Ok(()) } diff --git a/components/rendering/src/lib.rs b/components/rendering/src/lib.rs index 9e6d7257..acc08953 100644 --- a/components/rendering/src/lib.rs +++ b/components/rendering/src/lib.rs @@ -32,7 +32,7 @@ pub use table_of_contents::Header; pub use shortcode::render_shortcodes; pub use context::RenderContext; -pub fn render_content(content: &str, context: &RenderContext) -> Result<(String, Vec
)> { +pub fn render_content(content: &str, context: &RenderContext) -> Result { // Don't do anything if there is nothing like a shortcode in the content if content.contains("{{") || content.contains("{%") { let rendered = render_shortcodes(content, context)?; diff --git a/components/rendering/src/markdown.rs b/components/rendering/src/markdown.rs index 56bea619..a9c3b3e6 100644 --- a/components/rendering/src/markdown.rs +++ b/components/rendering/src/markdown.rs @@ -1,4 +1,4 @@ -use std::borrow::Cow::Owned; +use std::borrow::Cow::{Owned, Borrowed}; use pulldown_cmark as cmark; use self::cmark::{Parser, Event, Tag, Options, OPTION_ENABLE_TABLES, OPTION_ENABLE_FOOTNOTES}; @@ -14,6 +14,14 @@ use link_checker::check_url; use table_of_contents::{TempHeader, Header, make_table_of_contents}; use context::RenderContext; +const CONTINUE_READING: &str = "

\n"; + +pub struct Rendered { + pub body: String, + pub summary_len: Option, + pub toc: Vec
+} + // We might have cases where the slug is already present in our list of anchor // for example an article could have several titles named Example // We add a counter after the slug if the slug is already present, which @@ -36,8 +44,7 @@ fn is_colocated_asset_link(link: &str) -> bool { && !link.starts_with("mailto:") } - -pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result<(String, Vec
)> { +pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result { // the rendered html let mut html = String::with_capacity(content.len()); // Set while parsing @@ -57,6 +64,7 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result<(Strin let mut temp_header = TempHeader::default(); let mut opts = Options::empty(); + let mut has_summary = false; opts.insert(OPTION_ENABLE_TABLES); opts.insert(OPTION_ENABLE_FOOTNOTES); @@ -208,6 +216,10 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result<(Strin temp_header = TempHeader::default(); Event::Html(Owned(val)) } + Event::Html(ref markup) if markup.contains("") => { + has_summary = true; + Event::Html(Borrowed(CONTINUE_READING)) + } _ => event, } }); @@ -215,11 +227,14 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result<(Strin cmark::html::push_html(&mut html, parser); } - match error { - Some(e) => Err(e), - None => Ok(( - html.replace("

", "").replace("

", "

"), - make_table_of_contents(&headers) - )), + if let Some(e) = error { + return Err(e) + } else { + html = html.replace("

", "").replace("

", "

"); + Ok(Rendered { + summary_len: if has_summary { html.find(CONTINUE_READING) } else { None }, + body: html, + toc: make_table_of_contents(&headers) + }) } } From f2f3bed0804994fa90f9ad888ecd8ab753845b5b Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 24 Aug 2018 17:40:26 +0100 Subject: [PATCH 2/5] Markdown parsing: prefer Borrowed over Owned where possible As mentioned in #376 --- components/rendering/src/markdown.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/components/rendering/src/markdown.rs b/components/rendering/src/markdown.rs index a9c3b3e6..d7413b04 100644 --- a/components/rendering/src/markdown.rs +++ b/components/rendering/src/markdown.rs @@ -76,7 +76,7 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result Result or other things already there temp_header.title += &text; header_created = true; - return Event::Html(Owned(String::new())); + return Event::Html(Borrowed("")); } // if we are in the middle of a code block @@ -101,7 +101,7 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result { if !context.config.highlight_code { - return Event::Html(Owned("
".to_string()));
+                        return Event::Html(Borrowed("
"));
                     }
 
                     let theme = &THEME_SET.themes[&context.config.highlight_theme];
@@ -109,7 +109,7 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result highlighter = Some(h),
                         Err(err) => {
                             error = Some(format!("Could not load syntax: {}", err).into());
-                            return Event::Html(Owned(String::new()));
+                            return Event::Html(Borrowed(""));
                         }
                     }
                     let snippet = start_coloured_html_snippet(theme);
@@ -117,11 +117,11 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result {
                     if !context.config.highlight_code {
-                        return Event::Html(Owned("
\n".to_string()));
+                        return Event::Html(Borrowed("
\n")); } // reset highlight and close the code block highlighter = None; - Event::Html(Owned("".to_string())) + Event::Html(Borrowed("")) } Event::Start(Tag::Image(src, title)) => { if is_colocated_asset_link(&src) { @@ -147,7 +147,7 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result url, Err(_) => { error = Some(format!("Relative link {} not found.", link).into()); - return Event::Html(Owned(String::new())); + return Event::Html(Borrowed("")); } } } else if is_colocated_asset_link(&link) { @@ -175,7 +175,7 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result", fixed_link, title) }; temp_header.push(&html); - return Event::Html(Owned(String::new())); + return Event::Html(Borrowed("")); } Event::Start(Tag::Link(Owned(fixed_link), title)) @@ -183,28 +183,28 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result { if in_header { temp_header.push(""); - return Event::Html(Owned(String::new())); + return Event::Html(Borrowed("")); } event } Event::Start(Tag::Code) => { if in_header { temp_header.push(""); - return Event::Html(Owned(String::new())); + return Event::Html(Borrowed("")); } event } Event::End(Tag::Code) => { if in_header { temp_header.push(""); - return Event::Html(Owned(String::new())); + return Event::Html(Borrowed("")); } event } Event::Start(Tag::Header(num)) => { in_header = true; temp_header = TempHeader::new(num); - Event::Html(Owned(String::new())) + Event::Html(Borrowed("")) } Event::End(Tag::Header(_)) => { // End of a header, reset all the things and return the stringified From 5f1f9efe7a0c0e49e6a47d64a2d8e2e900310364 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 24 Aug 2018 22:37:39 +0100 Subject: [PATCH 3/5] Derive debug for markdown::Rendered --- components/rendering/src/markdown.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/rendering/src/markdown.rs b/components/rendering/src/markdown.rs index d7413b04..e7468db6 100644 --- a/components/rendering/src/markdown.rs +++ b/components/rendering/src/markdown.rs @@ -16,6 +16,7 @@ use context::RenderContext; const CONTINUE_READING: &str = "

\n"; +#[derive(Debug)] pub struct Rendered { pub body: String, pub summary_len: Option, From c53c4037900ec484519ee8169477f3d3ac2bceb5 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Fri, 24 Aug 2018 22:37:55 +0100 Subject: [PATCH 4/5] Update rendering tests --- components/rendering/tests/markdown.rs | 76 +++++++++++++------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/components/rendering/tests/markdown.rs b/components/rendering/tests/markdown.rs index 93e6f646..a3969513 100644 --- a/components/rendering/tests/markdown.rs +++ b/components/rendering/tests/markdown.rs @@ -22,7 +22,7 @@ fn can_do_render_content_simple() { let config = Config::default(); let context = RenderContext::new(&tera_ctx, &config, "", &permalinks_ctx, Path::new("something"), InsertAnchor::None); let res = render_content("hello", &context).unwrap(); - assert_eq!(res.0, "

hello

\n"); + assert_eq!(res.body, "

hello

\n"); } #[test] @@ -34,7 +34,7 @@ fn doesnt_highlight_code_block_with_highlighting_off() { let context = RenderContext::new(&tera_ctx, &config, "", &permalinks_ctx, Path::new("something"), InsertAnchor::None); let res = render_content("```\n$ gutenberg server\n```", &context).unwrap(); assert_eq!( - res.0, + res.body, "
$ gutenberg server\n
\n" ); } @@ -47,7 +47,7 @@ fn can_highlight_code_block_no_lang() { let context = RenderContext::new(&tera_ctx, &config, "", &permalinks_ctx, Path::new("something"), InsertAnchor::None); let res = render_content("```\n$ gutenberg server\n$ ping\n```", &context).unwrap(); assert_eq!( - res.0, + res.body, "
\n$ gutenberg server\n$ ping\n
" ); } @@ -60,7 +60,7 @@ fn can_highlight_code_block_with_lang() { let context = RenderContext::new(&tera_ctx, &config, "", &permalinks_ctx, Path::new("something"), InsertAnchor::None); let res = render_content("```python\nlist.append(1)\n```", &context).unwrap(); assert_eq!( - res.0, + res.body, "
\nlist.append(1)\n
" ); } @@ -74,7 +74,7 @@ fn can_higlight_code_block_with_unknown_lang() { let res = render_content("```yolo\nlist.append(1)\n```", &context).unwrap(); // defaults to plain text assert_eq!( - res.0, + res.body, "
\nlist.append(1)\n
" ); } @@ -89,8 +89,8 @@ Hello {{ youtube(id="ub36ffWAqgQ") }} "#, &context).unwrap(); - assert!(res.0.contains("

Hello

\n
")); - assert!(res.0.contains(r#"