loader: default rule for events #1693
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If a callable has an event set, but no rule, the bot will add a default
'.*' (catch-all) rule when the callable is registered.
This is a developer-friendliness change. Previously, an event-based
trigger also required a catch-all rule decorator ('.*') (but this was
not very well documented).
There should be no behavior change for event handlers that also have
explicit rule decorator(s).
Thanks to @Exirel for nudging me in the right direction when my initial
approach broke everything. :P
Co-Authored-By: Exirel <florian.strzelecki@gmail.com>
Now that events have a default catch-all rule, we don't need to repeatedly decorate stuff with '.*' everywhere. We can remove almost 20 rule decorators, mostly in coretasks' low-level protocol logic, and it cost us +/- 0 lines (excluding pep8 tweaks).
dgw
force-pushed
the
event-default-rule
branch
from
c112c1c
to
2d11bf1
Compare
Exirel
approved these changes
Sep 17, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Helped @deathbybandaid with
@eventdecorators not working and realized that this specific problem comes up a lot, relatively speaking, when Sopel users have trouble writing custom plugins.Obviously our documentation doesn't do a great job of making sure plugin devs know that
@ruleis required along with@event. But rather than fix the documentation, I decided to do the user-friendly (developer-friendly?) thing and make it soeventis considered triggerable just like a command or rule, and so it will also get the default.*rule applied if necessary once registered.An early version of this patch (pre-publication) also applied this default rule to intents, but:
'ACTION'), which almost always requires an explicit rule anyway.@intentspretty soon because it's obsolete (Message intents are dead #1683).Other rejected ideas were applying this to
@url(not needed) and to@echo(immediately determined to be the exact opposite of intuitive behavior once I gave it ten seconds of thought).So here we are: A ±0 line patch (if we ignore tests & code formatting) to the loader that saves us almost 20 real-code lines of boilerplate. Not bad for a bit of afternoon hacking, if I do say so myself.