From aa3c955572fb335c439a614a40b17ca1396a3230 Mon Sep 17 00:00:00 2001 From: Mykola Humanov Date: Mon, 4 Nov 2024 20:43:34 +0200 Subject: [PATCH 1/4] tests/selection-manipulation.rs: add test to replace selection --- src/selection.rs | 8 ++++++++ tests/selection-manipulation.rs | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/selection.rs b/src/selection.rs index 213a2d2..53b06b1 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -332,7 +332,15 @@ impl<'a> Selection<'a> { /// the nodes from the given selection. /// /// This follows the same rules as `append`. + /// pub fn replace_with_selection(&self, sel: &Selection) { + // This is unable to replace multiple targets, + // in this implementation only last target will get a replacement, + // the other will be empty. + // TODO: re-implement! Either build a fragment from the selection, + // and then handle it like replace_with_html, + // or copy the tree fragment into other tree. + for node in self.nodes() { for prev_sibling in sel.nodes() { node.append_prev_sibling(&prev_sibling.id); diff --git a/tests/selection-manipulation.rs b/tests/selection-manipulation.rs index 1f6d82f..3883c71 100644 --- a/tests/selection-manipulation.rs +++ b/tests/selection-manipulation.rs @@ -159,3 +159,26 @@ fn test_prepend_html_multiple_elements_to_multiple() { assert_eq!(doc.select(r#"div > .first + .second + .third"#).length(), 2) } + +#[cfg_attr(not(target_arch = "wasm32"), test)] +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] +fn test_replace_with_selection_multiple() { + let contents = r#" + + TEST + +
+

span-to-replace

+
+ example + + "#; + + let doc = Document::from(contents); + + let sel_dst = doc.select(".content p span"); + let sel_src = doc.select("span.source"); + + sel_dst.replace_with_selection(&sel_src); + dbg!(doc.html()); +} \ No newline at end of file From 10c4fc1e0263799c6944ab61346cc5ce2ad44007 Mon Sep 17 00:00:00 2001 From: Mykola Humanov Date: Mon, 4 Nov 2024 21:04:36 +0200 Subject: [PATCH 2/4] src/selection.rs: temporary solution for `Selection::replace_with_selection` --- src/selection.rs | 14 ++++++++------ tests/selection-manipulation.rs | 4 +++- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/selection.rs b/src/selection.rs index 53b06b1..65d99e5 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -334,17 +334,19 @@ impl<'a> Selection<'a> { /// This follows the same rules as `append`. /// pub fn replace_with_selection(&self, sel: &Selection) { - // This is unable to replace multiple targets, - // in this implementation only last target will get a replacement, - // the other will be empty. // TODO: re-implement! Either build a fragment from the selection, // and then handle it like replace_with_html, // or copy the tree fragment into other tree. + let mut contents: StrTendril = StrTendril::new(); + sel.iter().for_each(|s| contents.push_tendril(&s.html())); + let fragment = Document::from(contents); + sel.remove(); + for node in self.nodes() { - for prev_sibling in sel.nodes() { - node.append_prev_sibling(&prev_sibling.id); - } + node.tree.merge_with_fn(fragment.tree.clone(), |node_id| { + node.append_prev_siblings(&node_id) + }); } self.remove() diff --git a/tests/selection-manipulation.rs b/tests/selection-manipulation.rs index 3883c71..912dead 100644 --- a/tests/selection-manipulation.rs +++ b/tests/selection-manipulation.rs @@ -169,8 +169,10 @@ fn test_replace_with_selection_multiple() {

span-to-replace

+

span-to-replace

example + example "#; @@ -180,5 +182,5 @@ fn test_replace_with_selection_multiple() { let sel_src = doc.select("span.source"); sel_dst.replace_with_selection(&sel_src); - dbg!(doc.html()); + assert_eq!(doc.select(".source").length(), 4) } \ No newline at end of file From d235508b9f85ba2d9a746f4c964b807e493b427d Mon Sep 17 00:00:00 2001 From: Mykola Humanov Date: Tue, 5 Nov 2024 10:30:16 +0200 Subject: [PATCH 3/4] src/selection.rs: solved `Selection::replace_with_selection` and `Selection::append_selection` --- Examples.md | 2 +- README.md | 2 +- src/selection.rs | 33 +++++++++----- tests/selection-manipulation.rs | 78 ++++++++++++++++++++++++--------- 4 files changed, 81 insertions(+), 34 deletions(-) diff --git a/Examples.md b/Examples.md index 307df58..29f2d43 100644 --- a/Examples.md +++ b/Examples.md @@ -612,7 +612,7 @@ sel.rename("p"); // after renaming, there are no `div` and `span` elements assert_eq!(doc.select("div.content > div, div.content > span").length(), 0); -// but there are three `p` elements +// but there are four `p` elements assert_eq!(doc.select("div.content > p").length(), 4); ``` \ No newline at end of file diff --git a/README.md b/README.md index ed9d77c..0c85440 100644 --- a/README.md +++ b/README.md @@ -643,7 +643,7 @@ sel.rename("p"); // after renaming, there are no `div` and `span` elements assert_eq!(doc.select("div.content > div, div.content > span").length(), 0); -// but there are three `p` elements +// but there are four `p` elements assert_eq!(doc.select("div.content > p").length(), 4); ``` diff --git a/src/selection.rs b/src/selection.rs index 65d99e5..15539e7 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -334,9 +334,8 @@ impl<'a> Selection<'a> { /// This follows the same rules as `append`. /// pub fn replace_with_selection(&self, sel: &Selection) { - // TODO: re-implement! Either build a fragment from the selection, - // and then handle it like replace_with_html, - // or copy the tree fragment into other tree. + //! This is working solution, but it's not optimal yet! + //! Note: goquery's behavior is taken as the basis. let mut contents: StrTendril = StrTendril::new(); sel.iter().for_each(|s| contents.push_tendril(&s.html())); @@ -352,6 +351,24 @@ impl<'a> Selection<'a> { self.remove() } + /// Appends the elements in the selection to the end of each element + /// in the set of matched elements. + pub fn append_selection(&self, sel: &Selection) { + //! This is working solution, but it's not optimal yet! + //! Note: goquery's behavior is taken as the basis. + + let mut contents: StrTendril = StrTendril::new(); + sel.iter().for_each(|s| contents.push_tendril(&s.html())); + let fragment = Document::from(contents); + sel.remove(); + + for node in self.nodes() { + node.tree.merge_with_fn(fragment.tree.clone(), |node_id| { + node.append_children(&node_id) + }); + } + } + /// Parses the html and appends it to the set of matched elements. pub fn append_html(&self, html: T) where @@ -380,15 +397,7 @@ impl<'a> Selection<'a> { } } - /// Appends the elements in the selection to the end of each element - /// in the set of matched elements. - pub fn append_selection(&self, sel: &Selection) { - for node in self.nodes() { - for child in sel.nodes() { - node.append_child(&child.id); - } - } - } + } // traversing methods diff --git a/tests/selection-manipulation.rs b/tests/selection-manipulation.rs index 912dead..0c62874 100644 --- a/tests/selection-manipulation.rs +++ b/tests/selection-manipulation.rs @@ -50,23 +50,6 @@ fn test_set_html_empty() { assert_eq!(doc.select("#main").children().length(), 0); } -#[cfg_attr(not(target_arch = "wasm32"), test)] -#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] -fn test_replace_with_selection() { - let doc = doc_with_siblings(); - - let s1 = doc.select("#nf5"); - let sel = doc.select("#nf6"); - - sel.replace_with_selection(&s1); - - assert!(sel.is("#nf6")); - assert_eq!(doc.select("#nf6").length(), 0); - assert_eq!(doc.select("#nf5").length(), 1); - s1.append_selection(&sel); - // after appending detached element, it can be matched - assert!(sel.is("#nf6")); -} #[cfg_attr(not(target_arch = "wasm32"), test)] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] @@ -162,14 +145,14 @@ fn test_prepend_html_multiple_elements_to_multiple() { #[cfg_attr(not(target_arch = "wasm32"), test)] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] -fn test_replace_with_selection_multiple() { +fn test_replace_with_selection() { let contents = r#" TEST
-

span-to-replace

-

span-to-replace

+

+

example example @@ -183,4 +166,59 @@ fn test_replace_with_selection_multiple() { sel_dst.replace_with_selection(&sel_src); assert_eq!(doc.select(".source").length(), 4) +} + + +#[cfg_attr(not(target_arch = "wasm32"), test)] +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] +fn test_append_selection_multiple() { + let contents = r#" + + TEST + +
+

+

+
+ example + + "#; + + let doc = Document::from(contents); + + let sel_dst = doc.select(".content p"); + let sel_src = doc.select("span.source"); + + sel_dst.append_selection(&sel_src); + assert_eq!(doc.select(".source").length(), 2) +} + + +#[cfg_attr(not(target_arch = "wasm32"), test)] +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] +fn test_replace_with_another_tree_selection() { + let contents_dst = r#" + + TEST + +
+

+

+
+ + "#; + + let doc_dst = Document::from(contents_dst); + + let contents_src = r#" + example + example"#; + + let doc_src = Document::from(contents_src); + + let sel_dst = doc_dst.select(".content p span"); + let sel_src = doc_src.select("span.source"); + + sel_dst.replace_with_selection(&sel_src); + assert_eq!(doc_dst.select(".source").length(), 4) } \ No newline at end of file From 92fdcb6ecc6f4ded4174125fc4f8562b2f23aeb9 Mon Sep 17 00:00:00 2001 From: Mykola Humanov Date: Tue, 5 Nov 2024 11:29:43 +0200 Subject: [PATCH 4/4] tests/selection-manipulation.rs: adjustments --- CHANGELOG.md | 5 ++ Examples.md | 1 - README.md | 1 - src/dom_tree.rs | 7 ++- src/node/iters.rs | 17 +++++-- src/node/node_ref.rs | 8 ++-- src/selection.rs | 4 +- tests/data.rs | 12 +++++ tests/selection-manipulation.rs | 82 +++++++++++++-------------------- 9 files changed, 72 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98c64c9..c757dbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ All notable changes to the `dom_query` crate will be documented in this file. - Added `NodeRef::prepend_html` method, that parses html string and inserts its parsed nodes at the beginning of the node content. - Added `Selection::prepend_html` method, which parses an HTML string and inserts its parsed nodes at the beginning of the content of all matched nodes. +### Fixed +- Fixed `Selection::append_selection` to work with selections with multiple nodes and selections from another tree. +- Fixed `Selection::replace_with_selection` to work with selections with multiple nodes and selections from another tree. + + ## [0.8.0] - 2024-11-03 ### Changed diff --git a/Examples.md b/Examples.md index 29f2d43..b2608ec 100644 --- a/Examples.md +++ b/Examples.md @@ -560,7 +560,6 @@ assert!(doc.select(r#"#main #second:has-text("test")"#).exists()); main_node.append_html(r#"

Wonderful

"#); assert_eq!(doc.select("#main #third").text().as_ref(), "Wonderful"); -dbg!(doc.html()); // There is also a `prepend_child` and `prepend_html` methods which allows // to insert content to the begging of the node. main_node.prepend_html(r#"

-1

0

"#); diff --git a/README.md b/README.md index 0c85440..e281832 100644 --- a/README.md +++ b/README.md @@ -592,7 +592,6 @@ assert!(doc.select(r#"#main #second:has-text("test")"#).exists()); main_node.append_html(r#"

Wonderful

"#); assert_eq!(doc.select("#main #third").text().as_ref(), "Wonderful"); -dbg!(doc.html()); // There is also a `prepend_child` and `prepend_html` methods which allows // to insert content to the begging of the node. main_node.prepend_html(r#"

-1

0

"#); diff --git a/src/dom_tree.rs b/src/dom_tree.rs index b156d6f..8280875 100644 --- a/src/dom_tree.rs +++ b/src/dom_tree.rs @@ -504,9 +504,12 @@ impl Tree { NodeId::new(self.nodes.borrow().len()) } - /// Adds nodes from another tree to the current tree and + /// Adds nodes from another tree to the current tree and /// then applies a function to the first merged node - pub(crate) fn merge_with_fn(&self, other: Tree, f: F) where F: FnOnce(NodeId) { + pub(crate) fn merge_with_fn(&self, other: Tree, f: F) + where + F: FnOnce(NodeId), + { let new_node_id = self.get_new_id(); self.merge(other); f(new_node_id); diff --git a/src/node/iters.rs b/src/node/iters.rs index 058e9b2..6194c29 100644 --- a/src/node/iters.rs +++ b/src/node/iters.rs @@ -8,7 +8,6 @@ pub struct ChildNodes<'a> { nodes: Ref<'a, Vec>, next_child_id: Option, rev: bool, - } impl<'a> ChildNodes<'a> { @@ -25,13 +24,19 @@ impl<'a> ChildNodes<'a> { pub fn new(nodes: Ref<'a, Vec>, node_id: &NodeId, rev: bool) -> Self { let first_child = nodes .get(node_id.value) - .map(|node| if rev { node.last_child} else {node.first_child}) + .map(|node| { + if rev { + node.last_child + } else { + node.first_child + } + }) .unwrap_or(None); ChildNodes { nodes, next_child_id: first_child, - rev + rev, } } } @@ -43,7 +48,11 @@ impl<'a> Iterator for ChildNodes<'a> { let current_id = self.next_child_id?; if let Some(node) = self.nodes.get(current_id.value) { - self.next_child_id = if self.rev { node.prev_sibling } else { node.next_sibling }; + self.next_child_id = if self.rev { + node.prev_sibling + } else { + node.next_sibling + }; Some(current_id) } else { None diff --git a/src/node/node_ref.rs b/src/node/node_ref.rs index fcce03b..45a4120 100644 --- a/src/node/node_ref.rs +++ b/src/node/node_ref.rs @@ -225,8 +225,8 @@ impl<'a> NodeRef<'a> { where T: Into, { - let fragment = Document::fragment(html); - self.tree.merge_with_fn(fragment.tree, |node_id|{ + let fragment = Document::fragment(html); + self.tree.merge_with_fn(fragment.tree, |node_id| { self.append_prev_siblings(&node_id); }); self.remove_from_parent(); @@ -238,7 +238,7 @@ impl<'a> NodeRef<'a> { T: Into, { let fragment = Document::fragment(html); - self.tree.merge_with_fn(fragment.tree, |node_id|{ + self.tree.merge_with_fn(fragment.tree, |node_id| { self.append_children(&node_id); }); } @@ -249,7 +249,7 @@ impl<'a> NodeRef<'a> { T: Into, { let fragment = Document::fragment(html); - self.tree.merge_with_fn(fragment.tree, |node_id|{ + self.tree.merge_with_fn(fragment.tree, |node_id| { self.prepend_children(&node_id); }); } diff --git a/src/selection.rs b/src/selection.rs index 15539e7..b8f909b 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -332,7 +332,7 @@ impl<'a> Selection<'a> { /// the nodes from the given selection. /// /// This follows the same rules as `append`. - /// + /// pub fn replace_with_selection(&self, sel: &Selection) { //! This is working solution, but it's not optimal yet! //! Note: goquery's behavior is taken as the basis. @@ -396,8 +396,6 @@ impl<'a> Selection<'a> { }); } } - - } // traversing methods diff --git a/tests/data.rs b/tests/data.rs index 3047d95..78c9434 100644 --- a/tests/data.rs +++ b/tests/data.rs @@ -66,6 +66,18 @@ pub static REPLACEMENT_CONTENTS: &str = r#" "#; +pub static REPLACEMENT_SEL_CONTENTS: &str = r#" + + + +
+

+

+
+ example + + "#; + pub static EMPTY_BLOCKS_CONTENTS: &str = r#" diff --git a/tests/selection-manipulation.rs b/tests/selection-manipulation.rs index 0c62874..29ddbd6 100644 --- a/tests/selection-manipulation.rs +++ b/tests/selection-manipulation.rs @@ -1,6 +1,6 @@ mod data; -use data::{doc_with_siblings, EMPTY_BLOCKS_CONTENTS}; +use data::{doc_with_siblings, EMPTY_BLOCKS_CONTENTS, REPLACEMENT_SEL_CONTENTS}; use dom_query::Document; #[cfg(target_arch = "wasm32")] @@ -50,7 +50,6 @@ fn test_set_html_empty() { assert_eq!(doc.select("#main").children().length(), 0); } - #[cfg_attr(not(target_arch = "wasm32"), test)] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn remove_descendant_attributes() { @@ -146,69 +145,32 @@ fn test_prepend_html_multiple_elements_to_multiple() { #[cfg_attr(not(target_arch = "wasm32"), test)] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_replace_with_selection() { - let contents = r#" - - TEST - -
-

-

-
- example - example - - "#; - - let doc = Document::from(contents); + let doc = Document::from(REPLACEMENT_SEL_CONTENTS); - let sel_dst = doc.select(".content p span"); + let sel_dst = doc.select(".ad-content p span"); let sel_src = doc.select("span.source"); sel_dst.replace_with_selection(&sel_src); - assert_eq!(doc.select(".source").length(), 4) + assert_eq!(doc.select(".ad-content .source").length(), 2) } - #[cfg_attr(not(target_arch = "wasm32"), test)] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_append_selection_multiple() { - let contents = r#" - - TEST - -
-

-

-
- example - - "#; + let doc = Document::from(REPLACEMENT_SEL_CONTENTS); - let doc = Document::from(contents); - - let sel_dst = doc.select(".content p"); + let sel_dst = doc.select(".ad-content p"); let sel_src = doc.select("span.source"); sel_dst.append_selection(&sel_src); - assert_eq!(doc.select(".source").length(), 2) + assert_eq!(doc.select(".ad-content .source").length(), 2); + assert_eq!(doc.select(".ad-content span").length(), 4) } - #[cfg_attr(not(target_arch = "wasm32"), test)] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_replace_with_another_tree_selection() { - let contents_dst = r#" - - TEST - -
-

-

-
- - "#; - - let doc_dst = Document::from(contents_dst); + let doc_dst = Document::from(REPLACEMENT_SEL_CONTENTS); let contents_src = r#" example @@ -216,9 +178,29 @@ fn test_replace_with_another_tree_selection() { let doc_src = Document::from(contents_src); - let sel_dst = doc_dst.select(".content p span"); + let sel_dst = doc_dst.select(".ad-content p span"); let sel_src = doc_src.select("span.source"); sel_dst.replace_with_selection(&sel_src); - assert_eq!(doc_dst.select(".source").length(), 4) -} \ No newline at end of file + assert_eq!(doc_dst.select(".ad-content .source").length(), 4) +} + + +#[cfg_attr(not(target_arch = "wasm32"), test)] +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] +fn test_append_tree_selection() { + let doc_dst = Document::from(REPLACEMENT_SEL_CONTENTS); + + let contents_src = r#" + example + example"#; + + let doc_src = Document::from(contents_src); + + let sel_dst = doc_dst.select(".ad-content p"); + let sel_src = doc_src.select("span.source"); + + sel_dst.append_selection(&sel_src); + assert_eq!(doc_dst.select(".ad-content .source").length(), 4); + assert_eq!(doc_dst.select(".ad-content span").length(), 6) +}