Add array support for slash commands #340
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit changes how Remora.Discord.Commands maps slash commands,
enabling support for arrays to be parsed and accepted, despite Discord
STILL not having support for this.
For better or worse, a snippet of code had to be removed:
```cs
var collectionType = parameter.GetActualParameterType();
if (!collectionType.IsCollection() && !collectionType.IsArray)
{
// Sanity check
throw new InvalidOperationException("Collection parameter was not a collection.");
}
```
^ For some reason, `.IsCollection()` returns...false? I gave up on
trying to figure out why that was the case - the code still works fine
and there's other validity checks elsewhere.
BREAKING-CHANGE: Commands with the structure of `name__numbers`
(e.g. `param__1`) are *no longer valid* and will get erased by the
pre-parsing step that is a requisite to map arrays. This should be an
absolutely niche case, but is still technically a breaking change, and
should be documented as such.
Some other things of note:
- Attributes applied to the array apply to all elements in the array
(this should go without saying, but hey)
- `[Range]` controls the array's length; `Min` = required, `Max` = Length
- If Min < Max, `Min` elements will be required and `Max - Min` elements
will be optional. Max will coallecse to Min when unspecified.
- Due to Discord's..questionable limitations, optional array parameters
are placed at the end of the argument list for slash commands, but do
preserve the order they're declared in relative to each other.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| else | ||
| { | ||
| tempParameters[name] = (List<string>)[..existingValues, ..values]; |
Check warning
Code scanning / InspectCode
Redundant cast Warning
| return (option.Name, values); | ||
| #pragma warning disable SA1010 // Stylecop doesn't handle collection expressions correctly here | ||
| // Casting to string[] is optional, but absolves reliance on Roslyn making an inline array here. | ||
| return (option.Name, (string[])[value]); |
Check warning
Code scanning / InspectCode
Redundant cast Warning
| @@ -37,7 +37,9 @@ | |||
| using Remora.Discord.Commands.Attributes; | |||
| using Remora.Discord.Commands.Services; | |||
| using Remora.Rest.Core; | |||
| using Remora.Rest.Extensions; | |||
Check warning
Code scanning / InspectCode
Redundant using directive Warning
…ents' into velvet/feat/variadic-slash-arguments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit changes how Remora.Discord.Commands maps slash commands, enabling support for arrays to be parsed and accepted, despite Discord STILL not having support for this.
For better or worse, a snippet of code had to be removed:
^ For some reason,
.IsCollection()returns...false? I gave up on trying to figure out why that was the case - the code still works fine and there's other validity checks elsewhere.BREAKING-CHANGE: Commands with the structure of
name__numbers(e.g.param__1) are no longer valid and will get erased by the pre-parsing step that is a requisite to map arrays. This should be an absolutely niche case, but is still technically a breaking change, and should be documented as such.Some other things of note:
[Range]controls the array's length;Min= required,Max= LengthMinelements will be required andMax - Minelements will be optional. Max will coalesce to Min when unspecified.