Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Help] Fix embed size calculation for additional text #3208

Merged
merged 9 commits into from Dec 26, 2019
Merged

[Help] Fix embed size calculation for additional text #3208

merged 9 commits into from Dec 26, 2019

Conversation

mikeshardmind
Copy link
Contributor

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

Handles the total sum of text in an embed properly.

@mikeshardmind mikeshardmind added this to the 3.2.0 milestone Dec 22, 2019
@Flame442 Flame442 self-assigned this Dec 26, 2019
Copy link
Member

@Flame442 Flame442 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this factor in the Red V3 field? I can't really tell...

This might fail if someone manages to have an obscene amount of pages, but I doubt we need to worry about that.

foot_text = embed_dict["footer"]["text"]
if foot_text:
offset += len(foot_text)
offset += len(embed_dict["description"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This throws an error any way I try to run [p]help

[2019-12-26 14:30:56] [ERROR] red: Exception in command 'help'
Traceback (most recent call last):
  File "<venv>\lib\site-packages\redbot\core\commands\help.py", line 644, in red_help
    await ctx.bot.send_help_for(ctx, thing_to_get_help_for)
  File "<venv>\lib\site-packages\redbot\core\bot.py", line 436, in send_help_for
    return await self._help_formatter.send_help(ctx, help_for)
  File "<venv>\lib\site-packages\redbot\core\commands\help.py", line 143, in send_help
    await self.format_command_help(ctx, help_for)
  File "<venv>\lib\site-packages\redbot\core\commands\help.py", line 233, in format_command_help
    await self.make_and_send_embeds(ctx, emb)
  File "<venv>\lib\site-packages\redbot\core\commands\help.py", line 303, in make_and_send_embeds
    offset += len(embed_dict["description"])
KeyError: 'description'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sigh of course. Give me a sec, I know why that's happening.

author_info = {"name": f"{ctx.me.display_name} Help Menu", "icon_url": ctx.me.avatar_url}

# Offset calculation here is for total embed size limit
# 20 accounts for# *Page {i} of {page_count}*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should half of this be on a separate line or is there just a random extra #?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was intended to split the literal code being accounted for from the rest of the comment.

Copy link
Collaborator

@Cog-CreatorsBot Cog-CreatorsBot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested by Flame#2941

@Flame442 Flame442 added the QA: Changes Requested Used by few QA members. Awaiting changes requested by maintainers or QA. label Dec 26, 2019
Copy link
Member

@Flame442 Flame442 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from this and some non-issues already discussed, this looks good to me.

"""
if limit <= 0:
await ctx.send(_("You must give a positive value!"))
if limit <= 500:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be <, otherwise "500" does not work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorted, thanks.

Michael H added 2 commits December 26, 2019 16:37
Copy link
Collaborator

@Cog-CreatorsBot Cog-CreatorsBot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved by Flame#2941

@Flame442 Flame442 added QA: Passed Used by few QA members. Has been approved by the assigned QA member(s). and removed QA: Changes Requested Used by few QA members. Awaiting changes requested by maintainers or QA. labels Dec 26, 2019
@mikeshardmind mikeshardmind merged commit 8b18526 into Cog-Creators:V3/develop Dec 26, 2019
@mikeshardmind mikeshardmind deleted the help-pagination-fix branch January 27, 2020 08:40
@Jackenmen Jackenmen added the Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing. label Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA: Passed Used by few QA members. Has been approved by the assigned QA member(s). Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants