Conversation
This add a args feature, for example if your arg is called `argreply`
You can do: `?reply Hello, read the following: {argreply}
lorenzo132
requested review from
ModmailTest,
StephenDaDev,
Taaku18,
martinbndr and
sebkuip
lorenzo132
dismissed stale reviews from StephenDaDev and martinbndr
via
fed5044
StephenDaDev
added
the
changelog
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| **self.bot.args, | ||
| channel=ctx.channel, | ||
| recipient=ctx.thread.recipient, | ||
| author=ctx.message.author, |
There was a problem hiding this comment.
In the freply, fareply, fpreply, and fpareply commands, args are added before the built-in variables (channel, recipient, author). This means that if an arg is named "channel", "recipient", or "author", it will be overridden by the built-in variables, potentially causing unexpected behavior. Consider either adding args after the built-in variables (so built-in variables take precedence) or documenting this behavior and warning users not to use reserved names.
| **self.bot.args, | ||
| channel=ctx.channel, | ||
| recipient=ctx.thread.recipient, | ||
| author=ctx.message.author, |
There was a problem hiding this comment.
In the freply, fareply, fpreply, and fpareply commands, args are added before the built-in variables (channel, recipient, author). This means that if an arg is named "channel", "recipient", or "author", it will be overridden by the built-in variables, potentially causing unexpected behavior. Consider either adding args after the built-in variables (so built-in variables take precedence) or documenting this behavior and warning users not to use reserved names.
| if name not in self.bot.args: | ||
| embed = create_not_found_embed(name, self.bot.args.keys(), "Arg") |
There was a problem hiding this comment.
The args command uses direct name lookup without resolving through aliases like the snippet command does with _resolve_snippet. This creates an inconsistency where snippets can be accessed via their aliases but args cannot. Consider adding a _resolve_arg method similar to _resolve_snippet to provide consistent behavior.
| async def args_add(self, ctx, name: str.lower, *, value: commands.clean_content): | ||
| """ | ||
| Add an arg. | ||
|
|
||
| Simply to add an arg, do: ``` | ||
| {prefix}args add name value | ||
| ``` | ||
| """ | ||
| if name in self.bot.args: | ||
| embed = discord.Embed( | ||
| title="Error", | ||
| color=self.bot.error_color, | ||
| description=f"Arg `{name}` already exists.", | ||
| ) | ||
| return await ctx.send(embed=embed) | ||
|
|
||
| if len(name) > 120: | ||
| embed = discord.Embed( | ||
| title="Error", | ||
| color=self.bot.error_color, | ||
| description="Arg names cannot be longer than 120 characters.", | ||
| ) | ||
| return await ctx.send(embed=embed) | ||
|
|
||
| self.bot.args[name] = value | ||
| await self.bot.config.update() | ||
|
|
||
| embed = discord.Embed( | ||
| title="Added arg", | ||
| color=self.bot.main_color, | ||
| description="Successfully created arg.", | ||
| ) | ||
| return await ctx.send(embed=embed) |
There was a problem hiding this comment.
The args_add command doesn't check if the arg name conflicts with existing commands, snippets, or aliases, unlike snippet_add which has these validation checks. This could lead to confusion or unexpected behavior. Consider adding similar validation to prevent args from conflicting with commands, snippets, or aliases.
| async def args_add(self, ctx, name: str.lower, *, value: commands.clean_content): | ||
| """ | ||
| Add an arg. | ||
|
|
||
| Simply to add an arg, do: ``` | ||
| {prefix}args add name value | ||
| ``` | ||
| """ | ||
| if name in self.bot.args: | ||
| embed = discord.Embed( | ||
| title="Error", | ||
| color=self.bot.error_color, | ||
| description=f"Arg `{name}` already exists.", | ||
| ) | ||
| return await ctx.send(embed=embed) | ||
|
|
||
| if len(name) > 120: | ||
| embed = discord.Embed( | ||
| title="Error", | ||
| color=self.bot.error_color, | ||
| description="Arg names cannot be longer than 120 characters.", | ||
| ) | ||
| return await ctx.send(embed=embed) | ||
|
|
||
| self.bot.args[name] = value | ||
| await self.bot.config.update() | ||
|
|
||
| embed = discord.Embed( | ||
| title="Added arg", | ||
| color=self.bot.main_color, | ||
| description="Successfully created arg.", | ||
| ) | ||
| return await ctx.send(embed=embed) |
There was a problem hiding this comment.
The args feature doesn't validate against reserved variable names that could conflict with format variables in the freply, fareply, fpreply, and fpareply commands (e.g., "channel", "recipient", "author"). Users could create args with these names, leading to unexpected behavior where the built-in variables override the user-defined args. Consider adding validation in args_add and args_rename to prevent using these reserved names.
|
|
||
| When `{prefix}args` is used by itself, this will retrieve | ||
| a list of args that are currently set. `{prefix}args name` will show what the | ||
| arg point to. |
There was a problem hiding this comment.
The description for the args command docstring says "arg point to" which is grammatically incorrect. It should be "arg points to" (singular verb agreement).
| arg point to. | |
| arg points to. |
| **self.bot.args, | ||
| channel=ctx.channel, | ||
| recipient=ctx.thread.recipient, | ||
| author=ctx.message.author, |
There was a problem hiding this comment.
In the freply, fareply, fpreply, and fpareply commands, args are added before the built-in variables (channel, recipient, author). This means that if an arg is named "channel", "recipient", or "author", it will be overridden by the built-in variables, potentially causing unexpected behavior. Consider either adding args after the built-in variables (so built-in variables take precedence) or documenting this behavior and warning users not to use reserved names.
| To create an arg: | ||
| - `{prefix}args add arg-name A value.` | ||
|
|
||
| You can use your arg in a reply with `{arg-name}`. |
There was a problem hiding this comment.
The documentation states "You can use your arg in a reply with {arg-name}" but doesn't mention that args only work in certain reply commands (reply, freply, fareply, fpreply, fpareply) and not in others (areply, preply, pareply). Consider updating the documentation to clarify which commands support args to avoid user confusion.
| You can use your arg in a reply with `{arg-name}`. | |
| You can use your arg in supported reply commands (` | |
| reply`, `freply`, `fareply`, `fpreply`, `fpareply`) with `{arg-name}`. Args are | |
| not available in `areply`, `preply`, or `pareply`. |
| if not self.bot.args: | ||
| embed = discord.Embed( | ||
| color=self.bot.error_color, | ||
| description="You dont have any args at the moment.", |
There was a problem hiding this comment.
Inconsistent contraction usage: "dont" should be "don't" with an apostrophe.
| description="You dont have any args at the moment.", | |
| description="You don't have any args at the moment.", |
| **self.bot.args, | ||
| channel=ctx.channel, | ||
| recipient=ctx.thread.recipient, | ||
| author=ctx.message.author, |
There was a problem hiding this comment.
In the freply, fareply, fpreply, and fpareply commands, args are added before the built-in variables (channel, recipient, author). This means that if an arg is named "channel", "recipient", or "author", it will be overridden by the built-in variables, potentially causing unexpected behavior. Consider either adding args after the built-in variables (so built-in variables take precedence) or documenting this behavior and warning users not to use reserved names.
3 participants
This add a args feature, for example if your arg is called
argreplyYou can do: `?reply Hello, read the following: {argreply}