Skip to content

Removed italics from the help command#2272

Merged
wookie184 merged 4 commits into
python-discord:mainfrom
Ibrahim2750mi:remove-italics
Sep 17, 2022
Merged

Removed italics from the help command#2272
wookie184 merged 4 commits into
python-discord:mainfrom
Ibrahim2750mi:remove-italics

Conversation

@Ibrahim2750mi
Copy link
Copy Markdown
Contributor

@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Sep 8, 2022

There are more uses of italics. Should these be removed too? We need to come to a consensus.

f"\n**`{PREFIX}{command.qualified_name}{signature}`**\n*{command.short_doc or 'No details provided'}*"

embed.description = f"**{cog.qualified_name}**\n*{cog.description}*"

description = f"**{category.name}**\n*{category.description}*"

There are also these, which are bold and italic

NOT_ALLOWED_TO_RUN_MESSAGE = "***You cannot run this command.***\n\n"

command_details += "***This command is disabled.***\n\n"

@Ibrahim2750mi
Copy link
Copy Markdown
Contributor Author

Ibrahim2750mi commented Sep 8, 2022

Should be changed

In these the title(or name) of the description uses bold which is fine by me, could remove the italics from their description if it is agreed upon:

Shouldn't be changed

I think in these it is fine to use both bold and italics:

@Ibrahim2750mi
Copy link
Copy Markdown
Contributor Author

@MarkKoz should I make another commit according to my comment I posted or should I wait until some further discussion?

@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Sep 8, 2022

@MarkKoz should I make another commit according to my comment I posted or should I wait until some further discussion?

It's up to you. But if you do, you may end up needing to revert some or all of it depending on the conclusion. If you provide pictures of each output that has/had italics, it would help gather opinions.

@Ibrahim2750mi
Copy link
Copy Markdown
Contributor Author

@MarkKoz

Line number edit Before After
Cog description #L375 2022-09-09_14-18_1 2022-09-09_14-19
Brief command detail #L334 2022-09-09_14-21 2022-09-09_14-21_1
Category description #L415 2022-09-09_14-24 2022-09-09_14-24_1
Command description #L310 2022-09-09_14-28 2022-09-09_14-28_1

Couldn't do this for the below because don't how to trigger these:

@wookie184
Copy link
Copy Markdown
Contributor

I think all the changes so far are improvements, I definitely think not being in italics makes reading longer text cleaner and easier.

RE the two that you couldn't trigger, I think they could be left as is and this changed to be italics:

lines.append("(nothing to display)")

My reasoning being that these are all somewhat "meta", I also wouldn't mind just making none of them in italics. I'd like to get another opinion first either way though.

@mbaruh
Copy link
Copy Markdown
Member

mbaruh commented Sep 10, 2022

Having placeholder values like "nothing to display" in italics and the rest without makes sense to me

@wookie184 wookie184 added a: frontend Related to output and formatting p: 2 - normal Normal Priority t: enhancement Changes or improvements to existing features labels Sep 10, 2022
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

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

Thanks!

@wookie184 wookie184 added the s: needs review Author is waiting for someone to review and approve label Sep 11, 2022
@wookie184 wookie184 enabled auto-merge (squash) September 17, 2022 12:56
@wookie184 wookie184 merged commit 343858f into python-discord:main Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: frontend Related to output and formatting p: 2 - normal Normal Priority s: needs review Author is waiting for someone to review and approve t: enhancement Changes or improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants