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

[V3] Send meaningful responses on conversion failure #1817

Merged
merged 4 commits into from
Jun 9, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 1 addition & 2 deletions redbot/cogs/admin/admin.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
from typing import Tuple

import discord
from discord.ext import commands

from redbot.core import Config, checks
from redbot.core import Config, checks, commands

import logging

Expand Down
2 changes: 1 addition & 1 deletion redbot/cogs/admin/announcer.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import asyncio

import discord
from discord.ext import commands
from redbot.core import commands


class Announcer:
Expand Down
2 changes: 1 addition & 1 deletion redbot/cogs/admin/converters.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import discord
from discord.ext import commands
from redbot.core import commands


class MemberDefaultAuthor(commands.Converter):
Expand Down
4 changes: 2 additions & 2 deletions redbot/cogs/alias/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from .alias import Alias
from discord.ext import commands
from redbot.core.bot import Red


def setup(bot: commands.Bot):
def setup(bot: Red):
bot.add_cog(Alias(bot))
2 changes: 1 addition & 1 deletion redbot/cogs/audio/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from .audio import Audio
from .manager import start_lavalink_server
from discord.ext import commands
from redbot.core import commands
from redbot.core.data_manager import cog_data_path
import redbot.core

Expand Down
3 changes: 1 addition & 2 deletions redbot/cogs/downloader/converters.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import discord
from discord.ext import commands
from .repo_manager import RepoManager
from redbot.core import commands
from .installable import Installable


Expand Down
14 changes: 5 additions & 9 deletions redbot/cogs/downloader/repo_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,13 @@
import functools
import os
import pkgutil
import shutil
Copy link
Member Author

@Tobotimus Tobotimus Jun 7, 2018

Choose a reason for hiding this comment

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

This import was unused

from concurrent.futures import ThreadPoolExecutor
from pathlib import Path
from subprocess import run as sp_run, PIPE
from sys import executable
from typing import Tuple, MutableMapping, Union

from discord.ext import commands

from redbot.core import Config
Copy link
Member Author

Choose a reason for hiding this comment

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

JSYK the Config import here was unused

Copy link
Member

Choose a reason for hiding this comment

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

The commands import is used for type hinting, did you account for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

The commands import is replaced by from redbot.core import commands

from redbot.core import data_manager
from redbot.core import data_manager, commands
from redbot.core.utils import safe_delete
from .errors import *
from .installable import Installable, InstallableType
Expand Down Expand Up @@ -232,7 +228,7 @@ async def current_commit(self, branch: str = None) -> str:
----------
branch : `str`, optional
Override for repo's branch attribute.

Returns
-------
str
Expand Down Expand Up @@ -381,7 +377,7 @@ async def install_libraries(
Directory to install shared libraries to.
libraries : `tuple` of `Installable`
A subset of available libraries.

Returns
-------
bool
Expand All @@ -403,7 +399,7 @@ async def install_libraries(

async def install_requirements(self, cog: Installable, target_dir: Path) -> bool:
"""Install a cog's requirements.

Requirements will be installed via pip directly into
:code:`target_dir`.

Expand Down Expand Up @@ -465,7 +461,7 @@ async def install_raw_requirements(self, requirements: Tuple[str], target_dir: P
@property
def available_cogs(self) -> Tuple[Installable]:
"""`tuple` of `installable` : All available cogs in this Repo.

This excludes hidden or shared packages.
"""
# noinspection PyTypeChecker
Expand Down
2 changes: 1 addition & 1 deletion redbot/cogs/mod/checks.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from discord.ext import commands
from redbot.core import commands
import discord


Expand Down
9 changes: 7 additions & 2 deletions redbot/cogs/permissions/converters.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ async def convert(self, ctx: commands.Context, arg: str) -> Tuple[str]:
if ret:
return "commands", ret.qualified_name

raise commands.BadArgument()
raise commands.BadArgument(
'Cog or command "{arg}" not found. Please note that this is case sensitive.'
"".format(arg=arg)
)


class RuleType(commands.Converter):
Expand All @@ -21,4 +24,6 @@ async def convert(self, ctx: commands.Context, arg: str) -> str:
if arg.lower() in ("deny", "blacklist", "denied"):
return "deny"

raise commands.BadArgument()
raise commands.BadArgument(
'"{arg}" is not a valid rule. Valid rules are "allow" or "deny"'.format(arg=arg)
)
2 changes: 1 addition & 1 deletion redbot/cogs/trivia/trivia.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from collections import Counter
import yaml
import discord
from discord.ext import commands
from redbot.core import commands
from redbot.ext import trivia as ext_trivia
from redbot.core import Config, checks
from redbot.core.data_manager import cog_data_path
Expand Down
2 changes: 1 addition & 1 deletion redbot/core/checks.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import discord
from discord.ext import commands
from redbot.core import commands


async def check_overrides(ctx, *, level):
Expand Down
1 change: 1 addition & 0 deletions redbot/core/commands/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
from discord.ext.commands import *
from .commands import *
from .context import *
from .errors import *
41 changes: 40 additions & 1 deletion redbot/core/commands/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,20 @@
replace those from the `discord.ext.commands` module.
"""
import inspect
from typing import TYPE_CHECKING

from discord.ext import commands

from .errors import ConversionFailure
from ..i18n import Translator

if TYPE_CHECKING:
from .context import Context

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

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


class Command(commands.Command):
"""Command class for Red.
Expand Down Expand Up @@ -53,7 +61,7 @@ def parents(self):
"""
Returns all parent commands of this command.

This is a list, sorted by the length of :attr:`.qualified_name` from highest to lowest.
This is a list, sorted by the length of :attr:`.qualified_name` from highest to lowest.
If the command has no parents, this will be an empty list.
"""
cmd = self.parent
Expand All @@ -63,6 +71,37 @@ def parents(self):
cmd = cmd.parent
return sorted(entries, key=lambda x: len(x.qualified_name), reverse=True)

async def do_conversion(self, ctx: "Context", converter, argument: str):
"""Convert an argument according to its type annotation.

Raises
------
ConversionFailure
If doing the conversion failed.

Returns
-------
Any
The converted argument.

"""
# Let's not worry about all of this junk if it's just a str converter
if converter is str:
return argument

try:
return await super().do_conversion(ctx, converter, argument)
except commands.BadArgument as exc:
raise ConversionFailure(converter, argument, *exc.args) from exc
except ValueError as exc:
# Some common converters need special treatment...
if converter in (int, float):
message = _('"{argument}" is not a number.').format(argument=argument)
raise ConversionFailure(converter, argument, message) from exc

# We should expose anything which might be a bug in the converter
raise exc

def command(self, cls=None, *args, **kwargs):
"""A shortcut decorator that invokes :func:`.command` and adds it to
the internal command list via :meth:`~.GroupMixin.add_command`.
Expand Down
13 changes: 13 additions & 0 deletions redbot/core/commands/errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""Errors module for the commands package."""
from discord.ext import commands

__all__ = ["ConversionFailure"]


class ConversionFailure(commands.BadArgument):
"""Raised when converting an argument fails."""

def __init__(self, converter, argument: str, *args):
self.converter = converter
self.argument = argument
super().__init__(*args)
8 changes: 6 additions & 2 deletions redbot/core/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@


import discord
from discord.ext import commands

from . import __version__
from . import __version__, commands
from .data_manager import storage_type
from .utils.chat_formatting import inline, bordered, pagify, box
from .utils import fuzzy_command_search
Expand Down Expand Up @@ -185,6 +184,11 @@ async def on_error(event_method, *args, **kwargs):
async def on_command_error(ctx, error):
if isinstance(error, commands.MissingRequiredArgument):
await ctx.send_help()
elif isinstance(error, commands.ConversionFailure):
if error.args:
await ctx.send(error.args[0])
else:
await ctx.send_help()
elif isinstance(error, commands.BadArgument):
await ctx.send_help()
elif isinstance(error, commands.DisabledCommand):
Expand Down