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

[Commands] Refactor command and group decorators #1818

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions redbot/core/bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

import discord
import sys
from discord.ext.commands.bot import BotBase
from discord.ext.commands import GroupMixin
from discord.ext.commands import when_mentioned_or

# This supresses the PyNaCl warning that isn't relevant here
Expand All @@ -24,7 +22,7 @@
from .sentry import SentryManager


class RedBase(BotBase, RPCMixin):
class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why call our own reference to BotBase? I don't see a whole lot of sense in doing this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can change it to discord.ext.commands.bot.BotBase if you'd like, it wouldn't make a difference

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could instead subclass discord.ext.commands.bot.BotBase and add the GroupMixin there. It wouldn't be any different to the current setup but would simplify RedBase's direct inheritance

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See here for an example of what I mean, I don't really think its necessary but all of this inheritance stuff is pretty confusing to some, perhaps this makes it a bit simpler? I'm not sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this is probably a better way to show the example

"""Mixin for the main bot class.

This exists because `Red` inherits from `discord.AutoShardedClient`, which
Expand Down Expand Up @@ -254,7 +252,7 @@ def unload_extension(self, name):
# first remove all the commands from the module
for cmd in self.all_commands.copy().values():
if cmd.module.startswith(lib_name):
if isinstance(cmd, GroupMixin):
if isinstance(cmd, discord.ext.commands.GroupMixin):
cmd.recursively_remove_all_commands()
self.remove_command(cmd.name)

Expand Down
25 changes: 15 additions & 10 deletions redbot/core/commands/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
if TYPE_CHECKING:
from .context import Context

__all__ = ["Command", "Group", "command", "group"]
__all__ = ["Command", "GroupMixin", "Group", "command", "group"]

_ = Translator("commands.commands", __file__)

Expand Down Expand Up @@ -102,11 +102,17 @@ async def do_conversion(self, ctx: "Context", converter, argument: str):
# We should expose anything which might be a bug in the converter
raise exc

def command(self, cls=None, *args, **kwargs):

class GroupMixin(commands.GroupMixin):
"""Mixin for `Group` and `Red` classes.

This class inherits from :class:`discord.ext.commands.GroupMixin`.
"""

def command(self, *args, **kwargs):
"""A shortcut decorator that invokes :func:`.command` and adds it to
the internal command list via :meth:`~.GroupMixin.add_command`.
the internal command list.
"""
cls = cls or self.__class__

def decorator(func):
result = command(*args, **kwargs)(func)
Expand All @@ -115,11 +121,10 @@ def decorator(func):

return decorator

def group(self, cls=None, *args, **kwargs):
def group(self, *args, **kwargs):
"""A shortcut decorator that invokes :func:`.group` and adds it to
the internal command list via :meth:`~.GroupMixin.add_command`.
the internal command list.
"""
cls = None or Group

def decorator(func):
result = group(*args, **kwargs)(func)
Expand All @@ -129,11 +134,11 @@ def decorator(func):
return decorator


class Group(Command, commands.Group):
class Group(Command, GroupMixin, commands.Group):
"""Group command class for Red.

This class inherits from `discord.ext.commands.Group`, with `Command` mixed
in.
This class inherits from `Command`, with :class:`GroupMixin` and
`discord.ext.commands.Group` mixed in.
"""

def __init__(self, *args, **kwargs):
Expand Down
34 changes: 34 additions & 0 deletions tests/core/test_commands.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import pytest
from redbot.core import commands


@pytest.fixture(scope="session")
def group():
@commands.group()
async def fixturegroup(*args, **kwargs):
return args, kwargs

return fixturegroup


def is_Command(obj):
return isinstance(obj, commands.Command)


def is_Group(obj):
return isinstance(obj, commands.Group)


def test_command_decorators(coroutine):
assert is_Command(commands.command(name="cmd")(coroutine))
assert is_Group(commands.group(name="grp")(coroutine))


def test_group_decorator_methods(group, coroutine):
assert is_Command(group.command(name="cmd")(coroutine))
assert is_Group(group.group(name="grp")(coroutine))


def test_bot_decorator_methods(red, coroutine):
assert is_Command(red.command(name="cmd")(coroutine))
assert is_Group(red.group(name="grp")(coroutine))