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

xkcd: account for Bing search returning mobile URLs #2247

Merged
merged 1 commit into from
Feb 10, 2022
Merged

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.

2 participants