refactor(from_trait): impl From trait properly

Author: kyu08

src/file/toml.rs

@@ -44,44 +44,33 @@ impl Histories {
         Self { histories }
     }
 
-    // TODO: impl as From trait
-    fn from(histories: histories::Histories) -> Self {
-        let mut result: Vec<History> = vec![];
-        for h in histories.histories {
-            result.push(History::from(h));
-        }
-        Self { histories: result }
-    }
-
     pub fn into(self) -> histories::Histories {
         let mut result: Vec<histories::History> = vec![];
         for h in self.histories {
-            result.push(History::into(h));
+            result.push(h.into());
         }
         histories::Histories { histories: result }
     }
 }
 
+impl From<histories::Histories> for Histories {
+    fn from(histories: histories::Histories) -> Histories {
+        let mut result: Vec<History> = vec![];
+        for h in histories.histories {
+            result.push(History::from(h));
+        }
+        Self { histories: result }
+    }
+}
+
 #[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
 pub struct History {
     path: PathBuf,
     commands: Vec<HistoryCommand>,
 }
 
 impl History {
-    // TODO: impl as From trait
-    fn from(history: histories::History) -> Self {
-        let mut commands: Vec<HistoryCommand> = vec![];
-        for h in history.commands {
-            commands.push(HistoryCommand::from(h));
-        }
-
-        History {
-            path: history.path,
-            commands,
-        }
-    }
-
+    // Implementing From<file::toml::History> on the model::History side is not desirable due to dependency direction, so the into method is manually defined.
     fn into(self) -> histories::History {
         let mut commands: Vec<histories::HistoryCommand> = vec![];
         for h in self.commands {
@@ -99,6 +88,20 @@ impl History {
     }
 }
 
+impl From<histories::History> for History {
+    fn from(history: histories::History) -> History {
+        let mut commands: Vec<HistoryCommand> = vec![];
+        for h in history.commands {
+            commands.push(HistoryCommand::from(h));
+        }
+
+        History {
+            path: history.path,
+            commands,
+        }
+    }
+}
+
 /// toml representation of histories::HistoryCommand.
 #[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
 #[serde(rename_all = "kebab-case")]
@@ -112,14 +115,6 @@ impl HistoryCommand {
         Self { runner_type, args }
     }
 
-    // TODO: impl as From trait
-    fn from(command: histories::HistoryCommand) -> Self {
-        Self {
-            runner_type: command.runner_type,
-            args: command.args.clone(),
-        }
-    }
-
     fn into(self) -> histories::HistoryCommand {
         histories::HistoryCommand {
             runner_type: self.runner_type,
@@ -128,6 +123,15 @@ impl HistoryCommand {
     }
 }
 
+impl From<histories::HistoryCommand> for HistoryCommand {
+    fn from(command: histories::HistoryCommand) -> HistoryCommand {
+        Self {
+            runner_type: command.runner_type,
+            args: command.args.clone(),
+        }
+    }
+}
+
 // TODO: should return Result not Option(returns when it fails to get the home dir)
 pub fn history_file_path() -> Option<(PathBuf, String)> {
     const HISTORY_FILE_NAME: &str = "history.toml";

src/model/runner_type.rs

@@ -22,25 +22,13 @@ pub enum JsPackageManager {
 impl RunnerType {
     pub fn to_runner(&self, runners: &Vec<runner::Runner>) -> Option<runner::Runner> {
         for r in runners {
-            if self.clone() == RunnerType::from(r) {
+            if self.clone() == RunnerType::from(r.clone()) {
                 return Some(r.clone());
             }
         }
         None
     }
 
-    // TODO: impl as From trait
-    pub fn from(runner: &runner::Runner) -> Self {
-        match runner {
-            runner::Runner::MakeCommand(_) => RunnerType::Make,
-            runner::Runner::JsPackageManager(js) => match js {
-                js::JsPackageManager::JsPnpm(_) => RunnerType::JsPackageManager(JsPackageManager::Pnpm),
-                js::JsPackageManager::JsYarn(_) => RunnerType::JsPackageManager(JsPackageManager::Yarn),
-            },
-            runner::Runner::Just(_) => RunnerType::Just,
-        }
-    }
-
     pub fn get_extension_for_highlighting(&self) -> &str {
         match self {
             RunnerType::Make => "mk",
@@ -52,6 +40,19 @@ impl RunnerType {
     }
 }
 
+impl From<runner::Runner> for RunnerType {
+    fn from(runner: runner::Runner) -> RunnerType {
+        match runner {
+            runner::Runner::MakeCommand(_) => RunnerType::Make,
+            runner::Runner::JsPackageManager(js) => match js {
+                js::JsPackageManager::JsPnpm(_) => RunnerType::JsPackageManager(JsPackageManager::Pnpm),
+                js::JsPackageManager::JsYarn(_) => RunnerType::JsPackageManager(JsPackageManager::Yarn),
+            },
+            runner::Runner::Just(_) => RunnerType::Just,
+        }
+    }
+}
+
 impl fmt::Display for RunnerType {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let name = match self {