Allow serenity model types in argument parsing system about framework HOT 6 CLOSED

The custom trait would also probably have to be async.

Probably, yes.

For 1, would it be a good idea to implement the custom trait for a few commonly used standard library types like the ints, floats, bool, String, etc., and some serenity models, and let the user implement the trait for other types they need?

With a blanket implementation impl<T: FromStr> ArgumentParse for T { ... }, all types with FromStr implemented would automatically have ArgumentParse implemented as well. This includes ints, floats, bool, String, even IpAddr, and also all third-party types with a FromStr implementation, for example url::Url or regex::Regex.

Anyways, I started to implement the trait but got hit by orphan rules complications:

error[E0119]: conflicting implementations of trait `argument::ArgumentParse` for type `serenity::model::guild::Member`:
  --> /home/kangalioo/dev/rust/_downloaded/serenity-framework/framework/src/argument.rs:34:1
   |
19 | impl<T: std::str::FromStr> ArgumentParse for T {
   | ---------------------------------------------- first implementation here
...
34 | impl ArgumentParse for Member {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `serenity::model::guild::Member`
   |
   = note: upstream crates may add a new impl of trait `std::str::FromStr` for type `serenity::model::guild::Member` in future versions

The only viable way to workaround this is to have the ArgumentParse trait and its implementations in the serenity crate itself (alternatives would be either waiting for specialization, or manually implementing ArgumentParse for all types, both of which have significant issues).

Does someone have a better idea? Otherwise I'd go ahead and create PRs for the serenity crate, and after that, for this crate to integrate with the new trait.

from framework.

To comment on the 1st way, if we went with that approach it would deny implementing the ArgumentParse trait on types that have a FromStr implementation, as they would conflict with one another. Specialisation would fix this, but it's currently unstable and unlikely to stabilise soon.

from framework.

Isn't it redundant anyways to implement the ArgumentParse trait on types with a FromStr implementation, when the blanket implementation exists?

from framework.

Yes, but if people wanted separate implementations for FromStr and ArgumentParse, this would prevent them from doing so. It's not a big concern, I'm just pointing an issue 1 could cause.

from framework.

I just realized that 2 is impossible, actually. Some parsing functions need extra context, for example parsing Member needs information about the members in a guild. Therefore, a custom trait is actually required, and it would probably need to look something like this:

pub trait ArgumentParse {
    type Err;
    fn parse(ctx: &crate::context::Context, msg: &Message, s: &str) -> Result<Self, Self::Err>;
}

impl<T: std::str::FromStr> ArgumentParse for T {
    type Err = <T as std::str::FromStr>::Err;
    fn parse(_ctx: &crate::context::Context, _msg: &Message, s: &str) -> Result<Self, Self::Err> {
        s.parse()
    }
}

from framework.

The custom trait would also probably have to be async.

For 1, would it be a good idea to implement the custom trait for a few commonly used standard library types like the ints, floats, bool, String, etc., and some serenity models, and let the user implement the trait for other types they need?

from framework.