module: add action_commands decorator

[deathbybandaid]

As per my idea in Issue #1658, this is my attempt to integrate an action_commands command structure to Sopel.

There are still things to work out here, I'm sure. I copied elements I saw in intents, commands, and nickname_commands. I am very well aware the docstrings will definitely need work, as well as tests added.

I'm also not happy with the import line adjusted, and will likely fix that with () later.

This is more/less a draft PR, but my brief tests were very successful.

[deathbybandaid]

Test file used:

# coding=utf-8

from __future__ import unicode_literals, absolute_import, division, print_function

import sopel.module


@sopel.module.commands('dbbtest')
def bot_command_dbbtest(bot, trigger):
    bot.say(str(trigger))
    bot.say(str(trigger.args))



@sopel.module.action_commands('rolls')
def bot_command_dbbtestb(bot, trigger):
    bot.say(str(trigger))
    bot.say(str(trigger.args))

Output:

<deathbybandaid> .dbbtest this out
<dbbbot> .dbbtest this out
<dbbbot> ['#deathbybandaid', '.dbbtest this out']


* deathbybandaid rolls dice
<dbbbot> rolls dice
<dbbbot> ['#deathbybandaid', 'rolls dice']

[dgw]

Looking at this led me to the discovery that inspired #1683. Specifically, the implementation here made me wonder "what other intents are there?", which led to finding out that the intent spec no longer exists. This decorator is a pretty good idea, actually, especially because we'll want to replace the internal references to "intents" on callables and triggers with something else that isn't obsolete. Could be as simple as trigger.is_action and a callable.on_actions Boolean (default False). Further discussion and ideas very much welcome in #1683!

[deathbybandaid]

Forced Pushed to rebase with current master.

I could use some help writing the tests for test_module.py.

A thorough review would also be nice to clean/tidy it up. However, I feel fairly positive about what is accomplished here.

As discussed via IRC, this can be implemented, with the current intents code, with the intention (not a pun) of other PRs being opened later, such as:

• Re-evaluating how CTCP is handled
• Based on ^ how we want to handle something like trigger.is_action
• Deprecating @intents, maybe in favor of a replacement @ctcp, since we may still want to allow @rule pattern matching.
• Removal of the ability to run @commands via a CTCP ACTION

[dgw]

Right, so it looks like this should do what is expected/desired. We do have to think about future-proofing and decorator interactions, though.

For example, if a function has both @commands and @nickname_commands, they'll both work, because normal PRIVMSGs are acceptable for both of those. Mix @action_commands in, though, and the other decorator types stop working as expected because of the added ACTION intent. This should, at minimum, be explicitly called out in the documentation. Ideally we'd make it so all of the command types can be mixed on a single callable, but I think that would require a rework of some internals. Definitely out of scope for this PR, but worth keeping in mind as we continue refining 7.x.

Thing is, having @action_commands is definitely more useful than not having it, even if there are some potential conflicts with the other decorators. I can personally streamline, at a quick estimate, three or four of my own plugins using this new feature. If my crappy old code from four years ago can use this—so can the Sopel ecosystem at large.

PS: The rebase introduced some whitespace oddities, most (all?) of which I marked with line notes.

[deathbybandaid]

That was really odd with the whitespace disappearing from my push. Looked normal in my editor.

Added all the requested changes though.

[dgw]

Hmm, that force-push undid all the changes. You should try rebasing/squashing again from 44cd95d.

[deathbybandaid]

@dgw, something is screwy on my end, and everytime I squash, those whitespaces change.
I'll look at it more when I get the chance.

[deathbybandaid]

I deleted the cloned directory, recloned, squashed, and it looks like the problem went away.

Not sure what was happening before, but it looks like everything is there.

[dgw]

If I'm touching tests, I like @Exirel to look at it. 😁

[Exirel]

Yeah, the tests are good @dgw

[dgw]

Had to rebase on master for that last tweak (getting logging & docstrings squared away in bot.py). I didn't want to rebase, but the alternative was making a separate PR, and that seemed silly.

[dgw]

And rebased again due to conflicting changes in #1708. 😦