phpcr / phpcr-shell

PHPCR Shell
MIT License
68 stars 19 forks source link

Command node:list (ls): Fix wrong node counter #188

Closed middlebrain closed 6 years ago

middlebrain commented 6 years ago

The displayed node counter for the 'ls' command is always set to 1.

dantleech commented 6 years ago

Thanks for the PR, I'm not really in a position to test these changes at the moment as I don't do any development with PHPCR currently and I can't get Jackrabbit working locally for whatever reason. So I would trust you that this change works.

The tests are failing apparently due to unrelated changes in other PHPCR libraries. Would be good to update the test expectations and make the build green before merging this.

dbu commented 6 years ago

can you rebase this on master please?

is it possible to write a test for this that could make sure the edge cases are now handled correctly? i notice that the counter is not right in the foreach($nodes as $node) line, so i guess there are lines we do not want to count...

middlebrain commented 6 years ago

I have reworked the fix a little bit and also added two tests. Node counting works now in the same way as property counting in the PHPCR/Shell/Console/Command/Phpcr/NodeListCommand::renderProperties function.

dantleech commented 6 years ago

Nice work :)