Skip to content

Conversation

@dgw
Copy link
Member

@dgw dgw commented Feb 7, 2022

Description

Yeah, .xkcd search hasn't been working very well because Bing decided to start returning m.xkcd.com URLs much of the time.

Interestingly, adding -site:m.xkcd.com made Bing return NO results, not the normal (non-mobile) link. That's why this patch modifies the regex to validate the result instead.

I'm not sure what's up with the optional protocol prefix, but didn't want to touch it in a patch intended for stable branch.

Effect

# Before (v7.1.7)
11:20:16 <dgw> .xkcd team chat
11:20:16 <Sopel> dgw: Could not find any comics for that query.

# After (patch head, b706c0e841a35d095c549825ec4a08bc2bf13af5)
11:27:01 <~dgw> ,xkcd team chat
11:27:03 <SopelTest> [xkcd] Team Chat | Alt-text: 2078: He announces that he's finally making the jump from
                     screen+irssi to tmux+weechat. | https://xkcd.com/1782

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

Notes

Back in November I tried to fix this on the search plugin side with a few never-submitted tweaks that didn't actually help reliably. This solution is so stupid-simple… 😅

Along the way I considered trying to make the search more robust by pulling in results from both sopel.modules.search.bing_search() and sopel.modules.search.duck_search(), but that's probably overkill.

Interestingly, adding '-site:m.xkcd.com' made Bing return NO results,
not the normal (non-mobile) link. That's why this patch modifies the
regex to validate the result instead.

I'm not sure what's up with the optional protocol prefix, but didn't
want to touch it in a patch intended for stable branch.
@dgw dgw added the Bugfix Generally, PRs that reference (and fix) one or more issue(s) label Feb 7, 2022
@dgw dgw added this to the 7.1.8 milestone Feb 7, 2022
@dgw dgw requested a review from a team February 7, 2022 17:39
Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean... that's a hotfix, not a rewrite, so yeah, all good.

@dgw dgw merged commit 365cad7 into master Feb 10, 2022
@dgw dgw deleted the xkcd-search-fix branch February 10, 2022 06:14
dgw added a commit that referenced this pull request Feb 10, 2022
xkcd: account for Bing search returning mobile URLs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugfix Generally, PRs that reference (and fix) one or more issue(s)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants