kemitix/paperoni
1
0
Fork 0
Code Issues 4 Pull requests 2 Projects Releases Packages Wiki Activity

Change function signature of next_element to return an Option

Browse source 
rather than mutate a given value.

The new function signature reads a little easier than before.
Remove TODO task in replace_brs
This commit is contained in:
Kenneth Gitere 2020-09-23 22:36:01 +03:00
parent 7fb09130e8
commit 7219198524
1 changed files with 13 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

40
src/moz_readability/mod.rs
View file

@ -143,16 +143,9 @@ impl Readability {
.borrow_mut() .borrow_mut()
.insert(attr_name, prev_value.value.clone()); .insert(attr_name, prev_value.value.clone());
} }
// TODO: Replace the code below with next_element
prev_elem.insert_after( let inner_node_child = Self::next_element(inner_node_ref.first_child());
inner_node_ref prev_elem.insert_after(inner_node_child.unwrap());
.first_child()
.unwrap()
.children()
.filter(Self::has_content)
.next()
.unwrap(),
);
prev_elem.detach(); prev_elem.detach();
} }
} }
@ -193,11 +186,9 @@ impl Readability {
/// <div>foo<br>bar<p>abc</p></div> /// <div>foo<br>bar<p>abc</p></div>
fn replace_brs(&mut self) { fn replace_brs(&mut self) {
if let Ok(mut br_tags) = self.root_node.select("br") { if let Ok(mut br_tags) = self.root_node.select("br") {
// TODO: Change to for loop
while let Some(br_tag) = br_tags.next() { while let Some(br_tag) = br_tags.next() {
let mut next = br_tag.as_node().next_sibling(); let mut next = Self::next_element(br_tag.as_node().next_sibling());
let mut replaced = false; let mut replaced = false;
Self::next_element(&mut next);
while let Some(next_elem) = next { while let Some(next_elem) = next {
if next_elem.as_element().is_some() if next_elem.as_element().is_some()
&& &next_elem.as_element().as_ref().unwrap().name.local == "br" && &next_elem.as_element().as_ref().unwrap().name.local == "br"
@ -205,11 +196,10 @@ impl Readability {
replaced = true; replaced = true;
let br_sibling = next_elem.next_sibling(); let br_sibling = next_elem.next_sibling();
next_elem.detach(); next_elem.detach();
next = br_sibling; next = Self::next_element(br_sibling);
} else { } else {
break; break;
} }
Self::next_element(&mut next);
} }
if replaced { if replaced {
let p = NodeRef::new_element( let p = NodeRef::new_element(
@ -309,22 +299,21 @@ impl Readability {
/// Finds the next element, starting from the given node, and ignoring /// Finds the next element, starting from the given node, and ignoring
/// whitespace in between. If the given node is an element, the same node is /// whitespace in between. If the given node is an element, the same node is
/// returned. /// returned.
fn next_element(node_ref: &mut Option<NodeRef>) { fn next_element(node_ref: Option<NodeRef>) -> Option<NodeRef> {
// The signature of this method affects how it is used in loops since the code let mut node_ref = node_ref;
// has to ensure that it is called before the next iteration. This probably
// makes it less obvious to understand the first time. This may change in the future.
while node_ref.is_some() { while node_ref.is_some() {
match node_ref.as_ref().unwrap().data() { match node_ref.as_ref().unwrap().data() {
NodeData::Element(_) => break, NodeData::Element(_) => break,
_ => { _ => {
if node_ref.as_ref().unwrap().text_contents().trim().is_empty() { if node_ref.as_ref().unwrap().text_contents().trim().is_empty() {
*node_ref = node_ref.as_ref().unwrap().next_sibling(); node_ref = node_ref.as_ref().unwrap().next_sibling();
} else { } else {
break; break;
} }
} }
} }
} }
node_ref
} }
/// Determine if a node qualifies as phrasing content. /// Determine if a node qualifies as phrasing content.
@ -433,7 +422,7 @@ mod test {
let p = doc.root_node.select_first("#a").unwrap(); let p = doc.root_node.select_first("#a").unwrap();
let p = p.as_node(); let p = p.as_node();
let mut p_node_option: Option<NodeRef> = Some(p.clone()); let mut p_node_option: Option<NodeRef> = Some(p.clone());
Readability::next_element(&mut p_node_option); p_node_option = Readability::next_element(p_node_option);
assert_eq!(Some(p.clone()), p_node_option); assert_eq!(Some(p.clone()), p_node_option);
let p_node_option = p_node_option.unwrap(); let p_node_option = p_node_option.unwrap();
@ -441,23 +430,20 @@ mod test {
let p_node_option_attr = p_node_option.unwrap().attributes.borrow(); let p_node_option_attr = p_node_option.unwrap().attributes.borrow();
assert_eq!("a", p_node_option_attr.get("id").unwrap()); assert_eq!("a", p_node_option_attr.get("id").unwrap());
let mut next = p.next_sibling(); let next = Readability::next_element(p.next_sibling());
Readability::next_element(&mut next);
let next = next.unwrap(); let next = next.unwrap();
let next_elem = next.as_element(); let next_elem = next.as_element();
let next_attr = next_elem.unwrap().attributes.borrow(); let next_attr = next_elem.unwrap().attributes.borrow();
assert_eq!("b", next_attr.get("id").unwrap()); assert_eq!("b", next_attr.get("id").unwrap());
let mut next = next.next_sibling(); let next = Readability::next_element(next.next_sibling());
Readability::next_element(&mut next);
let next = next.unwrap(); let next = next.unwrap();
assert_eq!(true, next.as_text().is_some()); assert_eq!(true, next.as_text().is_some());
assert_eq!("This is standalone text", next.text_contents().trim()); assert_eq!("This is standalone text", next.text_contents().trim());
let mut next = None; let next = Readability::next_element(None);
Readability::next_element(&mut next);
assert_eq!(None, next); assert_eq!(None, next);
} }