serenity-rs / poise

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

Support Result as parameter type #177

Open B-2U opened 1 year ago

B-2U commented 1 year ago

question

I'm curious about how does the the auto converter in commands arguments work, for example:

#[poise::command(prefix_command,)]
pub async fn delmsg(
    ctx: Context<'_>,
    #[description = "Message to be deleted"] message: Option<Message>,
) -> Result<(), Error> {...}

message: Option<Message> is really impressive that can deal with both Message ID and Message Link, but I didn't find such methods in the doc

feature request

It start from a issue of the converter mentioned above, when the bot receive some unconvertable input (like inaccessible or invalid Message ID), It will directly call ctx.say to handle the error, which came from

poise::SlashArgError::Parse { error, input } => {
    poise::FrameworkError::ArgumentParse {
        ctx: ctx.into(),
        error,
        input: Some(input),
    }
}

, I tried to look into the source but I'm not good enough to understand them

So I wonder, like we can wrap type in Option to make it optional, is it possible to add such feature that wrap with Result so we can handle the potential convert error in command?

kangalio commented 1 year ago

Regarding the question: poise uses serenity's ArgumentConvert trait to convert strings into Discord model types. Poise adds a few specialized implementations beyond that for primitive types

kangalio commented 1 year ago

So I wonder, like we can wrap type in Option to make it optional, is it possible to add such feature that wrap with Result so we can handle the potential convert error in command?

Btw, the original intended way was to set a command-specific on_error handler (#[poise::command(on_error = "function_name")])

kangalio commented 1 year ago

Is there a use case that's not satisfied with a command-specific on_error handler?

B-2U commented 1 year ago

Screenshot_20230914-190309

I idea was in this way I can do other jobs on the failed string which might a player's ign instead of completely go into error workflow,

Well I think it's not that necessary, especially if there's heavy works behind t, but it will be really cool and rusty if possible just like Option<>

elkowar commented 7 months ago

I do agree that it'd be a pretty neat, and more or less "obvious" API to support having commands take Results to be able to deal with parse-errors themselves -- although the command-specific on_error handler is definitely also a neat alternative

kangalio commented 7 months ago

I agree, it would fit into the existing API very well

Implementation may turn out to be quite complicated the way the code is currently structured (or maybe not - I haven't tried), which is why I labeled this as "potentially messy"