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

reddit: handle post and comment links in one regex #2176

Merged
merged 1 commit into from
Sep 11, 2021

Conversation

dgw
Copy link
Member

@dgw dgw commented Aug 18, 2021

Description

It's entirely impractical to try to handle post and comment links in separate callables, because comment links contain an entire post link. Before, this was handled by anchoring the post URL pattern at the end, but that broke Sopel handling e.g. ?sort=confidence if someone sorted the comments before copying the link into IRC.

Lookaround assertions didn't appear to prevent matching the post link contained in every comment link, so this solution seemed like the only way forward. It's a nice tidy dispatcher pattern, really.

Fix #2175.

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

@dgw dgw added the Bugfix Generally, PRs that reference (and fix) one or more issue(s) label Aug 18, 2021
@dgw dgw added this to the 7.1.4 milestone Aug 18, 2021
@dgw dgw requested a review from a team August 18, 2021 08:13
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.

If you say it works, I believe you, since I've no idea what these regex does. 😁

sopel/modules/reddit.py Outdated Show resolved Hide resolved
It's entirely impractical to try to handle post and comment links in
separate callables, because comment links contain an entire post link.

Before, this was handled by anchoring the post URL pattern at the end,
but that broke Sopel handling e.g. `?sort=confidence` if someone sorted
the comments before copying the link into IRC.

Lookaround assertions didn't appear to prevent matching the post link
contained in every comment link, so this solution seemed like the only
way forward. It's a nice tidy dispatcher pattern, really.
@dgw dgw force-pushed the reddit-comment-no-post-info branch from ada10d4 to f869d64 Compare September 10, 2021 18:42
@dgw dgw merged commit cd2df95 into master Sep 11, 2021
@dgw dgw deleted the reddit-comment-no-post-info branch September 11, 2021 05:57
dgw added a commit that referenced this pull request Sep 11, 2021
Backport of PR #2176 from sopel-irc/reddit-comment-no-post-info

Cherry-picked from cd2df95, a merge
commit on the master branch.
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.

reddit: comment links also output submission info
2 participants