Move FromStr implementations to non-overloaded methdos

FromStr doesn't indicate what the string format should be, it's too generic. There can also be multiple string encodings of some of the types. Move implementations to separate methods with more descriptive names. In general, we should avoid overloading unless absolutely necessary (e.g. when calling a library code with a trait bond).
osa1 · Dec 31, 2023 · cd63933 · cd63933
1 parent 3cc3df1
commit cd63933
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 76 deletions.
diff --git a/crates/libtiny_tui/src/config.rs b/crates/libtiny_tui/src/config.rs
@@ -6,9 +6,8 @@ use serde::de::{self, Deserializer, MapAccess, Visitor};
 use serde::Deserialize;
 use std::collections::HashMap;
 use std::path::Path;
-use std::str::FromStr;
 
-pub use termbox_simple::*;
+use termbox_simple::*;
 
 use crate::key_map::KeyMap;
 use crate::notifier::Notifier;
@@ -73,28 +72,7 @@ pub enum Chan {
 }
 
 impl Chan {
-    pub fn name(&self) -> &ChanNameRef {
-        match self {
-            Chan::Name(name) => name,
-            Chan::WithConfig { name, .. } => name,
-        }
-        .as_ref()
-    }
-}
-
-fn deser_chan_name<'de, D>(d: D) -> Result<ChanName, D::Error>
-where
-    D: Deserializer<'de>,
-{
-    let name: String = serde::de::Deserialize::deserialize(d)?;
-    Ok(ChanName::new(name))
-}
-
-// `FromStr` implementation used when parsing channel names in commands (not in config file).
-impl FromStr for Chan {
-    type Err = String;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+    pub fn from_cmd_args(s: &str) -> Result<Chan, String> {
         // Make sure channel starts with '#'
         let s = if !s.starts_with('#') {
             format!("#{}", s)
@@ -105,7 +83,7 @@ impl FromStr for Chan {
         match s.split_once(' ') {
             // with args
             Some((name, args)) => {
-                let config = TabConfig::from_str(args)?;
+                let config = TabConfig::from_cmd_args(args)?;
                 Ok(Chan::WithConfig {
                     name: ChanName::new(name.to_string()),
                     config,
@@ -115,6 +93,22 @@ impl FromStr for Chan {
             None => Ok(Chan::Name(ChanName::new(s))),
         }
     }
+
+    pub fn name(&self) -> &ChanNameRef {
+        match self {
+            Chan::Name(name) => name,
+            Chan::WithConfig { name, .. } => name,
+        }
+        .as_ref()
+    }
+}
+
+fn deser_chan_name<'de, D>(d: D) -> Result<ChanName, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let name: String = serde::de::Deserialize::deserialize(d)?;
+    Ok(ChanName::new(name))
 }
 
 /// Map of TabConfigs by tab names
@@ -205,32 +199,7 @@ pub struct TabConfig {
 }
 
 impl TabConfig {
-    pub fn user_tab_defaults() -> TabConfig {
-        TabConfig {
-            ignore: Some(false),
-            notify: Some(Notifier::Messages),
-        }
-    }
-
-    pub(crate) fn or_use(&self, config: &TabConfig) -> TabConfig {
-        TabConfig {
-            ignore: self.ignore.or(config.ignore),
-            notify: self.notify.or(config.notify),
-        }
-    }
-
-    pub(crate) fn toggle_ignore(&mut self) -> bool {
-        let ignore = self.ignore.get_or_insert(false);
-        *ignore = !&*ignore;
-        *ignore
-    }
-}
-
-// `FromStr` implementation used when parsing channel names in commands (not in config file).
-impl FromStr for TabConfig {
-    type Err = String;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+    pub(crate) fn from_cmd_args(s: &str) -> Result<TabConfig, String> {
         let mut config = TabConfig::default();
         let mut words = s.trim().split(' ').map(str::trim);
 
@@ -243,7 +212,7 @@ impl FromStr for TabConfig {
                 "-ignore" => config.ignore = Some(true),
                 "-notify" => match words.next() {
                     Some(notify_setting) => {
-                        config.notify = Some(Notifier::from_str(notify_setting)?)
+                        config.notify = Some(Notifier::from_cmd_args(notify_setting)?)
                     }
                     None => return Err("-notify parameter missing".to_string()),
                 },
@@ -253,6 +222,19 @@ impl FromStr for TabConfig {
 
         Ok(config)
     }
+
+    pub(crate) fn or_use(&self, config: &TabConfig) -> TabConfig {
+        TabConfig {
+            ignore: self.ignore.or(config.ignore),
+            notify: self.notify.or(config.notify),
+        }
+    }
+
+    pub(crate) fn toggle_ignore(&mut self) -> bool {
+        let ignore = self.ignore.get_or_insert(false);
+        *ignore = !&*ignore;
+        *ignore
+    }
 }
 
 fn default_max_nick_length() -> usize {

diff --git a/crates/libtiny_tui/src/notifier.rs b/crates/libtiny_tui/src/notifier.rs
@@ -1,5 +1,3 @@
-use std::str::FromStr;
-
 use crate::MsgTarget;
 
 use libtiny_wire::formatting::remove_irc_control_chars;
@@ -32,19 +30,6 @@ impl Default for Notifier {
     }
 }
 
-impl FromStr for Notifier {
-    type Err = String;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        match s.to_lowercase().as_str() {
-            "off" => Ok(Notifier::Off),
-            "mentions" => Ok(Notifier::Mentions),
-            "messages" => Ok(Notifier::Messages),
-            _ => Err(format!("Unknown Notifier variant: {}", s)),
-        }
-    }
-}
-
 #[cfg(feature = "desktop-notifications")]
 fn notify(summary: &str, body: &str) {
     // TODO: Report errors somehow
@@ -55,6 +40,15 @@ fn notify(summary: &str, body: &str) {
 fn notify(_summary: &str, _body: &str) {}
 
 impl Notifier {
+    pub(crate) fn from_cmd_args(s: &str) -> Result<Notifier, String> {
+        match s.to_lowercase().as_str() {
+            "off" => Ok(Notifier::Off),
+            "mentions" => Ok(Notifier::Mentions),
+            "messages" => Ok(Notifier::Messages),
+            _ => Err(format!("Unknown Notifier variant: {}", s)),
+        }
+    }
+
     pub(crate) fn notify_privmsg(
         &mut self,
         sender: &str,

diff --git a/crates/libtiny_tui/src/tests/config.rs b/crates/libtiny_tui/src/tests/config.rs
@@ -3,8 +3,6 @@ use libtiny_common::{ChanName, ChanNameRef};
 use crate::config::*;
 use crate::notifier::Notifier;
 
-use std::str::FromStr;
-
 #[test]
 fn parsing_tab_configs() {
     let config_str = r##"
@@ -104,35 +102,35 @@ fn parsing_tab_configs() {
 #[test]
 fn tab_config_command() {
     assert_eq!(
-        TabConfig::from_str("").unwrap(),
+        TabConfig::from_cmd_args("").unwrap(),
         TabConfig {
             ignore: None,
             notify: None
         }
     );
     assert_eq!(
-        TabConfig::from_str("-ignore").unwrap(),
+        TabConfig::from_cmd_args("-ignore").unwrap(),
         TabConfig {
             ignore: Some(true),
             notify: None
         }
     );
     assert_eq!(
-        TabConfig::from_str("-notify off").unwrap(),
+        TabConfig::from_cmd_args("-notify off").unwrap(),
         TabConfig {
             ignore: None,
             notify: Some(Notifier::Off)
         }
     );
     assert_eq!(
-        TabConfig::from_str("-notify off -ignore").unwrap(),
+        TabConfig::from_cmd_args("-notify off -ignore").unwrap(),
         TabConfig {
             ignore: Some(true),
             notify: Some(Notifier::Off)
         }
     );
     assert_eq!(
-        TabConfig::from_str("-ignore -notify off").unwrap(),
+        TabConfig::from_cmd_args("-ignore -notify off").unwrap(),
         TabConfig {
             ignore: Some(true),
             notify: Some(Notifier::Off)

diff --git a/crates/tiny/src/cmd.rs b/crates/tiny/src/cmd.rs
@@ -6,7 +6,6 @@ use libtiny_common::{ChanNameRef, MsgSource, MsgTarget};
 use libtiny_tui::config::Chan;
 
 use std::borrow::Borrow;
-use std::str::FromStr;
 
 pub(crate) fn run_cmd(
     cmd: &str,
@@ -320,7 +319,7 @@ fn join(args: CmdArgs) {
     let chans = args
         .split(',')
         .map(str::trim)
-        .filter_map(|c| match Chan::from_str(c) {
+        .filter_map(|c| match Chan::from_cmd_args(c) {
             Ok(c) => Some(c),
             Err(err) => {
                 ui.add_client_err_msg(&err, &MsgTarget::CurrentTab);