Cog-Creators / Red-DiscordBot

A multi-function Discord bot
https://docs.discord.red
GNU General Public License v3.0
4.72k stars 2.3k forks source link

Add links to the output of [p]repo list #6284

Open cswimr opened 8 months ago

cswimr commented 8 months ago

Description of the changes

img

img

Have the changes in this PR been tested?

Yes

Jackenmen commented 8 months ago

Any reason we would want to keep the code block variant? Having two switches in a command is going to be more confusing than just one.

Another thing I wonder about is whether there's a point to a list_urls argument too, I guess it makes the list shorter when not used but do we consider this shorter variant important enough to have it?

cswimr commented 8 months ago

I don't really know, I just didn't want to change how it already looked in a way that wasn't optional. If you'd prefer it I can remove the old codeblock entirely. I could also just make using list_urls not use a codeblock as I did initially and just keep the codeblock there if you don't set list_urls to True.

cswimr commented 8 months ago

Do I need to do anything else for this pr to be merged @Jackenmen? Never got a response from you on my questions originally

Jackenmen commented 8 months ago

My question was an open question to any interested party (including other maintainers of this repository). It doesn't seem like anyone cared enough to respond so I would suggest going ahead and making the [p]repo list true false (no code block, links included) variant the default one and removing the options.

cswimr commented 8 months ago

Done!
image

cswimr commented 6 months ago

bored and thought it might be fun to add embeds to this command (using ctx.embed_requested()), would that be worth the extra code complexity

edit: probably not now that I think about it, because I don't believe red has a way to pagify embed contents built-in (if it does please let me know as this would be incredibly useful, but I haven't seen it before)

Flame442 commented 6 months ago

The original reason for it being in a code block was to match the formatting of [p]cog list, which used the codeblock to format with diff formatting. However, it was always confusing whether green or red should mean installed vs can be installed, and the diff format color scheme became much uglier, so #6152 swapped the format to a more neutral one relying more on the headers.

The # Installed Repos was intended to be for color formatting the header, and I think it might be a bit to big if we are sticking to a codeblock-less output. ## seems like a much more reasonable size. Additionally, I'd suggest bolding the repo names to make them stand out a bit more from the short description.

it might be fun to add embeds to this command

I don't think that fits with the general style of the rest of the bot, generally Red avoids using embeds for outputs that are supposed to be more utility focused, and instead keeps them limited to mostly user focused informational commands like [p]info and [p]now.

cswimr commented 6 months ago

The # Installed Repos was intended to be for color formatting the header, and I think it might be a bit to big if we are sticking to a codeblock-less output. ## seems like a much more reasonable size. Additionally, I'd suggest bolding the repo names to make them stand out a bit more from the short description.

Done in most recent commit.

I don't think that fits with the general style of the rest of the bot, generally Red avoids using embeds for outputs that are supposed to be more utility focused, and instead keeps them limited to mostly user focused informational commands like [p]info and [p]now.

Fair enough, makes sense.