core: URL Callbacks new interface

[Exirel]

This new interface adds:

• bot.register_url_callback(regex, callback), to register a callback to call when a URL matches regex
• bot.unregister_url_callback(regex) to remove this regex from triggering callbacks
• bot.search_url_callbacks(url) to get a list of (callback, match) for each pattern matched by the URL

This provides a unified interface to work with URL Callbacks, but does not change how the built-in url plugin works (yet).

So this:

if not bot.memory.contains('url_callbacks'):
    bot.memory['url_callbacks'] = tools.SopelMemory()
bot.memory['url_callbacks'][instagram_pattern] = instaparse

Become this:

bot.register_url_callback(instagram_pattern, instaparse)

It does not change the fact that the url built-in plugin is required at the moment in order to work with URL Callback (see #1489 for that).

@dgw now my question is, should I keep moving forward, and replace the url built-in plugin? Or should we merge this PR as-is, and work from there in another PR?

[dgw]

You can forge on ahead with a new PR branch based on this one, like you've done before, unless you need to discuss direction (in #1489) first. That way, no matter how much bikeshedding happens with replacing the url module, we'll have a clean new interface (this PR) in the bag.

I like that this really is just adding convenience methods to do what plugins already do, so nothing will break (yet) if module code isn't updated. 😁

Do you think it's worth checking in the register/unregister functions added here whether the pattern is actually compiled? I can picture users who merely skimmed the documentation (or coders drunk on lack of sleep XD) trying to pass in an uncompiled regex string…

[dgw]

"Request changes" because it looks like some of what you intended to do in the docstrings is broken (see line notes).

While you're looking at those, maybe also think about (as I said in a "real" comment earlier tonight) whether validating the pattern is worthwhile. I just expect that Bad Things™ might happen if some enterprising coder passes in a string instead of a compiled re object—even if said Bad Things™ are just callbacks that don't trigger.

[Exirel]

I like that this really is just adding convenience methods to do what plugins already do, so nothing will break (yet) if module code isn't updated. 😁

I knew you would understand. 👍

Do you think it's worth checking in the register/unregister functions added here whether the pattern is actually compiled?

Yeah, I was wondering too. It's not that complicated, I'll do that.

[Exirel]

Also: of course I failed at docstring again. x)

[Exirel]

Hm. I'm not sure "re-request a review" was the right idea. Meh, I guess I need to learn what the new github has to offer with all these features.

Anyway @dgw I fixed the docstrings and added the safety measures to check if pattern is a compiled regex.

[Exirel]

Ahem. Checkstyle?

Edit: also tests are broken because of course they are. Mock objects are the worst. :(

[dgw]

Yay, I'm not the only one who gets bitten by checkstyle after pushing! 😁

It's nitpick-docstrings-o'clock again, lol.

[dgw]

Approved, with the caveat that the additions to test_tools.MockSopel will be temporary until we find a way to run tests on the "real" Sopel class instead of a mocked-up version.

[dgw]

Merging so @Exirel can properly get started on a PR to fix the mess that is url.py. 🖖