feat(fun): Added cowsay cog #803
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis pull request introduces a new Class diagram for the Cowsay cogclassDiagram
class Cowsay {
-bot: Tux
+__init__(bot: Tux)
+draw_textbox(message: str) str
+cowsay(ctx: commands.Context[Tux], message: str, creature: str) None
}
Cowsay -- Tux : has a
class Tux {
}
note for Cowsay "This class implements the cowsay cog and its functionality"
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @thanosengine - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using a dictionary to store the creatures and their corresponding ASCII art for better organization.
- The error message for invalid creatures could be improved by using a code block to format the list of valid creatures.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
tux/cogs/fun/cowsay.py
Outdated
| self.bot = bot | ||
| self.cowsay.usage = generate_usage(self.cowsay) | ||
|
|
||
| async def draw_textbox(self, message: str) -> str: |
There was a problem hiding this comment.
issue (bug_risk): Potential crash when message is empty.
If an empty string is passed to textwrap.wrap, it returns an empty list and subsequently calling max() on that list will raise a ValueError. Consider handling the empty message case explicitly.
tux/cogs/fun/cowsay.py
Outdated
| """, | ||
| } | ||
|
|
||
| if creature not in creatures: |
There was a problem hiding this comment.
issue (complexity): Consider using list slicing and join to simplify the loop for creating the list of valid creatures when an invalid creature is provided to the command.
You could simplify the manual loop by using list slicing and join. For example, replace:
if creature not in creatures:
valid_creatures: list[str] = []
for idx, key in enumerate(creatures.keys()):
if idx < len(creatures) - 1:
valid_creatures.append(f"{key}, ")
else:
valid_creatures.append(f" and {key}")
await ctx.send(
f'Error: "{creature}" is not a valid creature! valid creatures are: {"".join(valid_creatures)}',
ephemeral=True,
)
returnwith a more concise approach:
if creature not in creatures:
keys = list(creatures.keys())
if len(keys) > 1:
valid_creatures = ", ".join(keys[:-1]) + " and " + keys[-1]
else:
valid_creatures = keys[0]
await ctx.send(
f'Error: "{creature}" is not a valid creature! Valid creatures are: {valid_creatures}',
ephemeral=True,
)
returnThis reduces complexity while keeping functionality intact.
tux/cogs/fun/cowsay.py
Outdated
Comment on lines
17
to
20
| message_box_lines: list[str] = [] | ||
| message_box_lines.append("/" + ("-" * (max_line_length + 2)) + "\\\n") | ||
| for line in message_lines: | ||
| box_line = "| " + line + (" " * (max_line_length - len(line))) + " |\n" |
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Merge append into list declaration (
merge-list-append) - Use f-string instead of string concatenation (
use-fstring-for-concatenation)
tux/cogs/fun/cowsay.py
Outdated
| return | ||
|
|
||
| textbox = await self.draw_textbox(message) | ||
| await ctx.send("```" + textbox + creatures[creature] + "```") |
There was a problem hiding this comment.
suggestion (code-quality): Use f-string instead of string concatenation [×3] (use-fstring-for-concatenation)
Suggested change
| await ctx.send("```" + textbox + creatures[creature] + "```") | |
| await ctx.send(f"```{textbox}{creatures[creature]}```") |
…e the eye character
for more information, see https://pre-commit.ci
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Added a new cog, cowsay.py, under the fun category.
This cog implements a new command,
cowsay <message> [creature], which will take the provided message, place it in a textbox, and add the specified creature underneath it, similarly to the well-known cowsay programGuidelines
My code follows the style guidelines of this project (formatted with Ruff)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if needed
My changes generate no new warnings
I have tested this change
Any dependent changes have been merged and published in downstream modules
I have added all appropriate labels to this PR
I have followed all of these guidelines.
How Has This Been Tested? (if applicable)
command has been tested in as many conditions as possible using my test instance
Screenshots (if applicable)
Additional Information
Please add any other information that is important to this PR.
Summary by Sourcery
Add a new cowsay command to the bot's fun category, allowing users to generate ASCII art messages with various creatures
New Features:
Enhancements: