search

ash-jc-allen / favicon-fetcher

A Laravel package for fetching favicons from websites.
MIT License
167 stars 13 forks source link

Fixes for parsing malformed link #54

Closed MatthewPageUK closed 1 year ago

MatthewPageUK commented 1 year ago

Had a go at a quick tidy for the parsing issue. Didn't have a chance to setup the app and test it. Regexs are tested and should be ok.

ash-jc-allen commented 1 year ago

Hey @MatthewPageUK! Huge thank you for this PR, I really appreciate you taking the time to do it.

I think for the time being I'm probably gonna close it, sorry. I've been tinkering with moving over to using symfony/dom-crawler to do the HTML parsing rather than doing it ourselves. I feel like it'll take off some of the pressure from me needing to maintain any regex or patterns (which are both things that I'm not great at haha!).

I've got a PR open with my proposed changes (https://github.com/ash-jc-allen/favicon-fetcher/pull/56).

If that doesn't work out though, I'll probably use the approach that you've proposed here.

Huge thanks again 😄

MatthewPageUK commented 1 year ago

No worries @ash-jc-allen