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

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

Merged
merged 14 commits into from
Oct 29, 2024

Conversation

cswimr
Copy link
Contributor

@cswimr cswimr commented Jan 5, 2024

Description of the changes

  • added list_urls argument to [p]repo list
    • this argument determines if urls will be output by the command
  • added codeblock argument to [p]repo list
    • this argument determines if the output of the command will be a codeblock or not. useful if you want to right click copy urls from the command's output

img

img

Have the changes in this PR been tested?

Yes

cswimr added 2 commits January 5, 2024 00:13
detailed changes:
- added list_urls argument
  - this argument determines if urls will be output by the command
- added codeblock argument
  - this argument determines if the output of the command will be a codeblock or not. useful if you want to right click copy urls from the command's output
@cswimr cswimr requested a review from Jackenmen as a code owner January 5, 2024 05:19
@github-actions github-actions bot added the Category: Cogs - Downloader This is related to the Downloader cog. label Jan 5, 2024
@Flame442 Flame442 added the Type: Enhancement Something meant to enhance existing Red features. label Jan 5, 2024
@Jackenmen
Copy link
Member

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
Copy link
Contributor Author

cswimr commented Jan 5, 2024

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
Copy link
Contributor Author

cswimr commented Jan 14, 2024

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
Copy link
Member

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
Copy link
Contributor Author

cswimr commented Jan 14, 2024

Done!
image

@Kreusada Kreusada self-assigned this Feb 13, 2024
@cswimr
Copy link
Contributor Author

cswimr commented Mar 6, 2024

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
Copy link
Member

Flame442 commented Mar 6, 2024

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 and others added 2 commits March 6, 2024 15:45
bolded repository names and changed the size of the markdown header, see:
> 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.
@cswimr
Copy link
Contributor Author

cswimr commented Mar 6, 2024

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.

Copy link
Member

@Kreusada Kreusada left a comment

Choose a reason for hiding this comment

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

Tested and works great, thanks for contributing.

@Kreusada Kreusada added this to the 3.5.14 milestone Oct 29, 2024
@Kreusada Kreusada enabled auto-merge (squash) October 29, 2024 18:09
@Kreusada Kreusada merged commit 30058c0 into Cog-Creators:V3/develop Oct 29, 2024
15 of 16 checks passed
@red-githubbot red-githubbot bot added the Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. label Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Cogs - Downloader This is related to the Downloader cog. Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. Type: Enhancement Something meant to enhance existing Red features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants