mmpy_bot icon indicating copy to clipboard operation
mmpy_bot copied to clipboard

Implement PluginManager (3 of 3)

Open unode opened this issue 3 years ago • 12 comments

Reopened #212 targeting main instead of master.

unode avatar May 30 '21 17:05 unode

Some significant changes from #212 - Tests are not yet in sync. More on that in the next push:

  • Plugin.help and Plugin.get_help_string is now HelpPlugin.help and HelpPlugin.get_help_string.
  • The logic to collect help information about plugins is now PluginManager.get_help.
  • HelpPlugin is enabled by default but needs to be explicitly enabled if you specify your own plugins.
  • PluginManager is now available to Plugin instances as self.manager initialized via Plugin.initialize
  • help_trigger and help_trigger_bang are no more. There's only settings.RESPOND_CHANNEL_HELP that defaults to False and defines if the bot should respond to !help in any channel.
  • Arbitrary information can be passed via listen_to and listen_webhook as kwargs and made available via PluginHelpInfo.annotations. (e.g. @listen_to(..., category="webhook", syntax="Command Arg1 Arg2")
    • Two attributes are currently recognized by HelpPlugin.get_help_string:
      • category - grouping functions with the same category label (see screenshot below)
      • syntax - providing an alternative to displaying the regex pattern. Instead of ^reply at (.*)$ you can get reply at TIMESTAMP or any arbitrary text.
  • All help information is now set via function/class docstrings instead of get_help_string. The first line of the docstring will be displayed in the help output. The entire docstring is available as PluginHelpInfo.function_docfull. Plugin level docstrings are also available, see plugin_docfull and plugin_docheader
  • get_help_string is no longer part of the base ABC Plugin class nor Function subclasses.
  • Docstrings were added to the ExamplePlugin where missing.

The current help output with plugins HelpPlugin, ExamplePlugin and WebHookExample looks like: screenshot_2021-05-31_03-27-06_647421429

unode avatar May 31 '21 01:05 unode

I think this is ready for a final of review.

unode avatar Jun 13 '21 17:06 unode

This PR will also close #165

unode avatar Jun 13 '21 19:06 unode

Again @unode thanks for putting in all this work. I have a quick request, can you elaborate on what you mean by the bullet below and what exactly does it mean for users who use their own plugins:

HelpPlugin is enabled by default but needs to be explicitly enabled if you specify your own plugins.

attzonko avatar Jun 14 '21 21:06 attzonko

Again @unode thanks for putting in all this work. I have a quick request, can you elaborate on what you mean by the bullet below and what exactly does it mean for users who use their own plugins:

HelpPlugin is enabled by default but needs to be explicitly enabled if you specify your own plugins.

I mean this https://github.com/attzonko/mmpy_bot/blob/a584bf242673f7355b25b32f6d0afe0d33b76fb7/mmpy_bot/bot.py#L37

If you run the bot without adding any custom plugins, it will be included among the default plugins. If add custom plugins (the most likely case) you should include it, or a subclass of it, in the list of plugins to enable.

Initially I thought of having it always enabled (i.e. plugins + [HelpPlugin()]) but that would remove the ability to define or customize your own help plugin.

unode avatar Jun 15 '21 12:06 unode

Pulled #233 out of this one. That's a pretty self contained feature.

unode avatar Jun 20 '21 22:06 unode

After trying to customize a little the help info I noticed the current approach actually forces you to modify PluginManager.get_help if you want to filter out some functions and HelpPlugin.get_help_string if you want to customize the format of the help.

To simplify the use-case of filtering listeners, I now introduced a HelpPlugin.get_help method. It currently only calls to PluginManager.get_help but provides a location for users to override what is included without having to redefine HelpPlugin.get_help_string entirely. See also https://github.com/attzonko/mmpy_bot/pull/226/files#diff-6cc66ec79b3b320572cf875b2e985afab611933a46d625218fc030c3a000131dR188 as an example of such use-case.

unode avatar Jun 25 '21 13:06 unode

Round two sweat_smile These reviews are taking me a long time, so I would be grateful if you could either make PRs into this branch and/or not force-push to it, so I can look only at the latest changes

Sorry, I will stop pulling things in/out until the review is complete.

unode avatar Jul 05 '21 09:07 unode

Thanks @jneeven!

I'll give it another try at pulling out different PRs. Will likely go with your suggestion of making the PRs against the previous.

unode avatar Aug 16 '21 10:08 unode

Im eagerly awaiting this PR! Currently have a .help command to show what commands are available and what they do

aconitumnapellus avatar Aug 30 '21 12:08 aconitumnapellus

See #323 and #324 for an attempt to split this PR. The current history will be rebased on the other two PRs once merged.

unode avatar Jul 15 '22 21:07 unode

Code Climate has analyzed commit ef7180a6 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3

View more on Code Climate.

codeclimate[bot] avatar Jul 15 '22 21:07 codeclimate[bot]