Skip to content

#2257 the title in help commands is too large.#2286

Closed
ddjerqq wants to merge 2 commits into
python-discord:mainfrom
ddjerqq:feature/2257/trim-title-help-command-error
Closed

#2257 the title in help commands is too large.#2286
ddjerqq wants to merge 2 commits into
python-discord:mainfrom
ddjerqq:feature/2257/trim-title-help-command-error

Conversation

@ddjerqq
Copy link
Copy Markdown

@ddjerqq ddjerqq commented Oct 6, 2022

trimmed title in send_error_message. the title will be 256 characters at most now.

Copy link
Copy Markdown
Contributor

@CaedenPH CaedenPH left a comment

Choose a reason for hiding this comment

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

Perhaps there is a better way to format this instead of just completely cutting off anything after 256 characters. Does anyone have any ideas?

Comment thread bot/exts/info/help.py Outdated
Co-authored-by: Caeden <caedenperelliharris@gmail.com>
Comment thread bot/exts/info/help.py
async def send_error_message(self, error: HelpQueryNotFound) -> None:
"""Send the error message to the channel."""
embed = Embed(colour=Colour.red(), title=str(error))
# 2257 the title can be too big. so we trim it
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
# 2257 the title can be too big. so we trim it
# Embed titles have a length limits, so we trim here.

having a number out of context in a comment isn't very useful.

I know it links to the issue you are fixing, but in 2 months when someone looks at this code, they won't.
Linking the comment to the issue doesn't really add anything, since we already explain why we need to do this.

Comment thread bot/exts/info/help.py
"""Send the error message to the channel."""
embed = Embed(colour=Colour.red(), title=str(error))
# 2257 the title can be too big. so we trim it
embed = Embed(colour=Colour.red(), title=str(error)[:256])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a title of 256 characters is far too long. I know discord allows it to be this long, but that doesn't mean it would be a good title.

Suggested change
embed = Embed(colour=Colour.red(), title=str(error)[:256])
embed = Embed(
colour=Colour.red(),
title=textwrap.shorten(error, width=150, placeholder="…"),
)

Doing something like this, where we limit to 150 characters, and also append an ellipses would be better IMO.

This suggestion will also require an import textwrap at the top of the file.

@ChrisLovering ChrisLovering added t: bug Something isn't working p: 3 - low Low Priority a: utility Related to utility commands: (bot, eval, extensions, jams, reminders, snekbox, utils) s: waiting for author Waiting for author to address a review or respond to a comment labels Oct 19, 2022
@Xithrius
Copy link
Copy Markdown
Contributor

@ddjerqq Greetings. There seems to be request for changes on this PR.

Would you mind fixing them up?

Thanks!

@ddjerqq
Copy link
Copy Markdown
Author

ddjerqq commented Oct 26, 2022

oh im so sorry, I forgot i was assigned to this! ill fix the issues asap

@Xithrius
Copy link
Copy Markdown
Contributor

No problem. Thanks for the quick reply!

@wookie184 wookie184 self-assigned this Jan 14, 2023
@wookie184
Copy link
Copy Markdown
Contributor

I'll finish this off. I don't have commit permissions to this branch so i'll open a new PR.

@wookie184
Copy link
Copy Markdown
Contributor

Closing in favour of #2380

@wookie184 wookie184 closed this Jan 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: utility Related to utility commands: (bot, eval, extensions, jams, reminders, snekbox, utils) p: 3 - low Low Priority s: waiting for author Waiting for author to address a review or respond to a comment t: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HTTPException: 400 Bad Request (error code: 50035): Invalid Form Body

5 participants