-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Comments
Conversation
This add a args feature, for example if your arg is called `argreply`
You can do: `?reply Hello, read the following: {argreply}
fed5044
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.
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.
Copilot
AI
Jan 16, 2026
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.
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.
Copilot
AI
Jan 16, 2026
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.
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.
Copilot
AI
Jan 16, 2026
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 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.
Copilot
AI
Jan 16, 2026
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 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.
Copilot
AI
Jan 16, 2026
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 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.
Copilot
AI
Jan 16, 2026
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 description for the args command docstring says "arg point to" which is grammatically incorrect. It should be "arg points to" (singular verb agreement).
Copilot
AI
Jan 16, 2026
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.
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.
Copilot
AI
Jan 16, 2026
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 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.
Copilot
AI
Jan 16, 2026
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.
Inconsistent contraction usage: "dont" should be "don't" with an apostrophe.
Copilot
AI
Jan 16, 2026
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.
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.
This add a args feature, for example if your arg is called
argreplyYou can do: `?reply Hello, read the following: {argreply}