Escape nicknames in patrick responses. - #67
JoBeGaming wants to merge 12 commits into
Conversation
…me is used in a reply or log message.
Nickster258
left a comment
Nickster258
left a comment
There was a problem hiding this comment.
Probably shouldn't escape the logged statements. At the same time, would be worth logging the user ID in the event message.
|
67 |
|
Weird how my commit from VSC got added, but the reply didn't get send... |
|
Ok seems like I've forgotten to add cases such as |
assert escape_nickname("> job") == r"\> job"
assert escape_nickname("*job*") == r"\*job\*"
assert escape_nickname("**job**") == r"\*\*job\*\*"
assert escape_nickname("_job_") == r"\_job\_"
assert escape_nickname("__job__") == r"\_\_job\_\_"
assert escape_nickname("`job`") == r"\`job\`"
assert escape_nickname("> job") == r"\> job"
assert escape_nickname("~~job~~") == r"\~\~job\~\~"
assert escape_nickname("||job||") == r"\|\|job\|\|"
assert escape_nickname("# job") == r"\# job"
assert escape_nickname("- job") == r"\- job"
assert escape_nickname("-# job") == r"\-\# job"(Updated test-cases if anyone is interested) |
…nickname` doesn't undo any work.
| if matches: | ||
| logger.info( | ||
| f"Automod triggered for user {message.author.display_name} with message {message.content}" | ||
| f"Automod triggered for user {user_log_repr(message.author)} with message {message.content}" |
There was a problem hiding this comment.
Minor question. Most other log messages had ' used as quotes around the user part. Should we keep that here too? Although i feel like 'Name' (1234) would be the best option instead of 'Name (1234)', meaning @Wueffi was wrong and I should probably have user_log_repr return f"'{user.display_name}' ({user.id})" instead.
| if message.content.removeprefix(prefix) in commands: | ||
| bot.logger.info( | ||
| f"User '{message.author.display_name}' ran custom command '{message.content[1:]}'" | ||
| f"User {user_log_repr(message.author)} ran custom command '{message.content[1:]}'" |
There was a problem hiding this comment.
Also user_log_repr itself should be wrapped in ', I should probably add that here too.
| id, so log messages are the same as in chattore. | ||
| """ | ||
|
|
||
| return f"{user.display_name} ({user.id})" |
There was a problem hiding this comment.
I agree that it may be best to insert the ' here for us to keep a consistent scheme
There was a problem hiding this comment.
Ok so i should merge JoBeGaming#1 into the PR branch?
| # This is provided by discord.py and ensures that the context is valid for regular command processing | ||
| self.logger.info( | ||
| f"User '{message.author.display_name}' ran command '{ctx.command.name}'" | ||
| f"User '{user_log_repr(message.author)}' ran command '{ctx.command.name}'" |
| # A prefix was found, but no (custom) command was found. This means the user is trying to run a command that does not exist. | ||
| self.logger.info( | ||
| f"User '{ctx.author.display_name}' attempted to run an unrecognized command: '{ctx.message.content[1:]}'" | ||
| f"User '{user_log_repr(ctx.author)}' attempted to run an unrecognized command: '{ctx.message.content[1:]}'" |
| elif isinstance(error, commands.CommandNotFound): | ||
| self.bot.logger.info( | ||
| f"User '{ctx.author.display_name}' attempted to run an unrecognized command: '{ctx.message.content[1:]}'" | ||
| f"User '{user_log_repr(ctx.author)}' attempted to run an unrecognized command: '{ctx.message.content[1:]}'" |
3 participants
See https://discord.com/channels/116914772766752769/165640273810948097/1544019167798239233 for embeds, and https://discord.com/channels/116914772766752769/1216162486152335471/1544010187546038282 as well as https://discord.com/channels/116914772766752769/1216162486152335471/1544010104511266841 for nickame usage in normal commands.
For tests, the output of the test attached was sent in https://discord.com/channels/116914772766752769/165640273810948097/1544010979694608516, and seems to raise no issues with Discord Markdown. See https://discord.com/channels/116914772766752769/165640273810948097/1544023339042087113 for tests related to links. Note that I've not implemented direct link escaping (
https://www.youtube.com) yet.(Edit: Fix name of function in tests)