fix(tux/cogs/info/info.py): fixes embed lenght bug for roles and emotes #491
Conversation
Reviewer's Guide by SourceryThis pull request implements pagination for the File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @teilorr - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues 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 to tell me if it was helpful.
tux/cogs/info/info.py
Outdated
|
|
||
| embed.add_field(name="Roles", value=", ".join(roles), inline=False) | ||
| chunk_size = 32 | ||
| if not len(roles) > chunk_size: |
There was a problem hiding this comment.
suggestion: Extract paginated embed logic to a separate method
The pagination logic for both roles and emotes commands is very similar. Consider extracting this into a generic method to reduce code duplication and improve maintainability.
tux/cogs/info/info.py
Outdated
| menu = self._add_buttons_to_menu(menu) | ||
| await menu.start() | ||
|
|
||
| def _chunks[T](self, it: Iterator[T], size: int) -> Generator[list[T], None, None]: |
There was a problem hiding this comment.
suggestion: Use itertools.islice for chunking and lowercase type variable
Consider using itertools.islice from the standard library for chunking instead of a custom implementation. Also, use a lowercase letter for the type variable (t instead of T) to follow Python conventions.
from itertools import islice
def _chunks(self, it: Iterator[t], size: int) -> Generator[list[t], None, None]:
iterator = iter(it)
return iter(lambda: list(islice(iterator, size)), [])
tux/cogs/info/info.py
Outdated
| f"{list_type.capitalize()} list for {guild_name}:\n {" ".join(items) if items else "No items available."}" | ||
| ) | ||
|
|
||
| def _add_buttons_to_menu[T: ViewMenu](self, menu: T) -> T: |
There was a problem hiding this comment.
suggestion: Simplify type hint in _add_buttons_to_menu
The generic type hint T: ViewMenu seems unnecessary here. You could simply use ViewMenu as the type hint for both the parameter and return type.
| def _add_buttons_to_menu[T: ViewMenu](self, menu: T) -> T: | |
| def _add_buttons_to_menu(self, menu: ViewMenu) -> ViewMenu: |
tux/cogs/info/info.py
Outdated
| ) | ||
|
|
||
| embed.add_field(name="Roles", value=", ".join(roles), inline=False) | ||
| chunk_size = 32 |
There was a problem hiding this comment.
issue (complexity): Consider simplifying the pagination implementation and reducing method complexity.
While the pagination feature for roles and emotes is valuable, the implementation can be simplified to reduce complexity:
- Replace the custom
_chunksmethod withitertools.islice:
from itertools import islice
def chunks(iterable, size):
iterator = iter(iterable)
return iter(lambda: list(islice(iterator, size)), [])- Simplify the
rolesandemotescommands by inlining the embed description creation and reducing nesting:
@info.command(name="roles", aliases=["r"], usage="info roles")
async def roles(self, ctx: commands.Context[commands.Bot]) -> None:
if not ctx.guild:
return
roles = [role.mention for role in ctx.guild.roles]
chunk_size = 32
if len(roles) <= chunk_size:
embed = discord.Embed(title="Server Roles", color=discord.Color.blurple())
embed.description = f"Role list for {ctx.guild.name}:\n{' '.join(roles)}"
await ctx.send(embed=embed)
return
menu = ViewMenu(ctx, menu_type=ViewMenu.TypeEmbed)
for chunk in chunks(roles, chunk_size):
embed = discord.Embed(title="Server Roles", color=discord.Color.blurple())
embed.description = f"Role list for {ctx.guild.name}:\n{' '.join(chunk)}"
menu.add_page(embed)
menu.add_button(ViewButton.go_to_first_page())
menu.add_button(ViewButton.back())
menu.add_button(ViewButton.next())
menu.add_button(ViewButton.go_to_last_page())
menu.add_button(ViewButton.end_session())
await menu.start()Apply similar changes to the emotes command. This approach eliminates the need for _create_embed_desc and _add_buttons_to_menu methods, reducing overall complexity while maintaining functionality.
…lity and maintainability refactor(info.py): simplify command decorators for readability refactor(info.py): change ctx type from commands.Context[commands.Bot] to commands.Context for simplicity feat(info.py): add paginated_embed method to handle paginated embeds in a more reusable way fix(info.py): check if guild exists before proceeding with commands to prevent errors feat(info.py): add more detailed server and member information to embeds refactor(info.py): streamline embed creation for cleaner code refactor(info.py): remove unused _create_embed_desc method refactor(info.py): update _chunks method to return list of strings for consistency refactor(info.py): update _add_buttons_to_menu method to return ViewMenu for consistency refactor: replace individual button addition with loop to improve code readability and maintainability
2 participants
Description
Adds pagination to
/info/rolesandinfo/emotes. This is useful when the contents are more than the expected embed limit. Closes #449Type of Change
Guidelines
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 followed all of these guidelines.
How Has This Been Tested? (if applicable)
Please describe how you tested your code. e.g describe what commands you ran, what arguments, and any config stuff (if applicable)
Screenshots (if applicable)
Please add screenshots to help explain your changes.
Additional Information
Please add any other information that is important to this PR.
Summary by Sourcery
Implement pagination for the
/info/rolesand/info/emotescommands to address the embed length limitation, ensuring proper display of roles and emotes when their count exceeds the embed capacity.Bug Fixes: