serenity-rs / poise

Discord bot command framework for serenity, with advanced features like edit tracking and flexible argument parsing
MIT License
624 stars 111 forks source link

Add support for integer and double option choices. #230

Closed bavalpey closed 8 months ago

bavalpey commented 9 months ago

According to the discord API, paramater choices can be string, integer, or doubles for parameters that are registered as STRING, INTEGER, or NUMBER.

There is currently no way to register options in.

What I would like to see is support for this, ideally in the same form as entering descriptions.

#[poise::command(slash_command)]
async fn my_method(
    ctx: Context<'_>,
    #[description = "Description"] // This form is great and easy to use.  Why make it cumbersome with the enum form?
    #[choices = (1u8, 2u8, 3u8, 4u8, 5u8, 9u8)] name: u8, // tuple works best
) -> Result<(), Error> {
  // ...
}
kangalio commented 8 months ago

The syntax should be #[choices(1, 2, 3, 4, 5, 9)] name: u8, but I agree this is a good feature. It should work for all types that implement Display and can be parsed from a literal token

I'm just not quite sure how the macro should handle the types. Like

type MyInt = u32;
#[poise::command]
async fn my_method(ctx: Context<'_>, #[choices(1, 2, 3, 4, 5, 9)] name: MyInt)

The macro only sees ambiguous integer literals and "MyInt"

kangalio commented 8 months ago

Having written that code snippet out, it doesn't seem hard anymore lol. The macro can generate the string labels with $value.to_string() and dispatch from the choice index to the value with a match statement match index { 0 => $value0, 1 => $value2 }; and as the type it can just use literally MyInt everywhere. That should work

kangalio commented 8 months ago

The devil is being in the details, as always

kangalio commented 8 months ago

I've implemented this in the inline-choice branch, could I interest you in trying it out / testing it?

bavalpey commented 8 months ago

I've implemented this in the inline-choice branch, could I interest you in trying it out / testing it?

Yup! I'll test it here in a moment.

bavalpey commented 8 months ago

This works great!

kangalio commented 8 months ago

That's fun to hear, if you knew the messy data flow I've had to wedge into to implement this :D But great then. I suppose we can close the issue and release, then?

bavalpey commented 8 months ago

Works for me!

On Wed, Dec 20, 2023, 12:09 PM kangalio @.***> wrote:

That's fun to hear, if you knew the messy data flow I've had to wedge into to implement this :D But great then. I suppose we can close the issue and release, then?

— Reply to this email directly, view it on GitHub https://github.com/serenity-rs/poise/issues/230#issuecomment-1864916239, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIYCSI6GBQ2C4MXU57NW5VDYKMSXFAVCNFSM6AAAAABAQT4GCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRUHEYTMMRTHE . You are receiving this because you authored the thread.Message ID: @.***>