Document traverse works only once

[rauanmayemir]

I was trying to traverse the document tree and delete empty nodes, but it only works once. I.e it does traverse the document, finds all the node elements I need, but deletes only the first one it sees, discarding everything else.

I've checked the unit tests for Action\RemoveNode, and it only tests the deletion of the first attribute.

[veewee]

Hello,

Thanks for reporting. Would you mind adding a code example?
It is not completely clear what you are trying to do, how you do it, what is going wrong exactly, what you expect, ...

[rauanmayemir]

I've already deleted the traversal and went with xpath query and calling remove directly there on the node list:

$doc = Document::fromXmlString($xml);
$doc
    ->xpath()
    ->query('*//*[not(node())]')
    ->forEach(fn(DOMNode $node) => remove($node));

But I can see that there's no test for deep nested traversal and/or remove node action: https://github.com/veewee/xml/blob/main/tests/Xml/Dom/Traverser/TraverserTest.php.

Simply adjusting the node's attributes, tag or contents works fine as it's just traversing and mutating the objects, but returning remove node action will only work once.

What I was trying to do is remove all empty nodes in the xml like <nodeTag xsi:nil="true"/> or even just <nodeTag />.

[veewee]

I'm still confused to be honest.
Wouldn't you be able to perform the removal at onNodeLeave ?
That way you delete from the deepest level back to the -> root
We do something similar whilst removing namespaces here: https://github.com/veewee/xml/blob/main/src/Xml/Dom/Traverser/Visitor/RemoveNamespaces.php

The code you pasted is not recursive either.
We've implemented this in another package as well:
https://github.com/php-soap/psr18-transport/blob/main/src/Middleware/RemoveEmptyNodesMiddleware.php

As you can see, the xpath should go in a loop.
Yet it would be better if you'dd remove with the traverser instead.

[rauanmayemir]

The code you pasted is not recursive either.

Yes, but via xpath I'm just pulling all the empty nodes and removing them. It actually worked for me. 🙈

We've implemented this in another package as well:

Thank god, I was looking for this in the wrong place.

We do something similar whilst removing namespaces here:

Yes, this is mutating the node sort of in-place. But if you were to replace it with return new Action\RemoveNode(), it wouldn't work as expected.

[veewee]

@rauanmayemir Thanks for reporting! I found the issue in #50 !
Can you check if that works for you? I've added a visitor as well, so that you dont have to rewrite the code.
The problem was quite tricky :)

[rauanmayemir]

Can confirm now it works as expected. 💃

I would still rather use RemoveEmptyNodesMiddleware as a preflight clean up, but I'm glad I can finally use this package for safe general XML manipulation. (I'm already using the writer for smooth xml generation, but had trouble starting with the traversal, locators and manipulation - they need more examples. 😁 )

[veewee]

Nice to hear!
I can imagine it takes some getting-to-know-the-package, but feel free to provide better examples in the documentation :)