Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies #289

Closed
wants to merge 8 commits into from
Closed

Conversation

phpfui
Copy link

@phpfui phpfui commented Oct 7, 2021

This PR should solve a lot of the open issues with this library, mostly not compatible with newer libraries that requiring higher versions of what are locked in composer.json.

The string-encoder library will need to be updated first, as I needed to make a private method public. I suspect there is a better way to convert strings, but I wanted just to make the minimal changes.

It also includes three of the outstanding open PRs (#285, #260 and #257) which all looked like reasonable fixes. I would have put these in as separate commits, but I had to redo things, so those got merged in with everything else.

I also ran PHP-CS-Fixer. I could not figure out the array arrow alignment, as they changed that apparently (along with a few other things).

And I was able to fix a few issues pointed out by Phan, but more work is needed there.

@dyaskur
Copy link

dyaskur commented Dec 31, 2021

Since the original creator seemed to have abandoned this project and I can not install it on my project, so I created a fork and published it as a new package to allow me to install it on my project.

composer require 1foru/php-html-parser

hope it helps

@phpfui
Copy link
Author

phpfui commented Dec 31, 2021

I switched over to voku/simple_html_dom a while back.

Glad to see you are maintaining it, but I decided voku/simple_html_dom was a better solution for me.

@dyaskur
Copy link

dyaskur commented Dec 31, 2021

Actually, I just made a fork to just try/test this. Not try to maintain it except for my needs.

I was a simple_html_dom user some years ago. I liked it, but now I just want to try another alternative, so I am trying this. If this is not enough, I should just go back to simple_html_dom.

@phpfui
Copy link
Author

phpfui commented Dec 31, 2021

I was the same, but simple_html_dom is not maintained either from what I see.

I would use voku/simple_html_dom. It is an easy port and seems to be maintained. This package has issues with the string encoder, which is not ideal, not a great design. I think that may be why it is not maintained any longer.

I originally updated it because the dependencies were old, but the upgrade was a lot of work. Turns out the string encoder was poorly designed. I hacked it, but did not want to put in too much work to fix it, as I was not sure of who else used it, and it needed major refactoring.

So after my PR got no response, and my open issue on if the package was maintained still also had no response, I bailed on the project. Turns out I was only using a very small subsection of functionality and the port was easy.

This is one of the issues of open source. At the end of the day, someone needs to maintain things. I am happy to help maintain projects if people use them, but not sure this one is worth it in the end, which is why I switched.

@dyaskur
Copy link

dyaskur commented Dec 31, 2021

So I think I should not waste my time trying this poor library. I will definitely use the voku/simple_html_dom that seem to maintained.

Thanks for your enlightenment

@phpfui
Copy link
Author

phpfui commented Dec 31, 2021

Yeah, porting to a supported library is always the best.

Happy to point you in the right direction. Delete from Packagist if you are not going to maintain it. There is a lot of other forks of this library that are not maintained and should be trashed.

Also thanks for the heads up on the PR. Otherwise you would be stuck with some obsolete code!

Happy New Year if you celebrate!

@phpfui phpfui closed this Dec 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants