Many thanks for your feedback! > Your test suite looks like it could maybe use a mixture of inserts & deletes > in either order while iterating
The inserts are "missing" because they were not relevant for my use case and I hadn't thought a lot about them. As far as I did think about insertions, I wasn't sure how well that would work or behave as you'd intuitively would expect. Since the algorithm "detects" deletions via a change of the child nodes in the parent node, it won't notice when you delete a node and insert another instead. So I didn't spend much more time on that because I thought it would be too brittle anyway. Now that I think more about it, with the current implementation, I'd expect this behavior: * If you delete a node and insert another node instead, the iterator should go over the child nodes of the new node. I think that's actually reasonable. * If you insert a node after the one just yielded, the iterator should still yield the child nodes of the last yielded node and _then_ yield the newly inserted node and its children. Also not too bad. My problem right now is, although I think the behavior is reasonable, I don't know if a user would also find it reasonable. Also, obviously not everything is possible. For example, if the iterator is already yielding child nodes of a node and you delete the parent node (because you held on to the yielded value of an earlier iteration) this won't stop the iteration over the child nodes. I guess documenting the behavior of the iterator _might_ be more difficult than the implementation. > in either order while iterating At the moment only iterating forward is supported. Or did you mean something else, like inserting a node before or after the just-yielded node? > It may not be so appropriate to your "tree structure surgery while iterating" > use case, but I happened to do Yet Another Idea just the other day over at > [https://forum.nim-lang.org/t/5506](https://forum.nim-lang.org/t/5506) which > bypasses the Nim iterator syntax framework for a Nim template that accepts > multiple body clauses. At first I wasn't sure whether I understood it, but after a more thorough look at the example in the other thread, I think I understand what you're doing there. :-) On the one hand it's indeed interesting, on the other hand it feels kind of "heavy". I don't know how to describe it better. Maybe it would help if you could replace the second and third `do` with `pre_recurse` and `post_recurse`. Apart from iterators, I also like your callback idea from a messages above. This should be both flexible and relatively easy to implement with the tools we already have (like iterating over sibling notes with `node[childIndex]`). That said, iterators feel more lightweight and intuitive (as long as they're applicable to the problem.) Actually, I experimented with the callback idea yesterday to fix my special problem in the `vppdiff` tool, although I didn't use a callback but defined a recursive `proc` with the XML clean-up code. While doing that I noticed that `node.items()` wasn't a good fit for this because it takes the `len` of the parent node before it even starts iterating and when you delete child nodes during the iteration that leads to `IndexError` s. So while `items` is an iterator and you'd intuitively think it's lazy, it's only "partially lazy" in that it doesn't consider changes in the number of children. Now, you _usually_ should follow the advice not to modify a structure while iterating over it, but I still wonder if `XmlNode.items` should be changed to use a `while` loop instead fixing the upper boundary for the index at the start of the iteration.
