Skip to content

Conversation

@akiraveliara
Copy link
Member

as discussed approximately here: https://discord.com/channels/379378609942560770/379386901725052928/995957157298192447

this PR adds support for categorizing commands using a new CategoryAttribute. the categorizing behaviour is only invoked when at least one command in the searched batch is marked with the attribute, and it can be disabled by commenting out all related attributes - i do not believe a config toggle makes sense here, given the configuration is already quite huge.

@akiraveliara akiraveliara added enhancement commands-next For issues related to DSharpPlus.CommandsNext v4.x labels Aug 8, 2022
namespace DSharpPlus.CommandsNext.Attributes
{
[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
public sealed class CategoryAttribute : Attribute
Copy link
Member

Choose a reason for hiding this comment

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

Pain. I'd ask if we really need yet another attribute when one exists in the BCL, but we've rolled our own for everything else.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, that was roughly my line of thought. would be strange to somehow need System.ComponentModel for just this one attribute and none other.

Copy link
Contributor

Choose a reason for hiding this comment

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

Keep it consistent. I do think we should have a PR that replaces the CNext attributes with the ComponentModel attributes, but that'd be a big breaking change and should require an announcement


foreach(var category in categories)
{
this.EmbedBuilder.AddField(category, string.Join(", ", subcommands.Where(xm => xm.Category == category).Select(xm => Formatter.InlineCode(xm.Name))), false);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of constantly re-iterating over the entire command list, consider a GroupBy statement instead.

@VelvetToroyashi
Copy link
Member

No description provided.

@VelvetToroyashi VelvetToroyashi merged commit 00530ed into DSharpPlus:master Aug 8, 2022
Copy link
Contributor

@OoLunar OoLunar left a comment

Choose a reason for hiding this comment

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

LGTM, I guess

namespace DSharpPlus.CommandsNext.Attributes
{
[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
public sealed class CategoryAttribute : Attribute
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep it consistent. I do think we should have a PR that replaces the CNext attributes with the ComponentModel attributes, but that'd be a big breaking change and should require an announcement

commandBuilder.WithHiddenStatus(true);
break;

case CategoryAttribute c:
Copy link
Contributor

Choose a reason for hiding this comment

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

Variable naming >:(

{
this.EmbedBuilder.AddField(this.Command is not null ? "Subcommands" : "Commands", string.Join(", ", subcommands.Select(x => Formatter.InlineCode(x.Name))), false);

var categories = subcommands.GroupBy(xm => xm.Category).OrderBy(xm => xm.Key == null).ThenBy(xm => xm.Key);
Copy link
Contributor

Choose a reason for hiding this comment

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

Surely this can be done differently, more efficiently?

@akiraveliara akiraveliara deleted the v4/cnext-categories branch December 20, 2022 12:29
VelvetToroyashi pushed a commit that referenced this pull request Jan 28, 2023
* Change GuildMemberUpdatedEventArgs to contain before and after DiscordMember objects (#1359)

* Fix #1325 - WebhookBuilder Throws Bad Request (#1363)

* use DiscordMentions on RestWebhookPayloads

* keep null value when null

* remove null check from discordmentions arg

* Implemented PATCH /guilds/:guild_id/roles (tested) (#1311)

* Fix compile errors caused by #1333

* Update hosting.md

* Remove unused DSharpPlus.MSTest folder

* Cache threads upon thread creation (#1366)

* categorization for the cnext default help command (#1371)

* Fix memberBefore missing roles in GuildMemberUpdateEventArgs when not… (#1364)

* Fix memberBefore missing roles in GuildMemberUpdateEventArgs when not in cache

* Update cached guild member in OnGuildMemberUpdateEventAsync internal event handler

* Update DiscordClient.Dispatch.cs

Co-authored-by: Lunar Starstrum <lunar@forsaken-borders.net>

* Ported CommandsNext cooldown attribute to Slash Commands (#1375)

* Ported CommandsNext cooldown attribute to Slash Commands; Fixed compiler warnings and suggestions

* Still untested; Attempted to resolve @Erisa's review

* Made all exceptions inherit from ApplicationCommandExecutionChecksFailedException (previously Exception)

* Start on converters

* Added custom converter support:

- No, I did not implement the checks in the converters. I felt that should be done in a separate area of code
- Yes, I did break my Enter, Backspace, D and Apostrophe keys because of this PR. Don't ask how.

* Fixed startup errors; Added DiscordMessageArgumentConverter; Added (un)register converter methods (untested).

* Fix registering converters

* (untested) Add number converters; Fix enums not registering choices correctly

* Add DiscordMember converter; Switch from ArgumentException to new MissingArgumentConverterException

* Actually fix Nullable and update TimeSpan converter to match with CNext

* Add params/array support

* Boundry checks

* [ci-skip] Resolve naming errors

* "One less variable allocation"

* Remove unsupport NETSTANDARD1_3 support; Un-Emziware those variable names

* Remove unused property

* Extra parameter limit attribute check

* Remove UnregisterConverter(s) methods

* Changed BCL types to use framework names

* Pass InnerException to prevent passing TargetInvocationException

* Fix enum support (again)

---------

Co-authored-by: Bob Loblaw <razvanmandache@gmail.com>
Co-authored-by: waffle-lord <76401815+waffle-lord@users.noreply.github.com>
Co-authored-by: Iris Féanorá <61018569+ExaInsanity@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands-next For issues related to DSharpPlus.CommandsNext enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants