From d13258f668c6089dc4197aa8da22bd79c307014b Mon Sep 17 00:00:00 2001 From: Chris Emerson Date: Sat, 9 Dec 2023 12:33:27 +0000 Subject: [PATCH 1/3] Update some unwraps to expect() with a message. --- src/render/text_renderer.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/render/text_renderer.rs b/src/render/text_renderer.rs index 8beb1db..c2d9298 100644 --- a/src/render/text_renderer.rs +++ b/src/render/text_renderer.rs @@ -61,14 +61,16 @@ impl TextRenderer { /// Pop off the top builder and return it. /// Panics if empty pub fn pop(&mut self) -> SubRenderer { - self.subrender.pop().unwrap() + self.subrender.pop().expect("Attempt to pop a subrender from empty stack") } /// Pop off the only builder and return it. /// panics if there aren't exactly 1 available. pub fn into_inner(mut self) -> (SubRenderer, Vec) { assert_eq!(self.subrender.len(), 1); - (self.subrender.pop().unwrap(), self.links) + (self.subrender.pop().expect( + "Attempt to pop a subrenderer from an empty stack"), + self.links) } } @@ -1372,7 +1374,7 @@ impl Renderer for SubRenderer { self.text_filter_stack.push(filter_text_strikeout); } fn end_strikeout(&mut self) { - self.text_filter_stack.pop().unwrap(); + self.text_filter_stack.pop().expect("end_strikeout() called without a corresponding start_strokeout()"); let s = self.decorator.decorate_strikeout_end(); self.add_inline_text(&s); self.ann_stack.pop(); From 1622607e45107e7c0c22e573e315b24282263982 Mon Sep 17 00:00:00 2001 From: Chris Emerson Date: Sat, 9 Dec 2023 22:10:16 +0000 Subject: [PATCH 2/3] Changes to error handling. Introduce Result<> in some places rather than panics. In particular, some cases which end up with a zero width now return an error rather than panicking. The top-level from_read etc. still unwrap the error. --- Cargo.toml | 1 + src/lib.rs | 149 ++++++++++++++++++++---------------- src/render/mod.rs | 2 +- src/render/text_renderer.rs | 7 +- src/tests.rs | 104 ++++++++++++++++--------- 5 files changed, 161 insertions(+), 102 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 756e60b..933183f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ tendril = "0.4" xml5ever = "0.17" unicode-width = "0.1.5" backtrace = { version = "0.3", optional=true } +thiserror = "1.0.50" [features] html_trace = [] diff --git a/src/lib.rs b/src/lib.rs index ca3f938..9ba0f05 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -82,6 +82,20 @@ use std::io; use std::io::Write; use std::iter::{once, repeat}; +/// Errors from reading or rendering HTML +#[derive(thiserror::Error, Debug, Eq, PartialEq)] +#[non_exhaustive] +pub enum Error { + /// The output width was too narrow to render to. + #[error("Output width not wide enough.")] + TooNarrow, + /// An general error was encountered. + #[error("Unknown failure")] + Fail, +} + +type Result = std::result::Result; + /// A dummy writer which does nothing struct Discard {} impl Write for Discard { @@ -498,12 +512,12 @@ impl RenderNode { } } -fn precalc_size_estimate<'a>(node: &'a RenderNode) -> TreeMapResult<(), &'a RenderNode, ()> { +fn precalc_size_estimate<'a>(node: &'a RenderNode) -> Result> { use RenderNodeInfo::*; if node.size_estimate.get().is_some() { - return TreeMapResult::Nothing; + return Ok(TreeMapResult::Nothing); } - match node.info { + Ok(match node.info { Text(_) | Img(_, _) | Break | FragStart(_) => { let _ = node.get_size_estimate(); TreeMapResult::Nothing @@ -552,30 +566,29 @@ fn precalc_size_estimate<'a>(node: &'a RenderNode) -> TreeMapResult<(), &'a Rend } } TableRow(..) | TableBody(_) | TableCell(_) => unimplemented!(), - } + }) } /// Make a Vec of RenderNodes from the children of a node. -fn children_to_render_nodes(handle: Handle, err_out: &mut T) -> Vec { +fn children_to_render_nodes(handle: Handle, err_out: &mut T) -> Result> { /* process children, but don't add anything */ - let children = handle + handle .children .borrow() .iter() - .flat_map(|ch| dom_to_render_tree(ch.clone(), err_out)) - .collect(); - children + .flat_map(|ch| dom_to_render_tree(ch.clone(), err_out).transpose()) + .collect() } /// Make a Vec of RenderNodes from the
  • children of a node. -fn list_children_to_render_nodes(handle: Handle, err_out: &mut T) -> Vec { +fn list_children_to_render_nodes(handle: Handle, err_out: &mut T) -> Result> { let mut children = Vec::new(); for child in handle.children.borrow().iter() { match child.data { Element { ref name, .. } => match name.expanded() { expanded_name!(html "li") => { - let li_children = children_to_render_nodes(child.clone(), err_out); + let li_children = children_to_render_nodes(child.clone(), err_out)?; children.push(RenderNode::new(RenderNodeInfo::Block(li_children))); } _ => {} @@ -586,25 +599,25 @@ fn list_children_to_render_nodes(handle: Handle, err_out: &mut T) -> V } } } - children + Ok(children) } /// Make a Vec of DtElements from the
    and
    children of a node. fn desc_list_children_to_render_nodes( handle: Handle, err_out: &mut T, -) -> Vec { +) -> Result> { let mut children = Vec::new(); for child in handle.children.borrow().iter() { match child.data { Element { ref name, .. } => match name.expanded() { expanded_name!(html "dt") => { - let dt_children = children_to_render_nodes(child.clone(), err_out); + let dt_children = children_to_render_nodes(child.clone(), err_out)?; children.push(RenderNode::new(RenderNodeInfo::Dt(dt_children))); } expanded_name!(html "dd") => { - let dd_children = children_to_render_nodes(child.clone(), err_out); + let dd_children = children_to_render_nodes(child.clone(), err_out)?; children.push(RenderNode::new(RenderNodeInfo::Dd(dd_children))); } _ => {} @@ -615,7 +628,7 @@ fn desc_list_children_to_render_nodes( } } } - children + Ok(children) } /// Convert a table into a RenderNode @@ -748,7 +761,7 @@ fn td_to_render_tree<'a, 'b, T: Write>( type ResultReducer<'a, C, R> = dyn Fn(&mut C, Vec) -> Option + 'a; /// A closure to call before processing a child node. -type ChildPreFn = dyn Fn(&mut C, &N); +type ChildPreFn = dyn Fn(&mut C, &N) -> Result<()>; /// A closure to call after processing a child node, /// before adding the result to the processed results @@ -771,9 +784,9 @@ enum TreeMapResult<'a, C, N, R> { Nothing, } -fn tree_map_reduce<'a, C, N, R, M>(context: &mut C, top: N, mut process_node: M) -> Option +fn tree_map_reduce<'a, C, N, R, M>(context: &mut C, top: N, mut process_node: M) -> Result> where - M: for<'c> FnMut(&'c mut C, N) -> TreeMapResult<'a, C, N, R>, + M: for<'c> FnMut(&'c mut C, N) -> Result>, { /// A node partially decoded, waiting for its children to /// be processed. @@ -807,8 +820,9 @@ where .unwrap() .prefn .as_ref() - .map(|ref f| f(context, &h)); - match process_node(context, h) { + .map(|ref f| f(context, &h)) + .transpose()?; + match process_node(context, h)? { TreeMapResult::Finished(result) => { pending_stack .last_mut() @@ -844,12 +858,12 @@ where parent.children.push(node); } else { // Finished the whole stack! - break Some(node); + break Ok(Some(node)); } } else { /* Finished the stack, and have nothing */ if pending_stack.is_empty() { - break None; + break Ok(None); } } } @@ -857,7 +871,7 @@ where } /// Convert a DOM tree or subtree into a render tree. -pub fn dom_to_render_tree(handle: Handle, err_out: &mut T) -> Option { +pub fn dom_to_render_tree(handle: Handle, err_out: &mut T) -> Result> { html_trace!("### dom_to_render_tree: HTML: {:?}", handle); let result = tree_map_reduce(&mut (), handle, |_, handle| { process_dom_node(handle, err_out) @@ -944,11 +958,11 @@ fn prepend_marker(prefix: RenderNode, mut orig: RenderNode) -> RenderNode { fn process_dom_node<'a, 'b, T: Write>( handle: Handle, err_out: &'b mut T, -) -> TreeMapResult<'a, (), Handle, RenderNode> { +) -> Result> { use RenderNodeInfo::*; use TreeMapResult::*; - match handle.clone().data { + Ok(match handle.clone().data { Document => pending(handle, |&mut (), cs| Some(RenderNode::new(Container(cs)))), Comment { .. } => Nothing, Element { @@ -1068,7 +1082,7 @@ fn process_dom_node<'a, 'b, T: Write>( pending(handle, |_, cs| Some(RenderNode::new(BlockQuote(cs)))) } expanded_name!(html "ul") => Finished(RenderNode::new(Ul( - list_children_to_render_nodes(handle.clone(), err_out), + list_children_to_render_nodes(handle.clone(), err_out)?, ))), expanded_name!(html "ol") => { let borrowed = attrs.borrow(); @@ -1082,11 +1096,11 @@ fn process_dom_node<'a, 'b, T: Write>( Finished(RenderNode::new(Ol( start, - list_children_to_render_nodes(handle.clone(), err_out), + list_children_to_render_nodes(handle.clone(), err_out)?, ))) } expanded_name!(html "dl") => Finished(RenderNode::new(Dl( - desc_list_children_to_render_nodes(handle.clone(), err_out), + desc_list_children_to_render_nodes(handle.clone(), err_out)?, ))), _ => { html_trace!("Unhandled element: {:?}\n", name.local); @@ -1143,21 +1157,21 @@ fn process_dom_node<'a, 'b, T: Write>( write!(err_out, "Unhandled node type.\n").unwrap(); Nothing } - } + }) } fn render_tree_to_string( renderer: SubRenderer, tree: RenderNode, err_out: &mut T, -) -> SubRenderer { +) -> Result> { /* Phase 1: get size estimates. */ - tree_map_reduce(&mut (), &tree, |_, node| precalc_size_estimate(&node)); + tree_map_reduce(&mut (), &tree, |_, node| precalc_size_estimate(&node))?; /* Phase 2: actually render. */ let mut renderer = TextRenderer::new(renderer); tree_map_reduce(&mut renderer, tree, |renderer, node| { do_render_node(renderer, node, err_out) - }); + })?; let (mut renderer, links) = renderer.into_inner(); let lines = renderer.finalise(links); // And add the links @@ -1165,7 +1179,7 @@ fn render_tree_to_string( renderer.start_block(); renderer.fmt_links(lines); } - renderer + Ok(renderer) } fn pending2< @@ -1189,11 +1203,11 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( renderer: &mut TextRenderer, tree: RenderNode, err_out: &'b mut T, -) -> TreeMapResult<'static, TextRenderer, RenderNode, Option>> { +) -> Result, RenderNode, Option>>> { html_trace!("do_render_node({:?}", tree); use RenderNodeInfo::*; use TreeMapResult::*; - match tree.info { + Ok(match tree.info { Text(ref tstr) => { renderer.add_inline_text(tstr); Finished(None) @@ -1247,8 +1261,7 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( } Header(level, children) => { let prefix = renderer.header_prefix(level); - let min_width = max(renderer.width(), 1 + prefix.len()); - let sub_builder = renderer.new_sub_renderer(min_width - prefix.len()); + let sub_builder = renderer.new_sub_renderer(renderer.width().checked_sub(prefix.len()).ok_or(Error::TooNarrow)?)?; renderer.push(sub_builder); pending2(children, move |renderer: &mut TextRenderer, _| { let sub_builder = renderer.pop(); @@ -1277,7 +1290,7 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( } BlockQuote(children) => { let prefix = renderer.quote_prefix(); - let sub_builder = renderer.new_sub_renderer(renderer.width() - prefix.len()); + let sub_builder = renderer.new_sub_renderer(renderer.width().checked_sub(prefix.len()).ok_or(Error::TooNarrow)?)?; renderer.push(sub_builder); pending2(children, move |renderer: &mut TextRenderer, _| { let sub_builder = renderer.pop(); @@ -1298,8 +1311,9 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( children: items, cons: Box::new(|_, _| Some(None)), prefn: Some(Box::new(move |renderer: &mut TextRenderer, _| { - let sub_builder = renderer.new_sub_renderer(renderer.width() - prefix_len); + let sub_builder = renderer.new_sub_renderer(renderer.width().checked_sub(prefix_len).ok_or(Error::TooNarrow)?)?; renderer.push(sub_builder); + Ok(()) })), postfn: Some(Box::new(move |renderer: &mut TextRenderer, _| { let sub_builder = renderer.pop(); @@ -1332,8 +1346,9 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( children: items, cons: Box::new(|_, _| Some(None)), prefn: Some(Box::new(move |renderer: &mut TextRenderer, _| { - let sub_builder = renderer.new_sub_renderer(renderer.width() - prefix_width); + let sub_builder = renderer.new_sub_renderer(renderer.width().checked_sub(prefix_width).ok_or(Error::TooNarrow)?)?; renderer.push(sub_builder); + Ok(()) })), postfn: Some(Box::new(move |renderer: &mut TextRenderer, _| { let sub_builder = renderer.pop(); @@ -1367,7 +1382,7 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( }) } Dd(children) => { - let sub_builder = renderer.new_sub_renderer(renderer.width() - 2); + let sub_builder = renderer.new_sub_renderer(renderer.width().checked_sub(2).ok_or(Error::TooNarrow)?)?; renderer.push(sub_builder); pending2(children, |renderer: &mut TextRenderer, _| { let sub_builder = renderer.pop(); @@ -1388,7 +1403,7 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( renderer.record_frag_start(&fragname); Finished(None) } - } + }) } fn render_table_tree( @@ -1493,7 +1508,7 @@ fn render_table_tree( TreeMapResult::PendingChildren { children: table.into_rows(col_widths, vert_row), cons: Box::new(|_, _| Some(None)), - prefn: Some(Box::new(|_, _| {})), + prefn: Some(Box::new(|_, _| {Ok(())})), postfn: Some(Box::new(|_, _| {})), } } @@ -1514,8 +1529,9 @@ fn render_table_row( }), prefn: Some(Box::new(|renderer: &mut TextRenderer, node| { if let RenderNodeInfo::TableCell(ref cell) = node.info { - let sub_builder = renderer.new_sub_renderer(cell.col_width.unwrap()); + let sub_builder = renderer.new_sub_renderer(cell.col_width.unwrap())?; renderer.push(sub_builder); + Ok(()) } else { panic!() } @@ -1538,10 +1554,11 @@ fn render_table_row_vert( }), prefn: Some(Box::new(|renderer: &mut TextRenderer, node| { if let RenderNodeInfo::TableCell(ref cell) = node.info { - let sub_builder = renderer.new_sub_renderer(cell.col_width.unwrap()); + let sub_builder = renderer.new_sub_renderer(cell.col_width.unwrap())?; renderer.push(sub_builder); + Ok(()) } else { - panic!() + Err(Error::Fail) } })), postfn: Some(Box::new(|_renderer: &mut TextRenderer, _| {})), @@ -1563,9 +1580,9 @@ pub mod config { //! Configure the HTML to text translation using the `Config` type, which can be //! constructed using one of the functions in this module. - use crate::render::text_renderer::{ + use crate::{render::text_renderer::{ PlainDecorator, RichDecorator, TaggedLine, TextDecorator - }; + }, Result}; use super::parse; /// Configure the HTML processing. @@ -1576,18 +1593,18 @@ pub mod config { impl Config { /// Reads HTML from `input`, and returns a `String` with text wrapped to /// `width` columns. - pub fn string_from_read(self, input: R, width: usize) -> String { - parse(input).render(width, self.decorator).into_string() + pub fn string_from_read(self, input: R, width: usize) -> Result { + Ok(parse(input)?.render(width, self.decorator)?.into_string()) } /// Reads HTML from `input`, and returns text wrapped to `width` columns. /// The text is returned as a `Vec>`; the annotations are vectors /// of the provided text decorator's `Annotation`. The "outer" annotation comes first in /// the `Vec`. - pub fn lines_from_read(self, input: R, width: usize) -> Vec>> { - parse(input) - .render(width, self.decorator) - .into_lines() + pub fn lines_from_read(self, input: R, width: usize) -> Result>>> { + Ok(parse(input)? + .render(width, self.decorator)? + .into_lines()) } } @@ -1622,20 +1639,20 @@ pub struct RenderTree(RenderNode); impl RenderTree { /// Render this document using the given `decorator` and wrap it to `width` columns. - pub fn render(self, width: usize, decorator: D) -> RenderedText { + pub fn render(self, width: usize, decorator: D) -> Result> { if width == 0 { - panic!("Error: width can not be zero"); + return Err(Error::TooNarrow); } let builder = SubRenderer::new(width, decorator); - let builder = render_tree_to_string(builder, self.0, &mut Discard {}); - RenderedText(builder) + let builder = render_tree_to_string(builder, self.0, &mut Discard {})?; + Ok(RenderedText(builder)) } /// Render this document as plain text using the [`PlainDecorator`][] and wrap it to `width` /// columns. /// /// [`PlainDecorator`]: render/text_renderer/struct.PlainDecorator.html - pub fn render_plain(self, width: usize) -> RenderedText { + pub fn render_plain(self, width: usize) -> Result> { self.render(width, PlainDecorator::new()) } @@ -1643,7 +1660,7 @@ impl RenderTree { /// columns. /// /// [`RichDecorator`]: render/text_renderer/struct.RichDecorator.html - pub fn render_rich(self, width: usize) -> RenderedText { + pub fn render_rich(self, width: usize) -> Result> { self.render(width, RichDecorator::new()) } } @@ -1669,7 +1686,7 @@ impl RenderedText { } /// Reads and parses HTML from `input` and prepares a render tree. -pub fn parse(mut input: impl io::Read) -> RenderTree { +pub fn parse(mut input: impl io::Read) -> Result { let opts = ParseOpts { tree_builder: TreeBuilderOpts { drop_doctype: true, @@ -1681,8 +1698,9 @@ pub fn parse(mut input: impl io::Read) -> RenderTree { .from_utf8() .read_from(&mut input) .unwrap(); - let render_tree = dom_to_render_tree(dom.document.clone(), &mut Discard {}).unwrap(); - RenderTree(render_tree) + let render_tree = dom_to_render_tree(dom.document.clone(), &mut Discard {})? + .ok_or(Error::Fail)?; + Ok(RenderTree(render_tree)) } /// Reads HTML from `input`, decorates it using `decorator`, and @@ -1694,6 +1712,7 @@ where { config::with_decorator(decorator) .string_from_read(input, width) + .expect("Failed to convert to HTML") } /// Reads HTML from `input`, and returns a `String` with text wrapped to @@ -1704,6 +1723,7 @@ where { config::plain() .string_from_read(input, width) + .expect("Failed to convert to HTML") } /// Reads HTML from `input`, and returns text wrapped to `width` columns. @@ -1715,6 +1735,7 @@ where { config::rich() .lines_from_read(input, width) + .expect("Failed to convert to HTML") } #[cfg(feature = "ansi_colours")] diff --git a/src/render/mod.rs b/src/render/mod.rs index 3eb8c46..90b76d6 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -9,7 +9,7 @@ pub trait Renderer { fn add_empty_line(&mut self); /// Create a sub-renderer for nested blocks. - fn new_sub_renderer(&self, width: usize) -> Self; + fn new_sub_renderer(&self, width: usize) -> crate::Result where Self: Sized; /// Start a new block. fn start_block(&mut self); diff --git a/src/render/text_renderer.rs b/src/render/text_renderer.rs index c2d9298..8b97c2b 100644 --- a/src/render/text_renderer.rs +++ b/src/render/text_renderer.rs @@ -974,8 +974,11 @@ impl Renderer for SubRenderer { html_trace_quiet!("add_empty_line: new lines: {:?}", self.lines); } - fn new_sub_renderer(&self, width: usize) -> Self { - SubRenderer::new(width, self.decorator.make_subblock_decorator()) + fn new_sub_renderer(&self, width: usize) -> crate::Result { + if width < 1 { + return Err(crate::Error::TooNarrow); + } + Ok(SubRenderer::new(width, self.decorator.make_subblock_decorator())) } fn start_block(&mut self) { diff --git a/src/tests.rs b/src/tests.rs index b6d9ec2..8dab263 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1,3 +1,5 @@ +use crate::{config, Error}; + use super::render::text_renderer::{RichAnnotation, TaggedLine, TrivialDecorator}; use super::{from_read, from_read_with_decorator, parse, TextDecorator}; @@ -13,6 +15,18 @@ macro_rules! assert_eq_str { fn test_html(input: &[u8], expected: &str, width: usize) { assert_eq_str!(from_read(input, width), expected); } +fn test_html_err(input: &[u8], expected: Error, width: usize) { + let result = config::plain() + .string_from_read(input, width); + match result { + Err(e) => { + assert_eq!(e, expected); + } + Ok(text) => { + panic!("Expected error, got: [[{}]]", text); + } + } +} fn test_html_decorator(input: &[u8], expected: &str, width: usize, decorator: D) where @@ -981,7 +995,7 @@ hi, world #[test] fn test_header_width() { //0 size - test_html( + test_html_err( br##"

    @@ -989,21 +1003,11 @@ fn test_header_width() {

    "##, - r#"## ### A -## ### n -## ### y -## ### t -## ### h -## ### i -## ### n -## ### g -## -## -"#, + Error::TooNarrow, 7, ); //Underflow - test_html( + test_html_err( br##"

    @@ -1011,17 +1015,7 @@ fn test_header_width() {

    "##, - r#"## ### A -## ### n -## ### y -## ### t -## ### h -## ### i -## ### n -## ### g -## -## -"#, + Error::TooNarrow, 5, ); } @@ -1106,29 +1100,42 @@ fn test_s() { fn test_multi_parse() { let html: &[u8] = b"one two three four five six seven eight nine ten eleven twelve thirteen \ fourteen fifteen sixteen seventeen"; - let tree = parse(html); + let tree = parse(html).unwrap(); assert_eq!( "one two three four five six seven eight nine ten eleven twelve thirteen fourteen\n\ fifteen sixteen seventeen\n", - tree.clone().render_plain(80).into_string() + tree.clone() + .render_plain(80) + .unwrap() + .into_string() ); assert_eq!( "one two three four five six seven eight nine ten eleven twelve\n\ thirteen fourteen fifteen sixteen seventeen\n", - tree.clone().render_plain(70).into_string() + tree.clone() + .render_plain(70) + .unwrap() + .into_string() ); assert_eq!( "one two three four five six seven eight nine ten\n\ eleven twelve thirteen fourteen fifteen sixteen\n\ seventeen\n", - tree.clone().render_plain(50).into_string() + tree.clone() + .render_plain(50) + .unwrap() + .into_string() ); } #[test] fn test_read_rich() { let html: &[u8] = b"bold"; - let lines = parse(html).render_rich(80).into_lines(); + let lines = parse(html) + .unwrap() + .render_rich(80) + .unwrap() + .into_lines(); let tag = vec![RichAnnotation::Strong]; let line = TaggedLine::from_string("*bold*".to_owned(), &tag); assert_eq!(vec![line], lines); @@ -1137,7 +1144,11 @@ fn test_read_rich() { #[test] fn test_read_custom() { let html: &[u8] = b"bold"; - let lines = parse(html).render(80, TrivialDecorator::new()).into_lines(); + let lines = parse(html) + .unwrap() + .render(80, TrivialDecorator::new()) + .unwrap() + .into_lines(); let tag = vec![()]; let line = TaggedLine::from_string("bold".to_owned(), &tag); assert_eq!(vec![line], lines); @@ -1148,7 +1159,9 @@ fn test_pre_rich() { use RichAnnotation::*; assert_eq!( crate::parse("
    test
    ".as_bytes()) + .unwrap() .render_rich(100) + .unwrap() .into_lines(), [TaggedLine::from_string( "test".into(), @@ -1158,7 +1171,9 @@ fn test_pre_rich() { assert_eq!( crate::parse("
    testlong
    ".as_bytes()) + .unwrap() .render_rich(4) + .unwrap() .into_lines(), [ TaggedLine::from_string("test".into(), &vec![Preformat(false)]), @@ -1256,7 +1271,9 @@ fn test_finalise() { assert_eq!( crate::parse("test".as_bytes()) + .unwrap() .render(80, TestDecorator) + .unwrap() .into_lines(), vec![ TaggedLine::from_string("test".to_owned(), &Vec::new()), @@ -1372,13 +1389,10 @@ fn test_table_empty_single_row_empty_cell() { #[test] fn test_renderer_zero_width() { - test_html( + test_html_err( br##"
    • x
    "##, -// Unfortunately the "x" ends up not being rendered as it doesn't fit. - r#"* - -"#, + Error::TooNarrow, 2, ); } @@ -1555,3 +1569,23 @@ fn test_links_outside_table() { " ); } + +#[test] +fn test_narrow_width_nested() { + use crate::Error; + // Check different things which cause narrowing + for html in [ + r#"

    Hi

    "#, + r#"
    Hi
    "#, + r#"
    • Hi
    "#, + r#"
    1. Hi
      2. "#, + r#"
      Foo
      Definition of foo
      "#, + ] { + let result = config::plain().string_from_read(html.as_bytes(), 1); + if let Err(Error::TooNarrow) = result { + // ok + } else { + panic!("Expected too narrow, got: {:?}", result); + } + } +} From 998d2f6cfde6e97985bfadf69672acd70374c0d1 Mon Sep 17 00:00:00 2001 From: Chris Emerson Date: Wed, 13 Dec 2023 22:03:27 +0000 Subject: [PATCH 3/3] Add Result to most of the Renderer methods. Add another test case, which has a 2 cell wide character which tries to fit into a width of 1; this previously had an infinite loop but will now return an error. --- src/lib.rs | 208 ++++++++++++++++++------------------ src/render/mod.rs | 45 ++++---- src/render/text_renderer.rs | 189 ++++++++++++++++++-------------- src/tests.rs | 26 ++++- 4 files changed, 262 insertions(+), 206 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9ba0f05..23c400d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -542,7 +542,7 @@ fn precalc_size_estimate<'a>(node: &'a RenderNode) -> Result(node: &'a RenderNode) -> Result( html_trace!("Found in table: {:?}", bodynode.info); } } - Some(RenderNode::new(RenderNodeInfo::Table(RenderTable::new( + Ok(Some(RenderNode::new(RenderNodeInfo::Table(RenderTable::new( rows, - )))) + ))))) }) } @@ -698,7 +698,7 @@ fn tbody_to_render_tree<'a, 'b, T: Write>( } } - Some(RenderNode::new(RenderNodeInfo::TableBody(rows))) + Ok(Some(RenderNode::new(RenderNodeInfo::TableBody(rows)))) }) } @@ -719,13 +719,13 @@ fn tr_to_render_tree<'a, 'b, T: Write>( } }) .collect(); - Some(RenderNode::new(RenderNodeInfo::TableRow( + Ok(Some(RenderNode::new(RenderNodeInfo::TableRow( RenderTableRow { cells, col_sizes: None, }, false, - ))) + )))) }) } @@ -744,21 +744,21 @@ fn td_to_render_tree<'a, 'b, T: Write>( } } pending(handle, move |_, children| { - Some(RenderNode::new(RenderNodeInfo::TableCell( + Ok(Some(RenderNode::new(RenderNodeInfo::TableCell( RenderTableCell { colspan, content: children, size_estimate: Cell::new(None), col_width: None, }, - ))) + )))) }) } /// A reducer which combines results from mapping children into /// the result for the current node. Takes a context and a /// vector of results and returns a new result (or nothing). -type ResultReducer<'a, C, R> = dyn Fn(&mut C, Vec) -> Option + 'a; +type ResultReducer<'a, C, R> = dyn Fn(&mut C, Vec) -> Result> + 'a; /// A closure to call before processing a child node. type ChildPreFn = dyn Fn(&mut C, &N) -> Result<()>; @@ -766,7 +766,7 @@ type ChildPreFn = dyn Fn(&mut C, &N) -> Result<()>; /// A closure to call after processing a child node, /// before adding the result to the processed results /// vector. -type ChildPostFn = dyn Fn(&mut C, &R); +type ChildPostFn = dyn Fn(&mut C, &R) -> Result<()>; /// The result of trying to render one node. enum TreeMapResult<'a, C, N, R> { @@ -805,7 +805,7 @@ where let mut pending_stack = vec![PendingNode { // We only expect one child, which we'll just return. - construct: Box::new(|_, mut cs| cs.pop()), + construct: Box::new(|_, mut cs| Ok(cs.pop())), prefn: None, postfn: None, children: Vec::new(), @@ -851,7 +851,7 @@ where } else { // No more children, so finally construct the parent. let completed = pending_stack.pop().unwrap(); - let reduced = (completed.construct)(context, completed.children); + let reduced = (completed.construct)(context, completed.children)?; if let Some(node) = reduced { if let Some(parent) = pending_stack.last_mut() { parent.postfn.as_ref().map(|ref f| f(context, &node)); @@ -884,7 +884,7 @@ pub fn dom_to_render_tree(handle: Handle, err_out: &mut T) -> Result(handle: Handle, f: F) -> TreeMapResult<'a, (), Handle, RenderNode> where //for<'a> F: Fn(&'a mut C, Vec) -> Option+'static - for<'r> F: Fn(&'r mut (), std::vec::Vec) -> Option + 'static, + for<'r> F: Fn(&'r mut (), std::vec::Vec) -> Result> + 'static, { TreeMapResult::PendingChildren { children: handle.children.borrow().clone(), @@ -963,7 +963,7 @@ fn process_dom_node<'a, 'b, T: Write>( use TreeMapResult::*; Ok(match handle.clone().data { - Document => pending(handle, |&mut (), cs| Some(RenderNode::new(Container(cs)))), + Document => pending(handle, |&mut (), cs| Ok(Some(RenderNode::new(Container(cs))))), Comment { .. } => Nothing, Element { ref name, @@ -976,7 +976,7 @@ fn process_dom_node<'a, 'b, T: Write>( | expanded_name!(html "span") | expanded_name!(html "body") => { /* process children, but don't add anything */ - pending(handle, |_, cs| Some(RenderNode::new(Container(cs)))) + pending(handle, |_, cs| Ok(Some(RenderNode::new(Container(cs))))) } expanded_name!(html "link") | expanded_name!(html "meta") @@ -1008,27 +1008,27 @@ fn process_dom_node<'a, 'b, T: Write>( let href: String = href.into(); Box::new(move |_, cs: Vec| { if cs.iter().any(|c| !c.is_shallow_empty()) { - Some(RenderNode::new(Link(href.clone(), cs))) + Ok(Some(RenderNode::new(Link(href.clone(), cs)))) } else { - None + Ok(None) } }) } else { - Box::new(|_, cs| Some(RenderNode::new(Container(cs)))) + Box::new(|_, cs| Ok(Some(RenderNode::new(Container(cs))))) }, prefn: None, postfn: None, } } - expanded_name!(html "em") => pending(handle, |_, cs| Some(RenderNode::new(Em(cs)))), + expanded_name!(html "em") => pending(handle, |_, cs| Ok(Some(RenderNode::new(Em(cs))))), expanded_name!(html "strong") => { - pending(handle, |_, cs| Some(RenderNode::new(Strong(cs)))) + pending(handle, |_, cs| Ok(Some(RenderNode::new(Strong(cs))))) } expanded_name!(html "s") => { - pending(handle, |_, cs| Some(RenderNode::new(Strikeout(cs)))) + pending(handle, |_, cs| Ok(Some(RenderNode::new(Strikeout(cs))))) } expanded_name!(html "code") => { - pending(handle, |_, cs| Some(RenderNode::new(Code(cs)))) + pending(handle, |_, cs| Ok(Some(RenderNode::new(Code(cs))))) } expanded_name!(html "img") => { let borrowed = attrs.borrow(); @@ -1057,17 +1057,17 @@ fn process_dom_node<'a, 'b, T: Write>( | expanded_name!(html "h4") => { let level: usize = name.local[1..].parse().unwrap(); pending(handle, move |_, cs| { - Some(RenderNode::new(Header(level, cs))) + Ok(Some(RenderNode::new(Header(level, cs)))) }) } expanded_name!(html "p") => { - pending(handle, |_, cs| Some(RenderNode::new(Block(cs)))) + pending(handle, |_, cs| Ok(Some(RenderNode::new(Block(cs))))) } expanded_name!(html "div") => { - pending(handle, |_, cs| Some(RenderNode::new(Div(cs)))) + pending(handle, |_, cs| Ok(Some(RenderNode::new(Div(cs))))) } expanded_name!(html "pre") => { - pending(handle, |_, cs| Some(RenderNode::new(Pre(cs)))) + pending(handle, |_, cs| Ok(Some(RenderNode::new(Pre(cs))))) } expanded_name!(html "br") => Finished(RenderNode::new(Break)), expanded_name!(html "table") => table_to_render_tree(handle.clone(), err_out), @@ -1079,7 +1079,7 @@ fn process_dom_node<'a, 'b, T: Write>( td_to_render_tree(handle.clone(), err_out) } expanded_name!(html "blockquote") => { - pending(handle, |_, cs| Some(RenderNode::new(BlockQuote(cs)))) + pending(handle, |_, cs| Ok(Some(RenderNode::new(BlockQuote(cs))))) } expanded_name!(html "ul") => Finished(RenderNode::new(Ul( list_children_to_render_nodes(handle.clone(), err_out)?, @@ -1104,7 +1104,7 @@ fn process_dom_node<'a, 'b, T: Write>( ))), _ => { html_trace!("Unhandled element: {:?}\n", name.local); - pending(handle, |_, cs| Some(RenderNode::new(Container(cs)))) + pending(handle, |_, cs| Ok(Some(RenderNode::new(Container(cs))))) //None } }; @@ -1137,9 +1137,9 @@ fn process_dom_node<'a, 'b, T: Write>( postfn, cons: Box::new(move |ctx, ch| { let fragnode = RenderNode::new(FragStart(fragname.clone())); - match cons(ctx, ch) { - None => Some(fragnode), - Some(node) => Some(prepend_marker(fragnode, node)), + match cons(ctx, ch)? { + None => Ok(Some(fragnode)), + Some(node) => Ok(Some(prepend_marker(fragnode, node))), } }), } @@ -1176,7 +1176,7 @@ fn render_tree_to_string( let lines = renderer.finalise(links); // And add the links if !lines.is_empty() { - renderer.start_block(); + renderer.start_block()?; renderer.fmt_links(lines); } Ok(renderer) @@ -1185,7 +1185,7 @@ fn render_tree_to_string( fn pending2< 'a, D: TextDecorator, - F: Fn(&mut TextRenderer, Vec>>) -> Option>> + F: Fn(&mut TextRenderer, Vec>>) -> Result>>> + 'static, >( children: Vec, @@ -1209,54 +1209,54 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( use TreeMapResult::*; Ok(match tree.info { Text(ref tstr) => { - renderer.add_inline_text(tstr); + renderer.add_inline_text(tstr)?; Finished(None) } - Container(children) => pending2(children, |_, _| Some(None)), + Container(children) => pending2(children, |_, _| Ok(Some(None))), Link(href, children) => { - renderer.start_link(&href); + renderer.start_link(&href)?; pending2(children, |renderer: &mut TextRenderer, _| { - renderer.end_link(); - Some(None) + renderer.end_link()?; + Ok(Some(None)) }) } Em(children) => { - renderer.start_emphasis(); + renderer.start_emphasis()?; pending2(children, |renderer: &mut TextRenderer, _| { - renderer.end_emphasis(); - Some(None) + renderer.end_emphasis()?; + Ok(Some(None)) }) } Strong(children) => { - renderer.start_strong(); + renderer.start_strong()?; pending2(children, |renderer: &mut TextRenderer, _| { - renderer.end_strong(); - Some(None) + renderer.end_strong()?; + Ok(Some(None)) }) } Strikeout(children) => { - renderer.start_strikeout(); + renderer.start_strikeout()?; pending2(children, |renderer: &mut TextRenderer, _| { - renderer.end_strikeout(); - Some(None) + renderer.end_strikeout()?; + Ok(Some(None)) }) } Code(children) => { - renderer.start_code(); + renderer.start_code()?; pending2(children, |renderer: &mut TextRenderer, _| { - renderer.end_code(); - Some(None) + renderer.end_code()?; + Ok(Some(None)) }) } Img(src, title) => { - renderer.add_image(&src, &title); + renderer.add_image(&src, &title)?; Finished(None) } Block(children) => { - renderer.start_block(); + renderer.start_block()?; pending2(children, |renderer: &mut TextRenderer, _| { renderer.end_block(); - Some(None) + Ok(Some(None)) }) } Header(level, children) => { @@ -1266,26 +1266,26 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( pending2(children, move |renderer: &mut TextRenderer, _| { let sub_builder = renderer.pop(); - renderer.start_block(); - renderer.append_subrender(sub_builder, repeat(&prefix[..])); + renderer.start_block()?; + renderer.append_subrender(sub_builder, repeat(&prefix[..]))?; renderer.end_block(); - Some(None) + Ok(Some(None)) }) } Div(children) => { - renderer.new_line(); + renderer.new_line()?; pending2(children, |renderer: &mut TextRenderer, _| { - renderer.new_line(); - Some(None) + renderer.new_line()?; + Ok(Some(None)) }) } Pre(children) => { - renderer.new_line(); + renderer.new_line()?; renderer.start_pre(); pending2(children, |renderer: &mut TextRenderer, _| { - renderer.new_line(); + renderer.new_line()?; renderer.end_pre(); - Some(None) + Ok(Some(None)) }) } BlockQuote(children) => { @@ -1295,21 +1295,21 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( pending2(children, move |renderer: &mut TextRenderer, _| { let sub_builder = renderer.pop(); - renderer.start_block(); - renderer.append_subrender(sub_builder, repeat(&prefix[..])); + renderer.start_block()?; + renderer.append_subrender(sub_builder, repeat(&prefix[..]))?; renderer.end_block(); - Some(None) + Ok(Some(None)) }) } Ul(items) => { - renderer.start_block(); + renderer.start_block()?; let prefix = renderer.unordered_item_prefix(); let prefix_len = prefix.len(); TreeMapResult::PendingChildren { children: items, - cons: Box::new(|_, _| Some(None)), + cons: Box::new(|_, _| Ok(Some(None))), prefn: Some(Box::new(move |renderer: &mut TextRenderer, _| { let sub_builder = renderer.new_sub_renderer(renderer.width().checked_sub(prefix_len).ok_or(Error::TooNarrow)?)?; renderer.push(sub_builder); @@ -1323,12 +1323,13 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( renderer.append_subrender( sub_builder, once(&prefix[..]).chain(repeat(&indent[..])), - ); + )?; + Ok(()) })), } } Ol(start, items) => { - renderer.start_block(); + renderer.start_block()?; let num_items = items.len(); @@ -1344,7 +1345,7 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( TreeMapResult::PendingChildren { children: items, - cons: Box::new(|_, _| Some(None)), + cons: Box::new(|_, _| Ok(Some(None))), prefn: Some(Box::new(move |renderer: &mut TextRenderer, _| { let sub_builder = renderer.new_sub_renderer(renderer.width().checked_sub(prefix_width).ok_or(Error::TooNarrow)?)?; renderer.push(sub_builder); @@ -1358,27 +1359,28 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( renderer.append_subrender( sub_builder, once(prefix1.as_str()).chain(repeat(prefixn.as_str())), - ); + )?; i.set(i.get() + 1); + Ok(()) })), } } Dl(items) => { - renderer.start_block(); + renderer.start_block()?; TreeMapResult::PendingChildren { children: items, - cons: Box::new(|_, _| Some(None)), + cons: Box::new(|_, _| Ok(Some(None))), prefn: None, postfn: None, } } Dt(children) => { - renderer.new_line(); - renderer.start_emphasis(); + renderer.new_line()?; + renderer.start_emphasis()?; pending2(children, |renderer: &mut TextRenderer, _| { - renderer.end_emphasis(); - Some(None) + renderer.end_emphasis()?; + Ok(Some(None)) }) } Dd(children) => { @@ -1386,15 +1388,15 @@ fn do_render_node<'a, 'b, T: Write, D: TextDecorator>( renderer.push(sub_builder); pending2(children, |renderer: &mut TextRenderer, _| { let sub_builder = renderer.pop(); - renderer.append_subrender(sub_builder, repeat(" ")); - Some(None) + renderer.append_subrender(sub_builder, repeat(" "))?; + Ok(Some(None)) }) } Break => { - renderer.new_line_hard(); + renderer.new_line_hard()?; Finished(None) } - Table(tab) => render_table_tree(renderer, tab, err_out), + Table(tab) => render_table_tree(renderer, tab, err_out)?, TableRow(row, false) => render_table_row(renderer, row, err_out), TableRow(row, true) => render_table_row_vert(renderer, row, err_out), TableBody(_) => unimplemented!("Unexpected TableBody while rendering"), @@ -1410,7 +1412,7 @@ fn render_table_tree( renderer: &mut TextRenderer, table: RenderTable, _err_out: &mut T, -) -> TreeMapResult<'static, TextRenderer, RenderNode, Option>> { +) -> Result, RenderNode, Option>>> { /* Now lay out the table. */ let num_columns = table.num_columns; @@ -1490,7 +1492,7 @@ fn render_table_tree( } } - renderer.start_block(); + renderer.start_block()?; let table_width = if vert_row { width @@ -1503,14 +1505,14 @@ fn render_table_tree( .saturating_sub(1) }; - renderer.add_horizontal_border_width(table_width); + renderer.add_horizontal_border_width(table_width)?; - TreeMapResult::PendingChildren { + Ok(TreeMapResult::PendingChildren { children: table.into_rows(col_widths, vert_row), - cons: Box::new(|_, _| Some(None)), + cons: Box::new(|_, _| Ok(Some(None))), prefn: Some(Box::new(|_, _| {Ok(())})), - postfn: Some(Box::new(|_, _| {})), - } + postfn: Some(Box::new(|_, _| {Ok(())})), + }) } fn render_table_row( @@ -1523,9 +1525,9 @@ fn render_table_row( cons: Box::new(|builders, children| { let children: Vec<_> = children.into_iter().map(Option::unwrap).collect(); if children.iter().any(|c| !c.empty()) { - builders.append_columns_with_borders(children, true); + builders.append_columns_with_borders(children, true)?; } - Some(None) + Ok(Some(None)) }), prefn: Some(Box::new(|renderer: &mut TextRenderer, node| { if let RenderNodeInfo::TableCell(ref cell) = node.info { @@ -1536,7 +1538,7 @@ fn render_table_row( panic!() } })), - postfn: Some(Box::new(|_renderer: &mut TextRenderer, _| {})), + postfn: Some(Box::new(|_renderer: &mut TextRenderer, _| {Ok(())})), } } @@ -1549,8 +1551,8 @@ fn render_table_row_vert( children: row.into_cells(true), cons: Box::new(|builders, children| { let children: Vec<_> = children.into_iter().map(Option::unwrap).collect(); - builders.append_vert_row(children); - Some(None) + builders.append_vert_row(children)?; + Ok(Some(None)) }), prefn: Some(Box::new(|renderer: &mut TextRenderer, node| { if let RenderNodeInfo::TableCell(ref cell) = node.info { @@ -1561,7 +1563,7 @@ fn render_table_row_vert( Err(Error::Fail) } })), - postfn: Some(Box::new(|_renderer: &mut TextRenderer, _| {})), + postfn: Some(Box::new(|_renderer: &mut TextRenderer, _| {Ok(())})), } } @@ -1572,7 +1574,7 @@ fn render_table_cell( ) -> TreeMapResult<'static, TextRenderer, RenderNode, Option>> { pending2(cell.content, |renderer: &mut TextRenderer, _| { let sub_builder = renderer.pop(); - Some(Some(sub_builder)) + Ok(Some(Some(sub_builder))) }) } @@ -1594,7 +1596,7 @@ pub mod config { /// Reads HTML from `input`, and returns a `String` with text wrapped to /// `width` columns. pub fn string_from_read(self, input: R, width: usize) -> Result { - Ok(parse(input)?.render(width, self.decorator)?.into_string()) + Ok(parse(input)?.render(width, self.decorator)?.into_string()?) } /// Reads HTML from `input`, and returns text wrapped to `width` columns. @@ -1604,7 +1606,7 @@ pub mod config { pub fn lines_from_read(self, input: R, width: usize) -> Result>>> { Ok(parse(input)? .render(width, self.decorator)? - .into_lines()) + .into_lines()?) } } @@ -1670,18 +1672,18 @@ pub struct RenderedText(SubRenderer); impl RenderedText { /// Convert the rendered HTML document to a string. - pub fn into_string(self) -> String { + pub fn into_string(self) -> Result { self.0.into_string() } /// Convert the rendered HTML document to a vector of lines with the annotations created by the /// decorator. - pub fn into_lines(self) -> Vec>> { - self.0 - .into_lines() + pub fn into_lines(self) -> Result>>> { + Ok(self.0 + .into_lines()? .into_iter() .map(RenderLine::into_tagged_line) - .collect() + .collect()) } } diff --git a/src/render/mod.rs b/src/render/mod.rs index 90b76d6..4757610 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -1,34 +1,37 @@ //! Module containing the `Renderer` interface for constructing a //! particular text output. +use crate::Error; + pub mod text_renderer; /// A type which is a backend for HTML to text rendering. pub trait Renderer { /// Add an empty line to the output (ie between blocks). - fn add_empty_line(&mut self); + fn add_empty_line(&mut self) -> crate::Result<()>; /// Create a sub-renderer for nested blocks. fn new_sub_renderer(&self, width: usize) -> crate::Result where Self: Sized; /// Start a new block. - fn start_block(&mut self); + fn start_block(&mut self) -> crate::Result<()>; /// Mark the end of a block. fn end_block(&mut self); /// Start a new line, if necessary (but don't add a new line). - fn new_line(&mut self); + fn new_line(&mut self) -> Result<(), Error>; /// Start a new line. - fn new_line_hard(&mut self); + fn new_line_hard(&mut self) -> Result<(), Error>; /// Add a horizontal table border. - fn add_horizontal_border(&mut self); + fn add_horizontal_border(&mut self) -> Result<(), Error>; /// Add a horizontal border which is not the full width - fn add_horizontal_border_width(&mut self, #[allow(unused_variables)] width: usize) { - self.add_horizontal_border(); + fn add_horizontal_border_width(&mut self, #[allow(unused_variables)] width: usize) + -> Result<(), Error> { + self.add_horizontal_border() } /// Begin a preformatted block. Until the corresponding end, @@ -40,7 +43,7 @@ pub trait Renderer { /// Add some inline text (which should be wrapped at the /// appropriate width) to the current block. - fn add_inline_text(&mut self, text: &str); + fn add_inline_text(&mut self, text: &str) -> crate::Result<()>; /// Return the current width in character cells fn width(&self) -> usize; @@ -51,6 +54,7 @@ pub trait Renderer { /// Add a new block from a sub renderer, and prefix every line by the /// corresponding text from each iteration of prefixes. fn append_subrender<'a, I>(&mut self, other: Self, prefixes: I) + -> Result<(), Error> where I: Iterator; @@ -59,13 +63,14 @@ pub trait Renderer { /// If collapse is true, then merge top/bottom borders of the subrenderer /// with the surrounding one. fn append_columns_with_borders(&mut self, cols: I, collapse: bool) + -> Result<(), Error> where I: IntoIterator, Self: Sized; /// Append a set of sub renderers joined vertically with lines, for tables /// which would otherwise be too wide for the screen. - fn append_vert_row(&mut self, cols: I) + fn append_vert_row(&mut self, cols: I) -> Result<(), Error> where I: IntoIterator, Self: Sized; @@ -79,37 +84,37 @@ pub trait Renderer { /// Start a hyperlink /// TODO: return sub-builder or similar to make misuse /// of start/link harder? - fn start_link(&mut self, target: &str); + fn start_link(&mut self, target: &str) -> crate::Result<()>; /// Finish a hyperlink started earlier. - fn end_link(&mut self); + fn end_link(&mut self) -> crate::Result<()>; /// Start an emphasised region - fn start_emphasis(&mut self); + fn start_emphasis(&mut self) -> crate::Result<()>; /// Finish emphasised text started earlier. - fn end_emphasis(&mut self); + fn end_emphasis(&mut self) -> crate::Result<()>; /// Start a strong region - fn start_strong(&mut self); + fn start_strong(&mut self) -> crate::Result<()>; /// Finish strong text started earlier. - fn end_strong(&mut self); + fn end_strong(&mut self) -> crate::Result<()>; /// Start a strikeout region - fn start_strikeout(&mut self); + fn start_strikeout(&mut self) -> crate::Result<()>; /// Finish strikeout text started earlier. - fn end_strikeout(&mut self); + fn end_strikeout(&mut self) -> crate::Result<()>; /// Start a code region - fn start_code(&mut self); + fn start_code(&mut self) -> crate::Result<()>; /// End a code region - fn end_code(&mut self); + fn end_code(&mut self) -> crate::Result<()>; /// Add an image - fn add_image(&mut self, src: &str, title: &str); + fn add_image(&mut self, src: &str, title: &str) -> crate::Result<()>; /// Get prefix string of header in specific level. fn header_prefix(&mut self, level: usize) -> String; diff --git a/src/render/text_renderer.rs b/src/render/text_renderer.rs index 8b97c2b..e49c64b 100644 --- a/src/render/text_renderer.rs +++ b/src/render/text_renderer.rs @@ -3,6 +3,8 @@ //! This module implements helpers and concrete types for rendering from HTML //! into different text formats. +use crate::Error; + use super::Renderer; use std::cell::Cell; use std::mem; @@ -48,9 +50,10 @@ impl TextRenderer { // hack overloads start_link method otherwise coming from the Renderer trait // impl on SubRenderer /// Add link to global link collection - pub fn start_link(&mut self, target: &str) { + pub fn start_link(&mut self, target: &str) -> crate::Result<()> { self.links.push(target.to_string()); - self.subrender.last_mut().unwrap().start_link(target); + self.subrender.last_mut().unwrap().start_link(target)?; + Ok(()) } /// Push a new builder onto the stack @@ -301,7 +304,7 @@ impl WrappedBlock { } } - fn flush_word(&mut self) { + fn flush_word(&mut self) -> Result<(), Error> { use self::TaggedLineElement::Str; /* Finish the word. */ @@ -359,6 +362,12 @@ impl WrappedBlock { if c_w <= lineleft { lineleft -= c_w; } else { + // Check if we've made no progress, for example + // if the first character is 2 cells wide and we + // only have a width of 1. + if idx == 0 { + return Err(Error::TooNarrow); + } split_idx = idx; break; } @@ -389,6 +398,7 @@ impl WrappedBlock { } } self.wordlen = 0; + Ok(()) } fn flush_line(&mut self) { @@ -404,9 +414,10 @@ impl WrappedBlock { self.linelen = 0; } - fn flush(&mut self) { - self.flush_word(); + fn flush(&mut self) -> Result<(), Error> { + self.flush_word()?; self.flush_line(); + Ok(()) } /// Consume self and return a vector of lines. @@ -427,18 +438,18 @@ impl WrappedBlock { */ /// Consume self and return vector of lines including annotations. - pub fn into_lines(mut self) -> Vec> { - self.flush(); + pub fn into_lines(mut self) -> Result>, Error> { + self.flush()?; - self.text + Ok(self.text) } - pub fn add_text(&mut self, text: &str, tag: &T) { + pub fn add_text(&mut self, text: &str, tag: &T) -> Result<(), Error> { html_trace!("WrappedBlock::add_text({}), {:?}", text, tag); for c in text.chars() { if c.is_whitespace() { /* Whitespace is mostly ignored, except to terminate words. */ - self.flush_word(); + self.flush_word()?; self.spacetag = Some(tag.clone()); } else if let Some(charwidth) = UnicodeWidthChar::width(c) { /* Not whitespace; add to the current word. */ @@ -447,9 +458,11 @@ impl WrappedBlock { } html_trace_quiet!(" Added char {:?}, wordlen={}", c, self.wordlen); } + Ok(()) } - pub fn add_preformatted_text(&mut self, text: &str, tag_main: &T, tag_wrapped: &T) { + pub fn add_preformatted_text(&mut self, text: &str, tag_main: &T, tag_wrapped: &T) + -> Result<(), Error> { html_trace!( "WrappedBlock::add_preformatted_text({}), {:?}/{:?}", text, @@ -458,7 +471,7 @@ impl WrappedBlock { ); // Make sure that any previous word has been sent to the line, as we // bypass the word buffer. - self.flush_word(); + self.flush_word()?; for c in text.chars() { if let Some(charwidth) = UnicodeWidthChar::width(c) { @@ -506,6 +519,7 @@ impl WrappedBlock { } html_trace_quiet!(" Added char {:?}", c); } + Ok(()) } pub fn add_element(&mut self, elt: TaggedLineElement) { @@ -855,30 +869,32 @@ impl SubRenderer { } /// Flushes the current wrapped block into the lines. - fn flush_wrapping(&mut self) { + fn flush_wrapping(&mut self) -> Result<(), Error> { if let Some(w) = self.wrapping.take() { self.lines - .extend(w.into_lines().into_iter().map(RenderLine::Text)) + .extend(w.into_lines()?.into_iter().map(RenderLine::Text)) } + Ok(()) } /// Flush the wrapping text and border. Only one should have /// anything to do. - fn flush_all(&mut self) { - self.flush_wrapping(); + fn flush_all(&mut self) -> Result<(), Error> { + self.flush_wrapping()?; + Ok(()) } /// Consumes this renderer and return a multiline `String` with the result. - pub fn into_string(self) -> String { + pub fn into_string(self) -> Result { let mut result = String::new(); #[cfg(feature = "html_trace")] let width: usize = self.width; - for line in self.into_lines() { + for line in self.into_lines()? { result.push_str(&line.into_string()); result.push('\n'); } html_trace!("into_string({}, {:?})", width, result); - result + Ok(result) } #[cfg(feature = "html_trace")] @@ -940,14 +956,15 @@ impl SubRenderer { } /// Returns a `Vec` of `TaggedLine`s with the rendered text. - pub fn into_lines(mut self) -> LinkedList>> { - self.flush_wrapping(); - self.lines + pub fn into_lines(mut self) -> Result>>, Error> { + self.flush_wrapping()?; + Ok(self.lines) } - fn add_horizontal_line(&mut self, line: BorderHoriz) { - self.flush_wrapping(); + fn add_horizontal_line(&mut self, line: BorderHoriz) -> Result<(), Error> { + self.flush_wrapping()?; self.lines.push_back(RenderLine::Line(line)); + Ok(()) } } @@ -965,37 +982,39 @@ fn filter_text_strikeout(s: &str) -> Option { } impl Renderer for SubRenderer { - fn add_empty_line(&mut self) { + fn add_empty_line(&mut self) -> crate::Result<()> { html_trace!("add_empty_line()"); - self.flush_all(); + self.flush_all()?; self.lines.push_back(RenderLine::Text(TaggedLine::new())); html_trace_quiet!("add_empty_line: at_block_end <- false"); self.at_block_end = false; html_trace_quiet!("add_empty_line: new lines: {:?}", self.lines); + Ok(()) } fn new_sub_renderer(&self, width: usize) -> crate::Result { if width < 1 { - return Err(crate::Error::TooNarrow); + return Err(Error::TooNarrow); } Ok(SubRenderer::new(width, self.decorator.make_subblock_decorator())) } - fn start_block(&mut self) { + fn start_block(&mut self) -> crate::Result<()> { html_trace!("start_block({})", self.width); - self.flush_all(); + self.flush_all()?; if !self.lines.is_empty() { - self.add_empty_line(); + self.add_empty_line()?; } html_trace_quiet!("start_block; at_block_end <- false"); self.at_block_end = false; + Ok(()) } - fn new_line(&mut self) { - self.flush_all(); + fn new_line(&mut self) -> crate::Result<()> { + self.flush_all() } - fn new_line_hard(&mut self) { + fn new_line_hard(&mut self) -> Result<(), Error> { match self.wrapping { None => self.add_empty_line(), Some(WrappedBlock { @@ -1007,16 +1026,18 @@ impl Renderer for SubRenderer { } } - fn add_horizontal_border(&mut self) { - self.flush_wrapping(); + fn add_horizontal_border(&mut self) -> Result<(), Error> { + self.flush_wrapping()?; self.lines .push_back(RenderLine::Line(BorderHoriz::new(self.width))); + Ok(()) } - fn add_horizontal_border_width(&mut self, width: usize) { - self.flush_wrapping(); + fn add_horizontal_border_width(&mut self, width: usize) -> Result<(), Error> { + self.flush_wrapping()?; self.lines .push_back(RenderLine::Line(BorderHoriz::new(width))); + Ok(()) } fn start_pre(&mut self) { @@ -1035,14 +1056,14 @@ impl Renderer for SubRenderer { self.at_block_end = true; } - fn add_inline_text(&mut self, text: &str) { + fn add_inline_text(&mut self, text: &str) -> crate::Result<()> { html_trace!("add_inline_text({}, {})", self.width, text); if self.pre_depth == 0 && self.at_block_end && text.chars().all(char::is_whitespace) { // Ignore whitespace between blocks. - return; + return Ok(()); } if self.at_block_end { - self.start_block(); + self.start_block()?; } // ensure wrapping is set let _ = self.current_text(); @@ -1063,7 +1084,7 @@ impl Renderer for SubRenderer { self.wrapping .as_mut() .unwrap() - .add_text(filtered_text, &self.ann_stack); + .add_text(filtered_text, &self.ann_stack)?; } else { let mut tag_first = self.ann_stack.clone(); let mut tag_cont = self.ann_stack.clone(); @@ -1073,8 +1094,9 @@ impl Renderer for SubRenderer { filtered_text, &tag_first, &tag_cont, - ); + )?; } + Ok(()) } fn width(&self) -> usize { @@ -1085,17 +1107,17 @@ impl Renderer for SubRenderer { self.add_subblock(line); } - fn append_subrender<'a, I>(&mut self, other: Self, prefixes: I) + fn append_subrender<'a, I>(&mut self, other: Self, prefixes: I) -> Result<(), Error> where I: Iterator, { use self::TaggedLineElement::Str; - self.flush_wrapping(); + self.flush_wrapping()?; let tag = self.ann_stack.clone(); self.lines.extend( other - .into_lines() + .into_lines()? .into_iter() .zip(prefixes) .map(|(line, prefix)| match line { @@ -1122,9 +1144,10 @@ impl Renderer for SubRenderer { } }), ); + Ok(()) } - fn append_columns_with_borders(&mut self, cols: I, collapse: bool) + fn append_columns_with_borders(&mut self, cols: I, collapse: bool) -> Result<(), Error> where I: IntoIterator, Self: Sized, @@ -1133,7 +1156,7 @@ impl Renderer for SubRenderer { html_trace!("append_columns_with_borders(collapse={})", collapse); html_trace!("self=\n{}", self.to_string()); - self.flush_wrapping(); + self.flush_wrapping()?; let mut tot_width = 0; @@ -1143,10 +1166,10 @@ impl Renderer for SubRenderer { let width = sub_r.width; tot_width += width; html_trace!("Adding column:\n{}", sub_r.to_string()); - ( + Ok(( width, sub_r - .into_lines() + .into_lines()? .into_iter() .map(|mut line| { match line { @@ -1160,9 +1183,9 @@ impl Renderer for SubRenderer { line }) .collect(), - ) + )) }) - .collect::>)>>(); + .collect::>)>, Error>>()?; tot_width += line_sets.len().saturating_sub(1); @@ -1290,9 +1313,10 @@ impl Renderer for SubRenderer { self.lines.push_back(RenderLine::Text(line)); } self.lines.push_back(RenderLine::Line(next_border)); + Ok(()) } - fn append_vert_row(&mut self, cols: I) + fn append_vert_row(&mut self, cols: I) -> Result<(), Error> where I: IntoIterator, Self: Sized, @@ -1300,7 +1324,7 @@ impl Renderer for SubRenderer { html_trace!("append_vert_row()"); html_trace!("self=\n{}", self.to_string()); - self.flush_wrapping(); + self.flush_wrapping()?; let width = self.width(); @@ -1310,11 +1334,12 @@ impl Renderer for SubRenderer { first = false; } else { let border = BorderHoriz::new_type(width, BorderSegHoriz::StraightVert); - self.add_horizontal_line(border); + self.add_horizontal_line(border)?; } - self.append_subrender(col, std::iter::repeat("")); + self.append_subrender(col, std::iter::repeat(""))?; } - self.add_horizontal_border(); + self.add_horizontal_border()?; + Ok(()) } fn empty(&self) -> bool { @@ -1340,63 +1365,71 @@ impl Renderer for SubRenderer { result } - fn start_link(&mut self, target: &str) { + fn start_link(&mut self, target: &str) -> crate::Result<()> { let (s, annotation) = self.decorator.decorate_link_start(target); self.ann_stack.push(annotation); - self.add_inline_text(&s); + self.add_inline_text(&s) } - fn end_link(&mut self) { + fn end_link(&mut self) -> crate::Result<()> { let s = self.decorator.decorate_link_end(); - self.add_inline_text(&s); + self.add_inline_text(&s)?; self.ann_stack.pop(); + Ok(()) } - fn start_emphasis(&mut self) { + fn start_emphasis(&mut self) -> crate::Result<()> { let (s, annotation) = self.decorator.decorate_em_start(); self.ann_stack.push(annotation); - self.add_inline_text(&s); + self.add_inline_text(&s) } - fn end_emphasis(&mut self) { + fn end_emphasis(&mut self) -> crate::Result<()> { let s = self.decorator.decorate_em_end(); - self.add_inline_text(&s); + self.add_inline_text(&s)?; self.ann_stack.pop(); + Ok(()) } - fn start_strong(&mut self) { + fn start_strong(&mut self) -> crate::Result<()> { let (s, annotation) = self.decorator.decorate_strong_start(); self.ann_stack.push(annotation); - self.add_inline_text(&s); + self.add_inline_text(&s) } - fn end_strong(&mut self) { + fn end_strong(&mut self) -> crate::Result<()> { let s = self.decorator.decorate_strong_end(); - self.add_inline_text(&s); + self.add_inline_text(&s)?; self.ann_stack.pop(); + Ok(()) } - fn start_strikeout(&mut self) { + fn start_strikeout(&mut self) -> crate::Result<()> { let (s, annotation) = self.decorator.decorate_strikeout_start(); self.ann_stack.push(annotation); - self.add_inline_text(&s); + self.add_inline_text(&s)?; self.text_filter_stack.push(filter_text_strikeout); + Ok(()) } - fn end_strikeout(&mut self) { + fn end_strikeout(&mut self) -> crate::Result<()> { self.text_filter_stack.pop().expect("end_strikeout() called without a corresponding start_strokeout()"); let s = self.decorator.decorate_strikeout_end(); - self.add_inline_text(&s); + self.add_inline_text(&s)?; self.ann_stack.pop(); + Ok(()) } - fn start_code(&mut self) { + fn start_code(&mut self) -> crate::Result<()> { let (s, annotation) = self.decorator.decorate_code_start(); self.ann_stack.push(annotation); - self.add_inline_text(&s); + self.add_inline_text(&s)?; + Ok(()) } - fn end_code(&mut self) { + fn end_code(&mut self) -> crate::Result<()> { let s = self.decorator.decorate_code_end(); - self.add_inline_text(&s); + self.add_inline_text(&s)?; self.ann_stack.pop(); + Ok(()) } - fn add_image(&mut self, src: &str, title: &str) { + fn add_image(&mut self, src: &str, title: &str) -> crate::Result<()> { let (s, tag) = self.decorator.decorate_image(src, title); self.ann_stack.push(tag); - self.add_inline_text(&s); + self.add_inline_text(&s)?; self.ann_stack.pop(); + Ok(()) } fn header_prefix(&mut self, level: usize) -> String { diff --git a/src/tests.rs b/src/tests.rs index 8dab263..fcca2e7 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1108,6 +1108,7 @@ fn test_multi_parse() { .render_plain(80) .unwrap() .into_string() + .unwrap() ); assert_eq!( "one two three four five six seven eight nine ten eleven twelve\n\ @@ -1116,6 +1117,7 @@ fn test_multi_parse() { .render_plain(70) .unwrap() .into_string() + .unwrap() ); assert_eq!( "one two three four five six seven eight nine ten\n\ @@ -1125,6 +1127,7 @@ fn test_multi_parse() { .render_plain(50) .unwrap() .into_string() + .unwrap() ); } @@ -1135,7 +1138,8 @@ fn test_read_rich() { .unwrap() .render_rich(80) .unwrap() - .into_lines(); + .into_lines() + .unwrap(); let tag = vec![RichAnnotation::Strong]; let line = TaggedLine::from_string("*bold*".to_owned(), &tag); assert_eq!(vec![line], lines); @@ -1148,7 +1152,8 @@ fn test_read_custom() { .unwrap() .render(80, TrivialDecorator::new()) .unwrap() - .into_lines(); + .into_lines() + .unwrap(); let tag = vec![()]; let line = TaggedLine::from_string("bold".to_owned(), &tag); assert_eq!(vec![line], lines); @@ -1162,7 +1167,8 @@ fn test_pre_rich() { .unwrap() .render_rich(100) .unwrap() - .into_lines(), + .into_lines() + .unwrap(), [TaggedLine::from_string( "test".into(), &vec![Preformat(false)] @@ -1174,7 +1180,8 @@ fn test_pre_rich() { .unwrap() .render_rich(4) .unwrap() - .into_lines(), + .into_lines() + .unwrap(), [ TaggedLine::from_string("test".into(), &vec![Preformat(false)]), TaggedLine::from_string("long".into(), &vec![Preformat(true)]) @@ -1274,7 +1281,8 @@ fn test_finalise() { .unwrap() .render(80, TestDecorator) .unwrap() - .into_lines(), + .into_lines() + .unwrap(), vec![ TaggedLine::from_string("test".to_owned(), &Vec::new()), TaggedLine::new(), @@ -1589,3 +1597,11 @@ fn test_narrow_width_nested() { } } } + +#[test] +fn test_issue_93_x() { + let data=[60, 116, 97, 98, 108, 101, 62, 60, 116, 114, 62, 60, 116, 100, 62, 120, 105, 60, 48, 62, 0, 0, 0, 60, 116, 97, 98, 108, 101, 62, 58, 58, 58, 62, 58, 62, 62, 62, 58, 60, 112, 32, 32, 32, 32, 32, 32, 32, 71, 87, 85, 78, 16, 16, 62, 60, 15, 16, 16, 16, 16, 16, 16, 15, 38, 16, 16, 16, 15, 1, 16, 16, 16, 16, 16, 16, 162, 111, 107, 99, 91, 112, 57, 64, 94, 100, 60, 111, 108, 47, 62, 127, 60, 108, 73, 62, 125, 109, 121, 102, 99, 122, 110, 102, 114, 98, 60, 97, 32, 104, 114, 101, 102, 61, 98, 111, 103, 32, 105, 100, 61, 100, 62, 60, 111, 15, 15, 15, 15, 15, 15, 15, 39, 15, 15, 15, 106, 102, 59, 99, 32, 32, 32, 86, 102, 122, 110, 104, 93, 108, 71, 114, 117, 110, 100, 96, 121, 57, 60, 107, 116, 109, 247, 62, 60, 32, 60, 122, 98, 99, 98, 97, 32, 119, 127, 127, 62, 60, 112, 62, 121, 116, 60, 47, 116, 100, 62, 62, 60, 111, 98, 62, 123, 110, 109, 97, 101, 105, 119, 60, 112, 101, 101, 122, 102, 63, 120, 97, 62, 60, 101, 62, 60, 120, 109, 112, 32, 28, 52, 55, 50, 50, 49, 52, 185, 150, 99, 62, 255, 112, 76, 85, 60, 112, 62, 73, 100, 116, 116, 60, 75, 50, 73, 116, 120, 110, 127, 255, 118, 32, 42, 40, 49, 33, 112, 32, 36, 107, 57, 60, 5, 163, 62, 49, 55, 32, 33, 118, 99, 63, 60, 109, 107, 43, 119, 100, 62, 60, 104, 58, 101, 163, 163, 163, 163, 220, 220, 220, 220, 220, 220, 220, 220, 220, 220, 220, 220, 1, 107, 117, 107, 108, 44, 102, 58, 60, 116, 101, 97, 106, 98, 59, 60, 115, 109, 52, 58, 115, 98, 62, 232, 110, 114, 32, 60, 117, 93, 120, 112, 119, 111, 59, 98, 120, 61, 206, 19, 61, 206, 19, 59, 1, 110, 102, 60, 115, 0, 242, 64, 203, 8, 111, 50, 59, 121, 122, 32, 42, 35, 32, 37, 101, 120, 104, 121, 0, 242, 59, 63, 121, 231, 130, 130, 130, 170, 170, 1, 32, 0, 0, 0, 28, 134, 200, 90, 119, 48, 60, 111, 108, 118, 119, 116, 113, 59, 100, 60, 117, 43, 110, 99, 9, 216, 157, 137, 216, 157, 246, 167, 62, 60, 104, 61, 43, 28, 134, 200, 105, 119, 48, 60, 122, 110, 0, 242, 61, 61, 114, 231, 130, 130, 130, 170, 170, 170, 233, 222, 222, 162, 163, 163, 163, 163, 163, 163, 163, 85, 100, 116, 99, 61, 60, 163, 163, 163, 163, 163, 220, 220, 1, 109, 112, 105, 10, 59, 105, 220, 215, 10, 59, 122, 100, 100, 121, 97, 43, 43, 43, 102, 122, 100, 60, 62, 114, 116, 122, 115, 61, 60, 115, 101, 62, 215, 215, 215, 215, 215, 98, 59, 60, 109, 120, 57, 60, 97, 102, 113, 229, 43, 43, 43, 43, 43, 43, 43, 43, 43, 35, 43, 43, 101, 58, 60, 116, 98, 101, 107, 98, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 43, 98, 99, 62, 60, 112, 102, 59, 124, 107, 111, 97, 98, 108, 118, 60, 116, 102, 101, 104, 97, 62, 60, 255, 127, 46, 60, 116, 101, 62, 60, 105, 102, 63, 116, 116, 60, 47, 116, 101, 62, 62, 60, 115, 98, 62, 123, 109, 108, 97, 100, 119, 118, 60, 111, 99, 97, 103, 99, 62, 60, 255, 127, 46, 60, 103, 99, 62, 60, 116, 98, 63, 60, 101, 62, 60, 109, 109, 231, 130, 130, 130, 213, 213, 213, 233, 222, 222, 59, 101, 103, 58, 60, 100, 111, 61, 65, 114, 104, 60, 47, 101, 109, 62, 60, 99, 99, 172, 97, 97, 58, 60, 119, 99, 64, 126, 118, 104, 100, 100, 107, 105, 60, 120, 98, 255, 255, 255, 0, 60, 255, 127, 46, 60, 113, 127]; + let _local0 = crate::parse(&data[..]).unwrap(); + let d1 = TrivialDecorator::new(); + let _local1 = crate::RenderTree::render(_local0, 1, d1); +}