make serde optional, clippy lints

.gitignore

@@ -1,2 +1,3 @@
 /target
 Cargo.lock
+tarpaulin-report.html

Cargo.toml

@@ -1,5 +1,5 @@
 [package]
-name = "serde-git-config"
+name = "git-config"
 version = "0.1.0"
 authors = ["Edward Shen <code@eddie.sh>"]
 edition = "2018"
@@ -7,9 +7,16 @@ edition = "2018"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-serde = "1.0"
 nom = "6"
 bstr = "0.2.15"
 
+[dependencies.serde_crate]
+package = "serde"
+optional = true
+version = " 1"
+
 [dev-dependencies]
 serde_derive = "1.0"
+
+[features]
+serde = ["serde_crate"]

src/config.rs

@@ -336,7 +336,7 @@ impl<'a> GitConfig<'a> {
             .map(|vec| {
                 // get_section_ids_by_name_and_subname is guaranteed to return
                 // a non-empty vec, so max can never return empty.
-                *vec.into_iter().max().unwrap()
+                *vec.iter().max().unwrap()
             })
     }
 
@@ -517,7 +517,7 @@ impl<'a> GitConfig<'a> {
         if let Some(subsect_name) = subsection_name {
             for node in section_ids {
                 if let LookupTreeNode::NonTerminal(subsection_lookup) = node {
-                    maybe_ids = subsection_lookup.get(subsect_name.into());
+                    maybe_ids = subsection_lookup.get(subsect_name);
                     break;
                 }
             }

src/error.rs

@@ -1,5 +1,6 @@
 use std::fmt::{self, Display};
 
+#[cfg(feature = "serde")]
 use serde::{de, ser};
 
 pub type Result<T> = std::result::Result<T, Error>;
@@ -16,12 +17,14 @@ pub enum Error {
     InvalidBoolean(String),
 }
 
+#[cfg(feature = "serde")]
 impl ser::Error for Error {
     fn custom<T: Display>(msg: T) -> Self {
         Error::Message(msg.to_string())
     }
 }
 
+#[cfg(feature = "serde")]
 impl de::Error for Error {
     fn custom<T: Display>(msg: T) -> Self {
         Error::Message(msg.to_string())

src/lib.rs

@@ -1,5 +1,8 @@
 #![forbid(unsafe_code)]
 
+#[cfg(feature = "serde")]
+extern crate serde_crate as serde;
+
 // mod de;
 pub mod config;
 mod error;

src/parser.rs

@@ -472,7 +472,7 @@ impl<'a> Parser<'a> {
 /// data succeeding valid `git-config` data.
 pub fn parse_from_str(input: &str) -> Result<Parser<'_>, ParserError> {
     let (i, frontmatter) = many0(alt((
-        map(comment, |comment| Event::Comment(comment)),
+        map(comment, Event::Comment),
         map(take_spaces, |whitespace| {
             Event::Whitespace(Cow::Borrowed(whitespace.into()))
         }),
@@ -492,7 +492,7 @@ pub fn parse_from_str(input: &str) -> Result<Parser<'_>, ParserError> {
     })
 }
 
-fn comment<'a>(i: &'a [u8]) -> IResult<&'a [u8], ParsedComment<'a>> {
+fn comment(i: &[u8]) -> IResult<&[u8], ParsedComment> {
     let (i, comment_tag) = one_of(";#")(i)?;
     let (i, comment) = take_till(|c| c == b'\n')(i)?;
     Ok((
@@ -504,7 +504,7 @@ fn comment<'a>(i: &'a [u8]) -> IResult<&'a [u8], ParsedComment<'a>> {
     ))
 }
 
-fn section<'a>(i: &'a [u8]) -> IResult<&'a [u8], ParsedSection<'a>> {
+fn section(i: &[u8]) -> IResult<&[u8], ParsedSection> {
     let (i, section_header) = section_header(i)?;
     let (i, items) = many0(alt((
         map(take_spaces, |space| {
@@ -529,7 +529,7 @@ fn section<'a>(i: &'a [u8]) -> IResult<&'a [u8], ParsedSection<'a>> {
     ))
 }
 
-fn section_header<'a>(i: &'a [u8]) -> IResult<&'a [u8], ParsedSectionHeader<'a>> {
+fn section_header(i: &[u8]) -> IResult<&[u8], ParsedSectionHeader> {
     let (i, _) = char('[')(i)?;
     // No spaces must be between section name and section start
     let (i, name) = take_while(|c: u8| c.is_ascii_alphanumeric() || c == b'-' || c == b'.')(i)?;
@@ -581,7 +581,7 @@ fn section_header<'a>(i: &'a [u8]) -> IResult<&'a [u8], ParsedSectionHeader<'a>>
     ))
 }
 
-fn section_body<'a>(i: &'a [u8]) -> IResult<&'a [u8], (&'a [u8], Vec<Event<'a>>)> {
+fn section_body(i: &[u8]) -> IResult<&[u8], (&[u8], Vec<Event>)> {
     // maybe need to check for [ here
     let (i, name) = config_name(i)?;
     let (i, whitespace) = opt(take_spaces)(i)?;
@@ -597,7 +597,7 @@ fn section_body<'a>(i: &'a [u8]) -> IResult<&'a [u8], (&'a [u8], Vec<Event<'a>>)
 
 /// Parses the config name of a config pair. Assumes the input has already been
 /// trimmed of any leading whitespace.
-fn config_name<'a>(i: &'a [u8]) -> IResult<&'a [u8], &'a [u8]> {
+fn config_name(i: &[u8]) -> IResult<&[u8], &[u8]> {
     if i.is_empty() {
         return Err(nom::Err::Error(NomError {
             input: i,
@@ -615,7 +615,7 @@ fn config_name<'a>(i: &'a [u8]) -> IResult<&'a [u8], &'a [u8]> {
     take_while(|c: u8| (c as char).is_alphanumeric() || c == b'-')(i)
 }
 
-fn config_value<'a>(i: &'a [u8]) -> IResult<&'a [u8], Vec<Event<'a>>> {
+fn config_value(i: &[u8]) -> IResult<&[u8], Vec<Event>> {
     if let (i, Some(_)) = opt(char('='))(i)? {
         let mut events = vec![];
         events.push(Event::KeyValueSeparator);
@@ -631,7 +631,7 @@ fn config_value<'a>(i: &'a [u8]) -> IResult<&'a [u8], Vec<Event<'a>>> {
     }
 }
 
-fn value_impl<'a>(i: &'a [u8]) -> IResult<&'a [u8], Vec<Event<'a>>> {
+fn value_impl(i: &[u8]) -> IResult<&[u8], Vec<Event>> {
     let mut events = vec![];
     let mut parsed_index: usize = 0;
     let mut offset: usize = 0;
@@ -722,11 +722,11 @@ fn value_impl<'a>(i: &'a [u8]) -> IResult<&'a [u8], Vec<Event<'a>>> {
     Ok((i, events))
 }
 
-fn take_spaces<'a>(i: &'a [u8]) -> IResult<&'a [u8], &'a [u8]> {
+fn take_spaces(i: &[u8]) -> IResult<&[u8], &[u8]> {
     take_common(i, |c| (c as char).is_ascii() && is_space(c))
 }
 
-fn take_newline<'a>(i: &'a [u8]) -> IResult<&'a [u8], &'a [u8]> {
+fn take_newline(i: &[u8]) -> IResult<&[u8], &[u8]> {
     take_common(i, is_char_newline)
 }
 
@@ -734,7 +734,7 @@ fn is_char_newline(c: u8) -> bool {
     (c as char).is_ascii() && is_newline(c)
 }
 
-fn take_common<'a, F: Fn(u8) -> bool>(i: &'a [u8], f: F) -> IResult<&'a [u8], &'a [u8]> {
+fn take_common<F: Fn(u8) -> bool>(i: &[u8], f: F) -> IResult<&[u8], &[u8]> {
     let (i, v) = take_while(f)(i)?;
     if v.is_empty() {
         Err(nom::Err::Error(NomError {

src/test_util.rs

@@ -57,10 +57,6 @@ pub(crate) fn whitespace_event(value: &'static str) -> Event<'static> {
     Event::Whitespace(Cow::Borrowed(value.into()))
 }
 
-pub(crate) fn separator_event() -> Event<'static> {
-    Event::KeyValueSeparator
-}
-
 pub(crate) fn comment_event(tag: char, msg: &'static str) -> Event<'static> {
     Event::Comment(comment(tag, msg))
 }

src/values.rs

@@ -1,5 +1,6 @@
 use std::{borrow::Cow, fmt::Display, str::FromStr};
 
+#[cfg(feature = "serde")]
 use serde::{Serialize, Serializer};
 
 #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
@@ -20,9 +21,9 @@ impl<'a> Value<'a> {
             return Self::Integer(int);
         }
 
-        // if let Ok(color) = Color::from_str(s) {
-        //     return Self::Color(color);
-        // }
+        if let Ok(color) = Color::from_str(s) {
+            return Self::Color(color);
+        }
 
         Self::Other(Cow::Borrowed(s))
     }
@@ -34,6 +35,7 @@ impl<'a> Value<'a> {
 
 // todo display for value
 
+#[cfg(feature = "serde")]
 impl Serialize for Value<'_> {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -68,8 +70,16 @@ impl<'a> Boolean<'a> {
     }
 }
 
-// todo: Display for boolean
+impl Display for Boolean<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Boolean::True(v) => v.fmt(f),
+            Boolean::False(v) => v.fmt(f),
+        }
+    }
+}
 
+#[cfg(feature = "serde")]
 impl Serialize for Boolean<'_> {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -116,6 +126,7 @@ impl Display for TrueVariant<'_> {
     }
 }
 
+#[cfg(feature = "serde")]
 impl Serialize for TrueVariant<'_> {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -149,6 +160,7 @@ impl Display for FalseVariant<'_> {
     }
 }
 
+#[cfg(feature = "serde")]
 impl Serialize for FalseVariant<'_> {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -175,6 +187,7 @@ impl Display for Integer {
     }
 }
 
+#[cfg(feature = "serde")]
 impl Serialize for Integer {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -191,7 +204,7 @@ impl Serialize for Integer {
 impl FromStr for Integer {
     type Err = String;
 
-    fn from_str<'a>(s: &'a str) -> Result<Self, Self::Err> {
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
         if let Ok(value) = s.parse() {
             return Ok(Self {
                 value,
@@ -244,6 +257,7 @@ impl Display for IntegerSuffix {
     }
 }
 
+#[cfg(feature = "serde")]
 impl Serialize for IntegerSuffix {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -291,16 +305,17 @@ impl Display for Color {
 
         self.attributes
             .iter()
-            .map(|attr| write!(f, " ").and_then(|_| attr.fmt(f)))
-            .collect::<Result<_, _>>()
+            .try_for_each(|attr| write!(f, " ").and_then(|_| attr.fmt(f)))
     }
 }
 
+#[cfg(feature = "serde")]
 impl Serialize for Color {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
     {
+        // todo: maybe not?
         serializer.serialize_str(&self.to_string())
     }
 }
@@ -403,6 +418,7 @@ impl Display for ColorValue {
     }
 }
 
+#[cfg(feature = "serde")]
 impl Serialize for ColorValue {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -416,9 +432,17 @@ impl FromStr for ColorValue {
     type Err = ();
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let bright = s.starts_with("bright");
+        let mut s = s;
+        let bright = if s.starts_with("bright") {
+            s = &s[6..];
+            true
+        } else {
+            false
+        };
+
         match s {
-            "normal" => return Ok(Self::Normal),
+            "normal" if !bright => return Ok(Self::Normal),
+            "normal" if bright => return Err(()),
             "black" if !bright => return Ok(Self::Black),
             "black" if bright => return Ok(Self::BrightBlack),
             "red" if !bright => return Ok(Self::Red),
@@ -442,17 +466,16 @@ impl FromStr for ColorValue {
             return Ok(Self::Ansi(v));
         }
 
-        if s.starts_with("#") {
-            let s = &s[1..];
+        if let Some(s) = s.strip_prefix('#') {
             if s.len() == 6 {
                 let rgb = (
                     u8::from_str_radix(&s[..2], 16),
                     u8::from_str_radix(&s[2..4], 16),
                     u8::from_str_radix(&s[4..], 16),
                 );
-                match rgb {
-                    (Ok(r), Ok(g), Ok(b)) => return Ok(Self::Rgb(r, g, b)),
-                    _ => (),
+
+                if let (Ok(r), Ok(g), Ok(b)) = rgb {
+                    return Ok(Self::Rgb(r, g, b));
                 }
             }
         }
@@ -500,6 +523,7 @@ impl Display for ColorAttribute {
     }
 }
 
+#[cfg(feature = "serde")]
 impl Serialize for ColorAttribute {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -534,7 +558,7 @@ impl FromStr for ColorAttribute {
         if inverted {
             parsed = &parsed[2..];
 
-            if parsed.starts_with("-") {
+            if parsed.starts_with('-') {
                 parsed = &parsed[1..];
             }
         }
@@ -621,6 +645,91 @@ mod integer {
     }
 }
 
+#[cfg(test)]
+mod color_value {
+    use super::ColorValue;
+    use std::str::FromStr;
+
+    #[test]
+    fn non_bright() {
+        assert_eq!(ColorValue::from_str("normal"), Ok(ColorValue::Normal));
+        assert_eq!(ColorValue::from_str("black"), Ok(ColorValue::Black));
+        assert_eq!(ColorValue::from_str("red"), Ok(ColorValue::Red));
+        assert_eq!(ColorValue::from_str("green"), Ok(ColorValue::Green));
+        assert_eq!(ColorValue::from_str("yellow"), Ok(ColorValue::Yellow));
+        assert_eq!(ColorValue::from_str("blue"), Ok(ColorValue::Blue));
+        assert_eq!(ColorValue::from_str("magenta"), Ok(ColorValue::Magenta));
+        assert_eq!(ColorValue::from_str("cyan"), Ok(ColorValue::Cyan));
+        assert_eq!(ColorValue::from_str("white"), Ok(ColorValue::White));
+    }
+
+    #[test]
+    fn bright() {
+        assert_eq!(
+            ColorValue::from_str("brightblack"),
+            Ok(ColorValue::BrightBlack)
+        );
+        assert_eq!(ColorValue::from_str("brightred"), Ok(ColorValue::BrightRed));
+        assert_eq!(
+            ColorValue::from_str("brightgreen"),
+            Ok(ColorValue::BrightGreen)
+        );
+        assert_eq!(
+            ColorValue::from_str("brightyellow"),
+            Ok(ColorValue::BrightYellow)
+        );
+        assert_eq!(
+            ColorValue::from_str("brightblue"),
+            Ok(ColorValue::BrightBlue)
+        );
+        assert_eq!(
+            ColorValue::from_str("brightmagenta"),
+            Ok(ColorValue::BrightMagenta)
+        );
+        assert_eq!(
+            ColorValue::from_str("brightcyan"),
+            Ok(ColorValue::BrightCyan)
+        );
+        assert_eq!(
+            ColorValue::from_str("brightwhite"),
+            Ok(ColorValue::BrightWhite)
+        );
+    }
+
+    #[test]
+    fn ansi() {
+        assert_eq!(ColorValue::from_str("255"), Ok(ColorValue::Ansi(255)));
+        assert_eq!(ColorValue::from_str("0"), Ok(ColorValue::Ansi(0)));
+    }
+
+    #[test]
+    fn hex() {
+        assert_eq!(
+            ColorValue::from_str("#ff0010"),
+            Ok(ColorValue::Rgb(255, 0, 16))
+        );
+        assert_eq!(
+            ColorValue::from_str("#ffffff"),
+            Ok(ColorValue::Rgb(255, 255, 255))
+        );
+        assert_eq!(
+            ColorValue::from_str("#000000"),
+            Ok(ColorValue::Rgb(0, 0, 0))
+        );
+    }
+
+    #[test]
+    fn invalid() {
+        assert!(ColorValue::from_str("brightnormal").is_err());
+        assert!(ColorValue::from_str("").is_err());
+        assert!(ColorValue::from_str("bright").is_err());
+        assert!(ColorValue::from_str("256").is_err());
+        assert!(ColorValue::from_str("#").is_err());
+        assert!(ColorValue::from_str("#fff").is_err());
+        assert!(ColorValue::from_str("#gggggg").is_err());
+    }
+}
+
 #[cfg(test)]
 mod color_attribute {
     use super::ColorAttribute;

tests/parser_integration_tests.rs

@@ -1,6 +1,6 @@
 use std::borrow::Cow;
 
-use serde_git_config::parser::{parse_from_str, Event, ParsedSectionHeader};
+use git_config::parser::{parse_from_str, Event, ParsedSectionHeader};
 
 pub fn section_header_event(
     name: &str,