-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[V3] Send meaningful responses on conversion failure #1817
Conversation
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
@@ -2,17 +2,13 @@ | |||
import functools | |||
import os | |||
import pkgutil | |||
import shutil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import was unused
With this change, the bot can send a more meaningful message when converting an argument fails. Converters can do this by raising
commands.BadArgument
with its first argument being the message they would like to send to the user.If a message isn't passed into the exception, the bot will simply send the help message. If the exception raised from the converter doesn't inherit from
commands.BadArgument
, the command will error out as usual, so we don't suppress any bugs.Also note that this message will only be sent if the command is an instance of
redbot.core.commands.Command
.P.S. sorry for so many changed files, but I went through and updated existing converters and also made sure all
discord.ext.commands
imports are actually nowredbot.core.commands
, since this was sometimes causing issues and inconsistencies with this change.Resolves #967.