hakre
Nested child element parsing
Since Release v0.1.11 there is the \XMLReaderIterator::skipNextRead to fix reading children and continue iteration for parent nodes.
In current project parsing of children is extracted to different objects and it is not easy, to decide, if this method have to call or not. Also this method feels like a workaround and I think, this should be part of the reader itself.
I modified the XMLChildElementIterator.php, that he can break before XMLReader reads the next node, that isn't a child by using the node-type XMLReader::END_ELEMENT
On the example, I also add some different use-cases like empty children or parent is a self-closing-tag.
I recognize, you use a lot of self:: instead of $this (I think for performance-reasons?) . The only way, that only the XMLChildElementInterator can track open/closing-tags and not the XMLReaderIterator, which shouldn't care about "do i leave the container", was to extract reading into a protected method. I hope, you are fine with that.
Note: Provided example requires #20
- [x]
jwundrak-nested-child-element-parsing@hakre - [x] base on develop with #20 (fixes #12) @hakre
- [x] PR rebase
- [x] tests
Thanks for the initiative, I have to take a look. I also have a children related bug-fix I'd like to pull in first, then would review this PR and give feedback.
@jwundrak: I've pushed my changes to the develop branch and also resolved conflicts with your PR within jwundrak-nested-child-element-parsing ~~03b6e8923d0c4d656acec396822abe0c72074f23~~ b1041eb127485e133b5678b1077503db4379a30f .
I could not fully review your changes but I like the idea on first glance, I never considered looking into the element type for that (like closing etc). Also having readNext() as protected to be overridden by child classes. Still would have to chew on it, but I think I start to get the idea.
The self vs. $this issue I also asked myself (I'm not aware of a performance benefit just FYI) and had some changes to that in the earlier fix. In the resolved merge conflict readNext() still exists but is effectively set blank as it is not called any longer as I also changed next() logic in the earlier fix. Please take a look.
My code also has at least one FIXME still which needs to be addressed (but its just return false instead of null, if you stumble over it, just ignore).
You should run the test-suite and provide the fixtures/expectations when developing , e.g. for your example (all examples are automatically tested). Let me know if you need support to get it run on your box. You don't need to run it with all php version we do in the project, but you should run it at least with one php version out of the range 5.3 ... 8.2 master while developing and then CI will tell you some of the php version caveats if any in that range.
The .travis.yml normally gives a good summary on how the build is run (which does the testing). you only need php (with the obvious XML extensions), git and composer (and bash shell) IIRC. Let me know if you run into issues with getting it setup and let me know then as well your operating system.
I changed the PR target against the develop branch please see if the branch mentioned in the beginning is useful to you and feel free to do force pushes for this PR. Unfortunately I didn't look into the XPath PR earlier, I know it has a bug, and this earlier fix of mine only remotely addresses it. The XPath issue is missing tests and I started with this ChildIterator issue first.
Maybe rebasing on develop and a commit with a regression unit-test first, then the fix commit.
@jwundrak: v0.1.12 with the xpath fix (thanks!) and other fixes is out, I updated jwundrak-nested-child-element-parsing accordingly. so this is more in order now.
@hakre Thanks for update and sorry for delayed response.
After the rebase and the changes you made with 0.1.12, my customization are not working as expected. I will fix it and also provide tests, but I'm just busy right now and it might take until the weekend.
@hakre . I resolved the issue and add an unit and update the functional test.
PR is now ready for your review