search

oddluck / limnoria-plugins

Limnoria plugins I wrote or forked.
Do What The F*ck You Want To Public License
39 stars 17 forks source link

SpiffyTitles: Fix bad links #47

Closed cottongin closed 1 year ago

cottongin commented 2 years ago

Some further refactoring likely needs to take place here; see something like this from base Limnoria's Web implementation

fred0r commented 2 years ago

using this since a few days and. . . well . .. it doesnt create any errors and everything works like before :D

fred0r commented 2 years ago

well .. . . it does create errors compared to the current version - testurl: https://www.bbc.com/news/world-europe-61327638

fred0r commented 2 years ago

thx - the bbc example-link now works again. maybe you could also take a look into the announcement of msn links? example url: https://www.msn.com/en-ca/entertainment/entertainmenttopstories/heard-takes-stand-accuses-depp-of-violent-sexual-assault/ar-AAWXiBH?li=AAggNb9 but these msn-links also dont work with the unmodified variant.

fred0r commented 2 years ago

https://www.reuters.com/article/us-health-coronavirus-romania-posters-idUSKBN22B1YV works on orig but not with your variant.

oddluck commented 2 years ago

Sorry to ignore this for so long. Looks good but I'm going to run some tests... Ok, so tests run and I have to agree with @fred0r here and say this branch breaks more results than it fixes (which doesn't seem to be any? though I agree bad link handling is slightly broken)... I'll poke around at the code and figure out why that is...

fred0r commented 2 years ago

if i remember correct this url worked with this pull request, but not with the default: https://www.amazon.de/DenTek-Zahnseide-Sticks-Complete-Clean-St%C3%BCck/dp/B07JDM9G65

fred0r commented 2 years ago

my last 2 comments now work - maybe by updated 'requirements'.

maybe you could also take a look into the announcement of msn links? example url: https://www.msn.com/en-ca/entertainment/entertainmenttopstories/heard-takes-stand-accuses-depp-of-violent-sexual-assault/ar-AAWXiBH?li=AAggNb9

-> no announcement