Merge pull request #26 from niklak/fix/selection-replace-selection

Fixed `Selection::append_selection` and `Selection::replace_with_selection`

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

Examples.md

@@ -560,7 +560,6 @@ assert!(doc.select(r#"#main #second:has-text("test")"#).exists());
 
 main_node.append_html(r#"<p id="third">Wonderful</p>"#);
 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#"<p id="minus-one">-1</p><p id="zero">0</p>"#);
@@ -612,7 +611,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);
 ```
 </details>

README.md

@@ -592,7 +592,6 @@ assert!(doc.select(r#"#main #second:has-text("test")"#).exists());
 
 main_node.append_html(r#"<p id="third">Wonderful</p>"#);
 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#"<p id="minus-one">-1</p><p id="zero">0</p>"#);
@@ -643,7 +642,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);
 ```
 </details>

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<F>(&self, other: Tree, f: F) where F: FnOnce(NodeId) {
+    pub(crate) fn merge_with_fn<F>(&self, other: Tree, f: F)
+    where
+        F: FnOnce(NodeId),
+    {
         let new_node_id = self.get_new_id();
         self.merge(other);
         f(new_node_id);

src/node/iters.rs

@@ -8,7 +8,6 @@ pub struct ChildNodes<'a> {
     nodes: Ref<'a, Vec<TreeNode>>,
     next_child_id: Option<NodeId>,
     rev: bool,
-
 }
 
 impl<'a> ChildNodes<'a> {
@@ -25,13 +24,19 @@ impl<'a> ChildNodes<'a> {
     pub fn new(nodes: Ref<'a, Vec<TreeNode>>, 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

src/node/node_ref.rs

@@ -225,8 +225,8 @@ impl<'a> NodeRef<'a> {
     where
         T: Into<StrTendril>,
     {
-        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<StrTendril>,
     {
         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<StrTendril>,
     {
         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);
         });
     }

src/selection.rs

@@ -332,16 +332,43 @@ 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.
+
+        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()
     }
 
+    /// 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<T>(&self, html: T)
     where
@@ -369,16 +396,6 @@ 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

tests/data.rs

@@ -66,6 +66,18 @@ pub static REPLACEMENT_CONTENTS: &str = r#"<!DOCTYPE html>
         </body>
     </html>"#;
 
+pub static REPLACEMENT_SEL_CONTENTS: &str = r#"<!DOCTYPE html>
+    <html lang="en">
+        <head></head>
+        <body>
+            <div class="ad-content">
+                <p><span></span></p>
+                <p><span></span></p>
+            </div>
+            <span class="source">example</span>
+        </body>
+    </html>"#;
+
 pub static EMPTY_BLOCKS_CONTENTS: &str = r#"<!DOCTYPE html>
     <html>
         <head></head>

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,24 +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)]
 fn remove_descendant_attributes() {
@@ -159,3 +141,66 @@ 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() {
+    let doc = Document::from(REPLACEMENT_SEL_CONTENTS);
+
+    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(".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 doc = Document::from(REPLACEMENT_SEL_CONTENTS);
+
+    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(".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 doc_dst = Document::from(REPLACEMENT_SEL_CONTENTS);
+
+    let contents_src = r#"
+    <span class="source">example</span>
+    <span class="source">example</span>"#;
+
+    let doc_src = Document::from(contents_src);
+
+    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(".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#"
+    <span class="source">example</span>
+    <span class="source">example</span>"#;
+
+    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)
+}