Revamp errors in `Args` and commands

6 files changed, 141 insertions, 86 deletions

diff --git a/src/framework/standard/args.rs b/src/framework/standard/args.rs
index d22b1a8..d03076f 100644
--- a/src/framework/standard/args.rs
+++ b/src/framework/standard/args.rs
@@ -1,21 +1,59 @@
 use vec_shift::Shift;
 use std::str::FromStr;
 use std::error::Error as StdError;
+use std::fmt;
 use utils::parse_quotes;
 
 /// Defines how an operation on an `Args` method failed.
 #[derive(Debug)]
-pub enum Error {
+pub enum Error<E: StdError> {
     /// "END-OF-STRING", more precisely, there isn't anything to parse anymore.
     Eos,
     /// A parsing operation failed; the error in it can be of any returned from the `FromStr`
     /// trait.
-    Parse(Box<StdError>),
+    Parse(E),
 }
 
-type Result<T> = ::std::result::Result<T, Error>;
+impl<E: StdError> From<E> for Error<E> {
+    fn from(e: E) -> Self {
+        Error::Parse(e)
+    }
+} 
+
+impl<E: StdError> StdError for Error<E> {
+    fn description(&self) -> &str {
+        use self::Error::*;
+
+        match *self {
+            Eos => "end-of-string",
+            Parse(ref e) => e.description(),
+        }
+    }
+
+    fn cause(&self) -> Option<&StdError> {
+        use self::Error::*;
+
+        match *self {
+            Parse(ref e) => Some(e),
+            _ => None,
+        }
+    }
+}
 
-#[derive(Debug, Clone)]
+impl<E: StdError> fmt::Display for Error<E> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use self::Error::*;
+
+        match *self {
+            Eos => write!(f, "end of string"),
+            Parse(ref e) => fmt::Display::fmt(&e, f),
+        }
+    }
+}
+
+type Result<T, E> = ::std::result::Result<T, Error<E>>;
+
+#[derive(Clone, Debug)]
 pub struct Args {
     delimiter: String,
     delimiter_split: Vec<String>,
@@ -36,33 +74,31 @@ impl Args {
     }
 
     /// Removes the first element, parses it to a specific type if necessary, returns.
-    pub fn single<T: FromStr>(&mut self) -> Result<T>
-        where T::Err: StdError + 'static {
+    pub fn single<T: FromStr>(&mut self) -> Result<T, T::Err>
+        where T::Err: StdError {
         if self.delimiter_split.is_empty() {
             return Err(Error::Eos);
         }
 
-        self.delimiter_split
+        Ok(self.delimiter_split
             .shift()
             .ok_or(Error::Eos)?
-            .parse::<T>()
-            .map_err(|e| Error::Parse(Box::new(e)))
+            .parse::<T>()?)
     }
 
     /// Like [`single`], but doesn't remove the element.
     ///
     /// [`single`]: #method.single
-    pub fn single_n<T: FromStr>(&self) -> Result<T>
-        where T::Err: StdError + 'static {
+    pub fn single_n<T: FromStr>(&self) -> Result<T, T::Err>
+        where T::Err: StdError {
         if self.delimiter_split.is_empty() {
             return Err(Error::Eos);
         }
 
-        self.delimiter_split
+        Ok(self.delimiter_split
             .get(0)
             .ok_or(Error::Eos)?
-            .parse::<T>()
-            .map_err(|e| Error::Parse(Box::new(e)))
+            .parse::<T>()?)
     }
 
     /// Skips if there's a first element, but also returns it.
@@ -87,50 +123,48 @@ impl Args {
     /// Like [`single`], but takes quotes into account.
     ///
     /// [`single`]: #method.single
-    pub fn single_quoted<T: FromStr>(&mut self) -> Result<T>
-        where T::Err: StdError + 'static {
-        parse_quotes(&self.delimiter_split.shift().ok_or(Error::Eos)?)
+    pub fn single_quoted<T: FromStr>(&mut self) -> Result<T, T::Err>
+        where T::Err: StdError {
+        Ok(parse_quotes(&self.delimiter_split.shift().ok_or(Error::Eos)?)
             .remove(0)
-            .parse::<T>()
-            .map_err(|e| Error::Parse(Box::new(e)))
+            .parse::<T>()?)
     }
 
     /// Like [`single_quoted`], but doesn't remove the element.
     ///
     /// [`single_quoted`]: #method.single_quoted
-    pub fn single_quoted_n<T: FromStr>(&self) -> Result<T>
-        where T::Err: StdError + 'static {
-        parse_quotes(&self.delimiter_split.get(0).ok_or(Error::Eos)?)
+    pub fn single_quoted_n<T: FromStr>(&self) -> Result<T, T::Err>
+        where T::Err: StdError {
+        Ok(parse_quotes(&self.delimiter_split.get(0).ok_or(Error::Eos)?)
             .remove(0)
-            .parse::<T>()
-            .map_err(|e| Error::Parse(Box::new(e)))
+            .parse::<T>()?)
     }
 
     /// Like [`list`], but takes quotes into account.
     ///
     /// [`list`]: #method.list
-    pub fn multiple_quoted<T: FromStr>(self) -> Result<Vec<T>>
-        where T::Err: StdError + 'static {
+    pub fn multiple_quoted<T: FromStr>(self) -> Result<Vec<T>, T::Err>
+        where T::Err: StdError {
         if self.delimiter_split.is_empty() {
             return Err(Error::Eos);
         }
 
         parse_quotes(&self.delimiter_split.join(&self.delimiter))
             .into_iter()
-            .map(|s| s.parse::<T>().map_err(|e| Error::Parse(Box::new(e))))
+            .map(|s| s.parse::<T>().map_err(|e| Error::Parse(e)))
             .collect()
     }
 
     /// Empty outs the internal vector while parsing (if necessary) and returning them
-    pub fn list<T: FromStr>(self) -> Result<Vec<T>>
-        where T::Err: StdError + 'static {
+    pub fn list<T: FromStr>(self) -> Result<Vec<T>, T::Err>
+        where T::Err: StdError {
         if self.delimiter_split.is_empty() {
             return Err(Error::Eos);
         }
 
         self.delimiter_split
             .into_iter()
-            .map(|s| s.parse::<T>().map_err(|e| Error::Parse(Box::new(e))))
+            .map(|s| s.parse::<T>().map_err(|e| Error::Parse(e)))
             .collect()
     }
 
@@ -138,8 +172,8 @@ impl Args {
     pub fn full(&self) -> String { self.delimiter_split.join(&self.delimiter) }
 
     /// Returns the first argument that can be converted and removes it from the list.
-    pub fn find<T: FromStr>(&mut self) -> Result<T>
-        where T::Err: StdError + 'static {
+    pub fn find<T: FromStr>(&mut self) -> Result<T, T::Err>
+        where T::Err: StdError {
         if self.delimiter_split.is_empty() {
             return Err(Error::Eos);
         }
@@ -152,30 +186,28 @@ impl Args {
                 let value = self.delimiter_split
                     .get(index)
                     .ok_or(Error::Eos)?
-                    .parse::<T>()
-                    .map_err(|e| Error::Parse(Box::new(e)));
+                    .parse::<T>()?;
 
                 self.delimiter_split.remove(index);
 
-                value
+                Ok(value)
             },
             _ => Err(Error::Eos),
         }
     }
 
     /// Returns the first argument that can be converted and does not remove it from the list.
-    pub fn find_n<T: FromStr>(&self) -> Result<T>
-        where T::Err: StdError + 'static {
+    pub fn find_n<T: FromStr>(&self) -> Result<T, T::Err>
+        where T::Err: StdError {
         if self.delimiter_split.is_empty() {
             return Err(Error::Eos);
         }
 
-        self.delimiter_split
+        Ok(self.delimiter_split
             .iter()
             .find(|e| e.parse::<T>().is_ok())
             .ok_or(Error::Eos)?
-            .parse::<T>()
-            .map_err(|e| Error::Parse(Box::new(e)))
+            .parse::<T>()?)
     }
 }
 
diff --git a/src/framework/standard/command.rs b/src/framework/standard/command.rs
index 96c2f7d..aa8f7d8 100644
--- a/src/framework/standard/command.rs
+++ b/src/framework/standard/command.rs
@@ -3,19 +3,21 @@ use super::{Args, Configuration};
 use client::Context;
 use model::{Message, Permissions};
 use std::collections::HashMap;
+use std::error::Error as StdError;
+use std::fmt;
 
 pub type Check = Fn(&mut Context, &Message, &mut Args, &Arc<Command>) -> bool
                      + Send
                      + Sync
                      + 'static;
-pub type Exec = Fn(&mut Context, &Message, Args) -> Result<(), String> + Send + Sync + 'static;
+pub type Exec = Fn(&mut Context, &Message, Args) -> Result<(), Error> + Send + Sync + 'static;
 pub type Help = Fn(&mut Context, &Message, HashMap<String, Arc<CommandGroup>>, Args)
-                   -> Result<(), String>
+                   -> Result<(), Error>
                     + Send
                     + Sync
                     + 'static;
 pub type BeforeHook = Fn(&mut Context, &Message, &str) -> bool + Send + Sync + 'static;
-pub type AfterHook = Fn(&mut Context, &Message, &str, Result<(), String>) + Send + Sync + 'static;
+pub type AfterHook = Fn(&mut Context, &Message, &str, Result<(), Error>) + Send + Sync + 'static;
 pub(crate) type InternalCommand = Arc<Command>;
 pub type PrefixCheck = Fn(&mut Context, &Message) -> Option<String> + Send + Sync + 'static;
 
@@ -24,6 +26,40 @@ pub enum CommandOrAlias {
     Command(InternalCommand),
 }
 
+/// An error from a command.
+#[derive(Clone, Debug)]
+pub struct Error(String);
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Display::fmt(&self.0, f)
+    }
+}
+
+impl<E: StdError> From<E> for Error {
+    fn from(e: E) -> Self {
+        Error(format!("{}", e))
+    }
+}
+
+impl From<Custom> for Error {
+    fn from(Custom(e): Custom) -> Self {
+        Error(e)
+    }
+}
+
+/// A custom "variant" of [`Error`]
+/// 
+/// [`Error`]: #struct.Error.html
+#[derive(Clone, Debug)]
+pub struct Custom(String);
+
+impl From<String> for Custom {
+    fn from(e: String) -> Custom {
+        Custom(e)
+    }
+}
+
 /// Command function type. Allows to access internal framework things inside
 /// your commands.
 pub enum CommandType {
@@ -81,22 +117,31 @@ pub struct Command {
 
 impl Command {
     pub fn new<F>(f: F) -> Self
-        where F: Fn(&mut Context, &Message, Args) -> Result<(), String> + Send + Sync + 'static {
+        where F: Fn(&mut Context, &Message, Args) -> Result<(), Error> + Send + Sync + 'static {
+        Command {
+            exec: CommandType::Basic(Box::new(f)),
+            ..Command::default()
+        }
+    }
+}
+
+impl Default for Command {
+    fn default() -> Command {
         Command {
             aliases: Vec::new(),
             checks: Vec::default(),
-            exec: CommandType::Basic(Box::new(f)),
+            exec: CommandType::Basic(Box::new(|_, _, _| Ok(()))),
             desc: None,
             usage: None,
             example: None,
-            dm_only: false,
+            min_args: None,
             bucket: None,
+            max_args: None,
+            required_permissions: Permissions::empty(),
+            dm_only: false,
             guild_only: false,
             help_available: true,
-            min_args: None,
-            max_args: None,
             owners_only: false,
-            required_permissions: Permissions::empty(),
             allowed_roles: Vec::new(),
         }
     }
diff --git a/src/framework/standard/create_command.rs b/src/framework/standard/create_command.rs
index 30ef491..a6068da 100644
--- a/src/framework/standard/create_command.rs
+++ b/src/framework/standard/create_command.rs
@@ -1,7 +1,6 @@
-pub use super::{Args, Command, CommandGroup, CommandType};
+pub use super::{Args, Command, CommandGroup, CommandType, CommandError};
 
 use std::collections::HashMap;
-use std::default::Default;
 use std::sync::Arc;
 use client::Context;
 use model::{Message, Permissions};
@@ -103,7 +102,7 @@ impl CreateCommand {
     ///
     /// [`exec_str`]: #method.exec_str
     pub fn exec<F>(mut self, func: F) -> Self
-        where F: Fn(&mut Context, &Message, Args) -> Result<(), String> + Send + Sync + 'static {
+        where F: Fn(&mut Context, &Message, Args) -> Result<(), CommandError> + Send + Sync + 'static {
         self.0.exec = CommandType::Basic(Box::new(func));
 
         self
@@ -113,10 +112,10 @@ impl CreateCommand {
     /// the internal HashMap of commands, used specifically for creating a help
     /// command.
     ///
-    /// You can return `Err(string)` if there's an error.
+    /// You can return `Err(Custom(string))` if there's an error.
     pub fn exec_help<F>(mut self, f: F) -> Self
         where F: Fn(&mut Context, &Message, HashMap<String, Arc<CommandGroup>>, Args)
-                    -> Result<(), String>
+                    -> Result<(), CommandError>
                      + Send
                      + Sync
                      + 'static {
@@ -215,24 +214,3 @@ impl CreateCommand {
     }
 }
 
-impl Default for Command {
-    fn default() -> Command {
-        Command {
-            aliases: Vec::new(),
-            checks: Vec::default(),
-            exec: CommandType::Basic(Box::new(|_, _, _| Ok(()))),
-            desc: None,
-            usage: None,
-            example: None,
-            min_args: None,
-            bucket: None,
-            max_args: None,
-            required_permissions: Permissions::empty(),
-            dm_only: false,
-            guild_only: false,
-            help_available: true,
-            owners_only: false,
-            allowed_roles: Vec::new(),
-        }
-    }
-}
diff --git a/src/framework/standard/create_group.rs b/src/framework/standard/create_group.rs
index d4e2d15..699e56f 100644
--- a/src/framework/standard/create_group.rs
+++ b/src/framework/standard/create_group.rs
@@ -1,4 +1,4 @@
-pub use super::command::{Command, CommandGroup, CommandType};
+pub use super::command::{Command, CommandGroup, CommandType, Error as CommandError};
 pub(crate) use super::command::CommandOrAlias;
 pub use super::create_command::CreateCommand;
 pub use super::Args;
@@ -72,7 +72,7 @@ impl CreateGroup {
     /// Adds a command to group with simplified API.
     /// You can return Err(string) if there's an error.
     pub fn on<F>(mut self, command_name: &str, f: F) -> Self
-        where F: Fn(&mut Context, &Message, Args) -> Result<(), String> + Send + Sync + 'static {
+        where F: Fn(&mut Context, &Message, Args) -> Result<(), CommandError> + Send + Sync + 'static {
         let cmd = Arc::new(Command::new(f));
 
         self.0
diff --git a/src/framework/standard/help_commands.rs b/src/framework/standard/help_commands.rs
index a14f510..c25b227 100644
--- a/src/framework/standard/help_commands.rs
+++ b/src/framework/standard/help_commands.rs
@@ -27,7 +27,7 @@ use std::collections::HashMap;
 use std::sync::Arc;
 use std::fmt::Write;
 use super::command::InternalCommand;
-use super::{Args, Command, CommandGroup, CommandOrAlias};
+use super::{Args, Command, CommandGroup, CommandOrAlias, CommandError};
 use client::Context;
 use model::{ChannelId, Guild, Member, Message};
 use utils::Colour;
@@ -83,7 +83,7 @@ pub fn with_embeds(_: &mut Context,
                    msg: &Message,
                    groups: HashMap<String, Arc<CommandGroup>>,
                    args: Args)
-                   -> Result<(), String> {
+                   -> Result<(), CommandError> {
     if !args.is_empty() {
         let name = args.full();
 
@@ -256,7 +256,7 @@ pub fn plain(_: &mut Context,
              msg: &Message,
              groups: HashMap<String, Arc<CommandGroup>>,
              args: Args)
-             -> Result<(), String> {
+             -> Result<(), CommandError> {
     if !args.is_empty() {
         let name = args.full();
 
diff --git a/src/framework/standard/mod.rs b/src/framework/standard/mod.rs
index 17e2fdd..112f4d1 100644
--- a/src/framework/standard/mod.rs
+++ b/src/framework/standard/mod.rs
@@ -9,7 +9,7 @@ mod buckets;
 mod args;
 
 pub(crate) use self::buckets::{Bucket, Ratelimit};
-pub use self::command::{Command, CommandGroup, CommandType};
+pub use self::command::{Command, CommandGroup, CommandType, Error as CommandError};
 pub use self::command::CommandOrAlias;
 pub use self::configuration::Configuration;
 pub use self::create_command::CreateCommand;
@@ -71,7 +71,7 @@ macro_rules! command {
         pub fn $fname(mut $c: &mut $crate::client::Context,
                       _: &$crate::model::Message,
                       _: $crate::framework::standard::Args)
-                      -> ::std::result::Result<(), String> {
+                      -> ::std::result::Result<(), CommandError> {
             $b
 
             Ok(())
@@ -82,7 +82,7 @@ macro_rules! command {
         pub fn $fname(mut $c: &mut $crate::client::Context,
                       $m: &$crate::model::Message,
                       _: $crate::framework::standard::Args)
-                      -> ::std::result::Result<(), String> {
+                      -> ::std::result::Result<(), CommandError> {
             $b
 
             Ok(())
@@ -93,7 +93,7 @@ macro_rules! command {
         pub fn $fname(mut $c: &mut $crate::client::Context,
                       $m: &$crate::model::Message,
                       mut $a: $crate::framework::standard::Args)
-                      -> ::std::result::Result<(), String> {
+                      -> ::std::result::Result<(), CommandError> {
             $b
 
             Ok(())
@@ -597,7 +597,7 @@ impl StandardFramework {
     /// # }
     /// ```
     pub fn on<F, S>(mut self, command_name: S, f: F) -> Self
-        where F: Fn(&mut Context, &Message, Args) -> Result<(), String> + Send + Sync + 'static,
+        where F: Fn(&mut Context, &Message, Args) -> Result<(), CommandError> + Send + Sync + 'static,
               S: Into<String> {
         {
             let ungrouped = self.groups
@@ -822,7 +822,7 @@ impl StandardFramework {
     ///     }));
     /// ```
     pub fn after<F>(mut self, f: F) -> Self
-        where F: Fn(&mut Context, &Message, &str, Result<(), String>) + Send + Sync + 'static {
+        where F: Fn(&mut Context, &Message, &str, Result<(), CommandError>) + Send + Sync + 'static {
         self.after = Some(Arc::new(f));
 
         self