Merge pull request #34 from ksk001100/feature/33

refactoring
ksk001100 · Feb 27, 2020 · 9ad61b6 · 9ad61b6
2 parents 2513e53 + c9897cb
commit 9ad61b6
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 28 deletions.
diff --git a/README.md b/README.md
@@ -87,10 +87,10 @@ fn hello_command() -> Command {
 
 ```bash
 $ cargo run
-$ cargo run John --bye
-$ cargo run John --age 30
-$ cargo run John -b -a 30
-$ cargo run John --age=30
+$ cargo run hello John --bye
+$ cargo run hello John --age 30
+$ cargo run hello John -b -a 30
+$ cargo run hello John --age=30
 ```
 
 ### Single action application

diff --git a/src/app.rs b/src/app.rs
@@ -230,7 +230,7 @@ impl App {
                     }
                 };
 
-                if cmd.len() < 1 {
+                if cmd.is_empty() {
                     self.help();
                     return;
                 }
@@ -251,7 +251,6 @@ impl App {
             }
             AppType::Empty => {
                 self.help();
-                return;
             }
             AppType::Undefined => {
                 // TODO: I want to be able to check if there is a problem with the combination at compile time in the future (compile_error macro...)
@@ -310,7 +309,7 @@ impl App {
                     for flag in flags {
                         writeln!(out, "\t{}", flag.usage).unwrap();
                     }
-                    write!(out, "\n").unwrap();
+                    writeln!(out).unwrap();
                 }
                 None => (),
             },
@@ -321,9 +320,9 @@ impl App {
 
     /// Select command
     /// Gets the Command that matches the string passed in the argument
-    fn select_command(&self, cmd: &String) -> Option<&Command> {
+    fn select_command(&self, cmd: &str) -> Option<&Command> {
         match &self.commands {
-            Some(commands) => commands.iter().find(|command| &command.name == cmd),
+            Some(commands) => commands.iter().find(|command| command.name == cmd),
             None => None,
         }
     }

diff --git a/src/command.rs b/src/command.rs
@@ -106,7 +106,7 @@ impl Command {
     /// Run command
     /// Call this function only from `App`
     pub fn run(&self, v: Vec<String>) {
-        (self.action)(&Context::new(v.clone(), self.flags.clone()))
+        (self.action)(&Context::new(v, self.flags.clone()))
     }
 }
 

diff --git a/src/context.rs b/src/context.rs
@@ -53,7 +53,7 @@ impl Context {
     fn option_flag_value(&self, name: &str) -> Option<&FlagValue> {
         self.flags
             .as_ref()
-            .and_then(|flags| flags.into_iter().find(|flag| flag.0 == name.to_string()))
+            .and_then(|flags| flags.iter().find(|flag| flag.0 == name))
             .and_then(|flag| flag.1.as_ref())
     }
 
@@ -75,7 +75,7 @@ impl Context {
     /// ```
     pub fn bool_flag(&self, name: &str) -> bool {
         match self.option_flag_value(name) {
-            Some(FlagValue::Bool(val)) => (*val).clone(),
+            Some(FlagValue::Bool(val)) => *val,
             _ => false,
         }
     }
@@ -99,7 +99,7 @@ impl Context {
     /// ```
     pub fn string_flag(&self, name: &str) -> Option<String> {
         match self.option_flag_value(name) {
-            Some(FlagValue::String(val)) => Some((&val).to_string()),
+            Some(FlagValue::String(val)) => Some(val.to_string()),
             _ => None,
         }
     }
@@ -123,7 +123,7 @@ impl Context {
     /// ```
     pub fn int_flag(&self, name: &str) -> Option<isize> {
         match self.option_flag_value(name) {
-            Some(FlagValue::Int(val)) => Some(val.clone()),
+            Some(FlagValue::Int(val)) => Some(*val),
             _ => None,
         }
     }
@@ -147,7 +147,7 @@ impl Context {
     /// ```
     pub fn float_flag(&self, name: &str) -> Option<f64> {
         match self.option_flag_value(name) {
-            Some(FlagValue::Float(val)) => Some(val.clone()),
+            Some(FlagValue::Float(val)) => Some(*val),
             _ => None,
         }
     }

diff --git a/src/flag.rs b/src/flag.rs
@@ -93,30 +93,23 @@ impl Flag {
         self
     }
 
-    /// Get flag position from `Vec<String>` command line argument
-    pub fn option_index(&self, v: &Vec<String>) -> Option<usize> {
+    /// Get flag position from command line argument
+    pub fn option_index(&self, v: &[String]) -> Option<usize> {
         match &self.alias {
             Some(alias) => v.iter().position(|r| {
-                alias
-                    .iter()
-                    .map(|a| format!("-{}", a))
-                    .collect::<Vec<String>>()
-                    .contains(r)
-                    || r == &format!("--{}", &self.name)
+                r == &format!("--{}", &self.name) || alias.iter().any(|a| r == &format!("-{}", a))
             }),
             None => v.iter().position(|r| r == &format!("--{}", &self.name)),
         }
     }
 
     /// Get flag value
-    pub fn value(&self, v: &Vec<String>) -> Option<FlagValue> {
+    pub fn value(&self, v: &[String]) -> Option<FlagValue> {
         match self.flag_type {
             FlagType::Bool => match &self.alias {
                 Some(alias) => Some(FlagValue::Bool(
-                    alias
-                        .iter()
-                        .map(|a| v.contains(&format!("-{}", a)))
-                        .any(|b| b == true || v.contains(&format!("--{}", self.name))),
+                    v.contains(&format!("--{}", self.name))
+                        || alias.iter().any(|a| v.contains(&format!("-{}", a))),
                 )),
                 None => Some(FlagValue::Bool(v.contains(&format!("--{}", self.name)))),
             },
@@ -140,6 +133,29 @@ impl Flag {
 mod tests {
     use crate::{Flag, FlagType, FlagValue};
 
+    #[test]
+    fn opiton_index() {
+        let v = vec![
+            "cli".to_string(),
+            "command".to_string(),
+            "-a".to_string(),
+            "--bool".to_string(),
+            "-c".to_string(),
+        ];
+        {
+            let f = Flag::new("bool", "", FlagType::Bool);
+            assert_eq!(f.option_index(&v), Some(3));
+        }
+        {
+            let f = Flag::new("age", "", FlagType::Bool).alias("a");
+            assert_eq!(f.option_index(&v), Some(2));
+        }
+        {
+            let f = Flag::new("dance", "", FlagType::Bool);
+            assert_eq!(f.option_index(&v), None);
+        }
+    }
+
     #[test]
     #[should_panic]
     fn construct_fail_1() {