fix(api): Fix `insert` and `remove` behavior for `KdlNode`

src/node.rs

@@ -227,9 +227,6 @@ impl KdlNode {
                             return Some(entry);
                         }
                         current_idx += 1;
-                        if current_idx > idx + 1 {
-                            return None;
-                        }
                     }
                 }
                 None
@@ -266,13 +263,10 @@ impl KdlNode {
                 let mut current_idx = 0;
                 for entry in &mut self.entries {
                     if entry.name.is_none() {
-                        if current_idx >= idx {
+                        if current_idx == idx {
                             return Some(entry);
                         }
                         current_idx += 1;
-                        if current_idx >= idx {
-                            return None;
-                        }
                     }
                 }
                 None
@@ -326,9 +320,6 @@ impl KdlNode {
                                 return Some(entry);
                             }
                             current_idx += 1;
-                            if current_idx >= idx {
-                                break;
-                            }
                         }
                     }
                     if idx > current_idx {
@@ -357,7 +348,7 @@ impl KdlNode {
     fn remove_impl(&mut self, key: NodeKey) -> Option<KdlEntry> {
         match key {
             NodeKey::Key(key) => {
-                for (idx, entry) in self.entries.iter_mut().enumerate() {
+                for (idx, entry) in self.entries.iter().enumerate() {
                     if entry.name.is_some() && entry.name.as_ref() == Some(&key) {
                         return Some(self.entries.remove(idx));
                     }
@@ -366,15 +357,12 @@ impl KdlNode {
             }
             NodeKey::Index(idx) => {
                 let mut current_idx = 0;
-                for entry in &mut self.entries {
+                for (idx_entry, entry) in self.entries.iter().enumerate() {
                     if entry.name.is_none() {
                         if current_idx == idx {
-                            return Some(self.entries.remove(idx));
+                            return Some(self.entries.remove(idx_entry));
                         }
                         current_idx += 1;
-                        if current_idx >= idx {
-                            return None;
-                        }
                     }
                 }
                 None
@@ -680,4 +668,44 @@ mod test {
         node.entries_mut().push(KdlEntry::new_prop("x", 2));
         assert_eq!(&node["x"], &2.into())
     }
+
+    #[test]
+    fn insertion() {
+        let mut node = KdlNode::new("foo");
+        node.push("pos0");
+        node.insert("keyword", 6.0);
+        node.push("pos1");
+        assert_eq!(node.entries().len(), 3);
+
+        node.insert(0, "inserted0");
+        node.insert(1, "inserted1");
+        assert_eq!(node.entries().len(), 3);
+        assert_eq!(node[0], "inserted0".into());
+        assert_eq!(node[1], "inserted1".into());
+    }
+
+    #[test]
+    fn removal() {
+        let mut node = KdlNode::new("foo");
+        node.push("pos0");
+        node.insert("keyword", 6.0);
+        node.push("pos1");
+        assert_eq!(node.entries().len(), 3);
+
+        node.remove(1);
+        assert_eq!(node.entries().len(), 2, "index removal should succeed");
+        assert!(
+            node.get("keyword").is_some(),
+            "keyword property should not be removed by index removal"
+        );
+        node.remove(1);
+        assert_eq!(node.entries().len(), 2, "index removal should not succeed");
+        node.remove("not an existing keyword");
+        assert_eq!(node.entries().len(), 2, "key removal should not succeed");
+        node.remove("keyword");
+        assert_eq!(node.entries().len(), 1, "key removal should succeed");
+        node.remove(0);
+        assert_eq!(node.entries().len(), 0, "index removal should suceed");
+        node.remove(0); // should not panic
+    }
 }