poise
poise copied to clipboard
Get rid of autoref-specialisation
This significantly simplifies poise internals, by removing the fancy autoderef specialization used to fallback from the poise traits, to the serenity traits, and finally to FromStr.
It is replaced by normal implementations, which does not significantly impact usability as users may define newtypes to implement the traits themselves if needed.
This PR is currently missing:
- testing to make sure I didn't break anything
What sort of runtime testing is required for this?
What sort of runtime testing is required for this?
I'm guessing just testing if all the current things that parse still parse the same way, check the existing impls and cross reference i guess?
If that all works it should be fine
I'm not even sure what the specialization was trying to solve. Was it a workaround for trying to write a blanket implementation of impl<T: FromStr> SlashArgument for T in order to avoid overlapping impls (and likewise for PopArgument)?
Yes.
Tested this out a bit and it looks great other than the review comment, haven't had any other issues.
Tested this out a bit and it looks great other than the review comment, haven't had any other issues.
Can you test to verify the behavior change around string arguments, where whitespace is now trimmed?
Superseded by https://github.com/serenity-rs/poise/pull/357