From f82d32953e15d456acb5b07db53513f355f64216 Mon Sep 17 00:00:00 2001
From: Edward Shen <code@eddie.sh>
Date: Tue, 23 Feb 2021 11:30:48 -0500
Subject: [PATCH] Add key-value delimination event

---
 src/config.rs                     |  73 ++++++++++---
 src/lib.rs                        |   2 +
 src/parser.rs                     | 160 ++++++++++++++++++++++-------
 src/values.rs                     | 165 +++++++++++++++++++++++++++---
 tests/parser_integration_tests.rs |  18 +++-
 5 files changed, 343 insertions(+), 75 deletions(-)

diff --git a/src/config.rs b/src/config.rs
index 7c9a3f0..054df2a 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -1,5 +1,5 @@
 use crate::parser::{parse_from_str, Event, ParsedSectionHeader, Parser, ParserError};
-use serde::Serialize;
+use serde::{ser::SerializeMap, Serialize, Serializer};
 use std::collections::{HashMap, VecDeque};
 use std::{borrow::Cow, fmt::Display};
 
@@ -50,7 +50,9 @@ enum LookupTreeNode<'a> {
 
 impl<'a> GitConfig<'a> {
     /// Convenience constructor. Attempts to parse the provided string into a
-    /// [`GitConfig`].
+    /// [`GitConfig`]. See [`parse_from_str`] for more information.
+    ///
+    /// [`parse_from_str`]: crate::parser::parse_from_str
     pub fn from_str(str: &'a str) -> Result<Self, ParserError> {
         Ok(Self::from_parser(parse_from_str(str)?))
     }
@@ -94,7 +96,8 @@ impl<'a> GitConfig<'a> {
                 e @ Event::Key(_)
                 | e @ Event::Value(_)
                 | e @ Event::ValueNotDone(_)
-                | e @ Event::ValueDone(_) => maybe_section
+                | e @ Event::ValueDone(_)
+                | e @ Event::KeyValueSeparator => maybe_section
                     .as_mut()
                     .expect("Got a section-only event before a section")
                     .push(e),
@@ -542,6 +545,12 @@ impl<'a> GitConfig<'a> {
     }
 }
 
+impl<'a> From<Parser<'a>> for GitConfig<'a> {
+    fn from(p: Parser<'a>) -> Self {
+        Self::from_parser(p)
+    }
+}
+
 impl Display for GitConfig<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         for front_matter in &self.front_matter_events {
@@ -550,20 +559,8 @@ impl Display for GitConfig<'_> {
 
         for section_id in &self.section_order {
             self.section_headers.get(section_id).unwrap().fmt(f)?;
-            let mut found_key = false;
             for event in self.sections.get(section_id).unwrap() {
-                match event {
-                    Event::Key(k) => {
-                        found_key = true;
-                        k.fmt(f)?;
-                    }
-                    Event::Whitespace(w) if found_key => {
-                        found_key = false;
-                        w.fmt(f)?;
-                        write!(f, "=")?;
-                    }
-                    e => e.fmt(f)?,
-                }
+                event.fmt(f)?;
             }
         }
 
@@ -571,6 +568,8 @@ impl Display for GitConfig<'_> {
     }
 }
 
+// todo impl serialize
+
 #[cfg(test)]
 mod from_parser {
     use super::*;
@@ -618,9 +617,11 @@ mod from_parser {
                 vec![
                     Event::Newline(Cow::Borrowed("\n")),
                     Event::Key(Cow::Borrowed("a")),
+                    Event::KeyValueSeparator,
                     Event::Value(Cow::Borrowed("b")),
                     Event::Newline(Cow::Borrowed("\n")),
                     Event::Key(Cow::Borrowed("c")),
+                    Event::KeyValueSeparator,
                     Event::Value(Cow::Borrowed("d")),
                 ],
             );
@@ -665,9 +666,11 @@ mod from_parser {
                 vec![
                     Event::Newline(Cow::Borrowed("\n")),
                     Event::Key(Cow::Borrowed("a")),
+                    Event::KeyValueSeparator,
                     Event::Value(Cow::Borrowed("b")),
                     Event::Newline(Cow::Borrowed("\n")),
                     Event::Key(Cow::Borrowed("c")),
+                    Event::KeyValueSeparator,
                     Event::Value(Cow::Borrowed("d")),
                 ],
             );
@@ -722,9 +725,11 @@ mod from_parser {
                 vec![
                     Event::Newline(Cow::Borrowed("\n")),
                     Event::Key(Cow::Borrowed("a")),
+                    Event::KeyValueSeparator,
                     Event::Value(Cow::Borrowed("b")),
                     Event::Newline(Cow::Borrowed("\n")),
                     Event::Key(Cow::Borrowed("c")),
+                    Event::KeyValueSeparator,
                     Event::Value(Cow::Borrowed("d")),
                     Event::Newline(Cow::Borrowed("\n")),
                 ],
@@ -733,6 +738,7 @@ mod from_parser {
                 SectionId(1),
                 vec![
                     Event::Key(Cow::Borrowed("e")),
+                    Event::KeyValueSeparator,
                     Event::Value(Cow::Borrowed("f")),
                 ],
             );
@@ -786,9 +792,11 @@ mod from_parser {
                 vec![
                     Event::Newline(Cow::Borrowed("\n")),
                     Event::Key(Cow::Borrowed("a")),
+                    Event::KeyValueSeparator,
                     Event::Value(Cow::Borrowed("b")),
                     Event::Newline(Cow::Borrowed("\n")),
                     Event::Key(Cow::Borrowed("c")),
+                    Event::KeyValueSeparator,
                     Event::Value(Cow::Borrowed("d")),
                     Event::Newline(Cow::Borrowed("\n")),
                 ],
@@ -797,6 +805,7 @@ mod from_parser {
                 SectionId(1),
                 vec![
                     Event::Key(Cow::Borrowed("e")),
+                    Event::KeyValueSeparator,
                     Event::Value(Cow::Borrowed("f")),
                 ],
             );
@@ -983,7 +992,6 @@ mod display {
     fn can_reconstruct_non_empty_config() {
         let config = r#"[user]
         email = code@eddie.sh
-        name = Edward Shen
 [core]
         autocrlf = input
 [push]
@@ -1003,4 +1011,35 @@ mod display {
 
         assert_eq!(GitConfig::from_str(config).unwrap().to_string(), config);
     }
+
+    #[test]
+    fn can_reconstruct_configs_with_implicits() {
+        let config = r#"[user]
+        email
+        name
+[core]
+        autocrlf
+[push]
+        default
+[commit]
+        gpgsign"#;
+
+        assert_eq!(GitConfig::from_str(config).unwrap().to_string(), config);
+    }
+
+    #[test]
+    fn can_reconstruct_configs_without_whitespace_in_middle() {
+        let config = r#"[core]
+        autocrlf=input
+[push]
+        default=simple
+[commit]
+        gpgsign=true
+[pull]
+        ff = only
+[init]
+        defaultBranch = master"#;
+
+        assert_eq!(GitConfig::from_str(config).unwrap().to_string(), config);
+    }
 }
diff --git a/src/lib.rs b/src/lib.rs
index a967901..ae2ce2a 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,3 +1,5 @@
+#![forbid(unsafe_code)]
+
 // mod de;
 pub mod config;
 mod error;
diff --git a/src/parser.rs b/src/parser.rs
index f221d97..693ff37 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -21,7 +21,17 @@ use std::borrow::{Borrow, Cow};
 use std::fmt::Display;
 use std::iter::FusedIterator;
 
-/// Syntactic events that occurs in the config.
+/// Syntactic events that occurs in the config. Despite all these variants
+/// holding a [`Cow`] instead over a [`&str`], the parser will only emit
+/// borrowed `Cow` variants.
+///
+/// The `Cow` smart pointer is used here for ease of inserting events in a
+/// middle of an Event iterator. This is used, for example, in the [`GitConfig`]
+/// struct when adding values.
+///
+/// [`Cow`]: std::borrow::Cow
+/// [`&str`]: std::str
+/// [`GitConfig`]: crate::config::GitConfig
 #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
 pub enum Event<'a> {
     /// A comment with a comment tag and the comment itself. Note that the
@@ -52,6 +62,10 @@ pub enum Event<'a> {
     /// A continuous section of insignificant whitespace. Values with internal
     /// spaces will not be separated by this event.
     Whitespace(Cow<'a, str>),
+    /// This event is emitted when the parser counters a valid `=` character
+    /// separating the key and value. This event is necessary as it eliminates
+    /// the ambiguity for whitespace events between a key and value event.
+    KeyValueSeparator,
 }
 
 impl Display for Event<'_> {
@@ -65,6 +79,7 @@ impl Display for Event<'_> {
             Self::ValueNotDone(e) => e.fmt(f),
             Self::ValueDone(e) => e.fmt(f),
             Self::Whitespace(e) => e.fmt(f),
+            Self::KeyValueSeparator => write!(f, "="),
         }
     }
 }
@@ -77,6 +92,7 @@ pub struct ParsedSection<'a> {
     /// The syntactic events found in this section.
     pub events: Vec<Event<'a>>,
 }
+
 impl Display for ParsedSection<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", self.section_header)?;
@@ -240,6 +256,7 @@ impl<'a> From<nom::Err<NomError<&'a str>>> for ParserError<'a> {
 /// Event::Whitespace(Cow::Borrowed("  ")),
 /// Event::Key(Cow::Borrowed("autocrlf")),
 /// Event::Whitespace(Cow::Borrowed(" ")),
+/// Event::KeyValueSeparator,
 /// Event::Whitespace(Cow::Borrowed(" ")),
 /// Event::Value(Cow::Borrowed("input")),
 /// # ]);
@@ -250,6 +267,37 @@ impl<'a> From<nom::Err<NomError<&'a str>>> for ParserError<'a> {
 /// equal sign. So if the config instead had `autocrlf=input`, those whitespace
 /// events would no longer be present.
 ///
+/// ## `KeyValueSeparator` event is not guaranteed to emit
+///
+/// Consider the following `git-config` example:
+///
+/// ```text
+/// [core]
+///   autocrlf
+/// ```
+///
+/// This is a valid config with a `autocrlf` key having an implicit `true`
+/// value. This means that there is not a `=` separating the key and value,
+/// which means that the corresponding event won't appear either:
+///
+/// ```
+/// # use serde_git_config::parser::{Event, ParsedSectionHeader, parse_from_str};
+/// # use std::borrow::Cow;
+/// # let section_header = ParsedSectionHeader {
+/// #   name: Cow::Borrowed("core"),
+/// #   separator: None,
+/// #   subsection_name: None,
+/// # };
+/// # let section_data = "[core]\n  autocrlf";
+/// # assert_eq!(parse_from_str(section_data).unwrap().into_vec(), vec![
+/// Event::SectionHeader(section_header),
+/// Event::Newline(Cow::Borrowed("\n")),
+/// Event::Whitespace(Cow::Borrowed("  ")),
+/// Event::Key(Cow::Borrowed("autocrlf")),
+/// Event::Value(Cow::Borrowed("")),
+/// # ]);
+/// ```
+///
 /// ## Quoted values are not unquoted
 ///
 /// Consider the following `git-config` example:
@@ -279,9 +327,11 @@ impl<'a> From<nom::Err<NomError<&'a str>>> for ParserError<'a> {
 /// Event::SectionHeader(section_header),
 /// Event::Newline(Cow::Borrowed("\n")),
 /// Event::Key(Cow::Borrowed("autocrlf")),
+/// Event::KeyValueSeparator,
 /// Event::Value(Cow::Borrowed(r#"true"""#)),
 /// Event::Newline(Cow::Borrowed("\n")),
 /// Event::Key(Cow::Borrowed("filemode")),
+/// Event::KeyValueSeparator,
 /// Event::Value(Cow::Borrowed(r#"fa"lse""#)),
 /// # ]);
 /// ```
@@ -314,6 +364,7 @@ impl<'a> From<nom::Err<NomError<&'a str>>> for ParserError<'a> {
 /// Event::SectionHeader(section_header),
 /// Event::Newline(Cow::Borrowed("\n")),
 /// Event::Key(Cow::Borrowed("file")),
+/// Event::KeyValueSeparator,
 /// Event::ValueNotDone(Cow::Borrowed("a")),
 /// Event::Newline(Cow::Borrowed("\n")),
 /// Event::ValueDone(Cow::Borrowed("    c")),
@@ -550,15 +601,15 @@ fn config_name<'a>(i: &'a str) -> IResult<&'a str, &'a str> {
 
 fn config_value<'a>(i: &'a str) -> IResult<&'a str, Vec<Event<'a>>> {
     if let (i, Some(_)) = opt(char('='))(i)? {
+        let mut events = vec![];
+        events.push(Event::KeyValueSeparator);
         let (i, whitespace) = opt(take_spaces)(i)?;
         let (i, values) = value_impl(i)?;
         if let Some(whitespace) = whitespace {
-            let mut events = vec![Event::Whitespace(Cow::Borrowed(whitespace))];
-            events.extend(values);
-            Ok((i, events))
-        } else {
-            Ok((i, values))
+            events.push(Event::Whitespace(Cow::Borrowed(whitespace)));
         }
+        events.extend(values);
+        Ok((i, events))
     } else {
         Ok((i, vec![Event::Value(Cow::Borrowed(""))]))
     }
@@ -833,6 +884,37 @@ mod config_name {
     }
 }
 
+#[cfg(test)]
+mod section_body {
+    use super::*;
+
+    #[test]
+    fn whitespace_is_not_ambigious() {
+        assert_eq!(
+            section_body("a =b").unwrap().1,
+            (
+                "a",
+                vec![
+                    Event::Whitespace(Cow::from(" ")),
+                    Event::KeyValueSeparator,
+                    Event::Value(Cow::from("b"))
+                ]
+            )
+        );
+        assert_eq!(
+            section_body("a= b").unwrap().1,
+            (
+                "a",
+                vec![
+                    Event::KeyValueSeparator,
+                    Event::Whitespace(Cow::from(" ")),
+                    Event::Value(Cow::from("b"))
+                ]
+            )
+        );
+    }
+}
+
 #[cfg(test)]
 mod value_no_continuation {
     use super::*;
@@ -841,7 +923,7 @@ mod value_no_continuation {
     fn no_comment() {
         assert_eq!(
             value_impl("hello").unwrap(),
-            fully_consumed(vec![Event::Value(Cow::Borrowed("hello"))])
+            fully_consumed(vec![Event::Value(Cow::from("hello"))])
         );
     }
 
@@ -849,7 +931,7 @@ mod value_no_continuation {
     fn no_comment_newline() {
         assert_eq!(
             value_impl("hello\na").unwrap(),
-            ("\na", vec![Event::Value(Cow::Borrowed("hello"))])
+            ("\na", vec![Event::Value(Cow::from("hello"))])
         )
     }
 
@@ -857,7 +939,7 @@ mod value_no_continuation {
     fn semicolon_comment_not_consumed() {
         assert_eq!(
             value_impl("hello;world").unwrap(),
-            (";world", vec![Event::Value(Cow::Borrowed("hello")),])
+            (";world", vec![Event::Value(Cow::from("hello")),])
         );
     }
 
@@ -865,7 +947,7 @@ mod value_no_continuation {
     fn octothorpe_comment_not_consumed() {
         assert_eq!(
             value_impl("hello#world").unwrap(),
-            ("#world", vec![Event::Value(Cow::Borrowed("hello")),])
+            ("#world", vec![Event::Value(Cow::from("hello")),])
         );
     }
 
@@ -873,10 +955,7 @@ mod value_no_continuation {
     fn values_with_extraneous_whitespace_without_comment() {
         assert_eq!(
             value_impl("hello               ").unwrap(),
-            (
-                "               ",
-                vec![Event::Value(Cow::Borrowed("hello"))]
-            )
+            ("               ", vec![Event::Value(Cow::from("hello"))])
         );
     }
 
@@ -886,14 +965,14 @@ mod value_no_continuation {
             value_impl("hello             #world").unwrap(),
             (
                 "             #world",
-                vec![Event::Value(Cow::Borrowed("hello"))]
+                vec![Event::Value(Cow::from("hello"))]
             )
         );
         assert_eq!(
             value_impl("hello             ;world").unwrap(),
             (
                 "             ;world",
-                vec![Event::Value(Cow::Borrowed("hello"))]
+                vec![Event::Value(Cow::from("hello"))]
             )
         );
     }
@@ -902,10 +981,7 @@ mod value_no_continuation {
     fn trans_escaped_comment_marker_not_consumed() {
         assert_eq!(
             value_impl(r##"hello"#"world; a"##).unwrap(),
-            (
-                "; a",
-                vec![Event::Value(Cow::Borrowed(r##"hello"#"world"##))]
-            )
+            ("; a", vec![Event::Value(Cow::from(r##"hello"#"world"##))])
         );
     }
 
@@ -913,7 +989,7 @@ mod value_no_continuation {
     fn complex_test() {
         assert_eq!(
             value_impl(r#"value";";ahhhh"#).unwrap(),
-            (";ahhhh", vec![Event::Value(Cow::Borrowed(r#"value";""#))])
+            (";ahhhh", vec![Event::Value(Cow::from(r#"value";""#))])
         );
     }
 
@@ -932,9 +1008,9 @@ mod value_continuation {
         assert_eq!(
             value_impl("hello\\\nworld").unwrap(),
             fully_consumed(vec![
-                Event::ValueNotDone(Cow::Borrowed("hello")),
-                Event::Newline(Cow::Borrowed("\n")),
-                Event::ValueDone(Cow::Borrowed("world"))
+                Event::ValueNotDone(Cow::from("hello")),
+                Event::Newline(Cow::from("\n")),
+                Event::ValueDone(Cow::from("world"))
             ])
         );
     }
@@ -944,9 +1020,9 @@ mod value_continuation {
         assert_eq!(
             value_impl("hello\\\n        world").unwrap(),
             fully_consumed(vec![
-                Event::ValueNotDone(Cow::Borrowed("hello")),
-                Event::Newline(Cow::Borrowed("\n")),
-                Event::ValueDone(Cow::Borrowed("        world"))
+                Event::ValueNotDone(Cow::from("hello")),
+                Event::Newline(Cow::from("\n")),
+                Event::ValueDone(Cow::from("        world"))
             ])
         )
     }
@@ -958,11 +1034,11 @@ mod value_continuation {
             (
                 " # \"b\t ; c",
                 vec![
-                    Event::ValueNotDone(Cow::Borrowed(r#"1    "\""#)),
-                    Event::Newline(Cow::Borrowed("\n")),
-                    Event::ValueNotDone(Cow::Borrowed(r#"a ; e "\""#)),
-                    Event::Newline(Cow::Borrowed("\n")),
-                    Event::ValueDone(Cow::Borrowed("d")),
+                    Event::ValueNotDone(Cow::from(r#"1    "\""#)),
+                    Event::Newline(Cow::from("\n")),
+                    Event::ValueNotDone(Cow::from(r#"a ; e "\""#)),
+                    Event::Newline(Cow::from("\n")),
+                    Event::ValueDone(Cow::from("d")),
                 ]
             )
         );
@@ -975,9 +1051,9 @@ mod value_continuation {
             (
                 ";a",
                 vec![
-                    Event::ValueNotDone(Cow::Borrowed("\"")),
-                    Event::Newline(Cow::Borrowed("\n")),
-                    Event::ValueDone(Cow::Borrowed(";\"")),
+                    Event::ValueNotDone(Cow::from("\"")),
+                    Event::Newline(Cow::from("\n")),
+                    Event::ValueDone(Cow::from(";\"")),
                 ]
             )
         )
@@ -1010,10 +1086,11 @@ mod section {
             fully_consumed(ParsedSection {
                 section_header: gen_section_header("hello", None),
                 events: vec![
-                    Event::Newline(Cow::Borrowed("\n")),
-                    Event::Whitespace(Cow::Borrowed("            ")),
-                    Event::Key(Cow::Borrowed("a")),
-                    Event::Whitespace(Cow::Borrowed(" ")),
+                    Event::Newline(Cow::from("\n")),
+                    Event::Whitespace(Cow::from("            ")),
+                    Event::Key(Cow::from("a")),
+                    Event::Whitespace(Cow::from(" ")),
+                    Event::KeyValueSeparator,
                     Event::Whitespace(Cow::Borrowed(" ")),
                     Event::Value(Cow::Borrowed("b")),
                     Event::Newline(Cow::Borrowed("\n")),
@@ -1024,6 +1101,7 @@ mod section {
                     Event::Whitespace(Cow::Borrowed("            ")),
                     Event::Key(Cow::Borrowed("d")),
                     Event::Whitespace(Cow::Borrowed(" ")),
+                    Event::KeyValueSeparator,
                     Event::Whitespace(Cow::Borrowed(" ")),
                     Event::Value(Cow::Borrowed("\"lol\""))
                 ]
@@ -1067,6 +1145,7 @@ mod section {
                     Event::Whitespace(Cow::Borrowed("            ")),
                     Event::Key(Cow::Borrowed("a")),
                     Event::Whitespace(Cow::Borrowed(" ")),
+                    Event::KeyValueSeparator,
                     Event::Whitespace(Cow::Borrowed(" ")),
                     Event::Value(Cow::Borrowed("b")),
                     Event::Whitespace(Cow::Borrowed(" ")),
@@ -1090,6 +1169,7 @@ mod section {
                     Event::Whitespace(Cow::Borrowed("            ")),
                     Event::Key(Cow::Borrowed("c")),
                     Event::Whitespace(Cow::Borrowed(" ")),
+                    Event::KeyValueSeparator,
                     Event::Whitespace(Cow::Borrowed(" ")),
                     Event::Value(Cow::Borrowed("d")),
                 ]
@@ -1108,6 +1188,7 @@ mod section {
                     Event::Whitespace(Cow::Borrowed(" ")),
                     Event::Key(Cow::Borrowed("a")),
                     Event::Whitespace(Cow::Borrowed(" ")),
+                    Event::KeyValueSeparator,
                     Event::Whitespace(Cow::Borrowed(" ")),
                     Event::ValueNotDone(Cow::Borrowed(r#"1    "\""#)),
                     Event::Newline(Cow::Borrowed("\n")),
@@ -1134,6 +1215,7 @@ mod section {
                     Event::Whitespace(Cow::Borrowed(" ")),
                     Event::Key(Cow::Borrowed("b")),
                     Event::Whitespace(Cow::Borrowed(" ")),
+                    Event::KeyValueSeparator,
                     Event::ValueNotDone(Cow::Borrowed("\"")),
                     Event::Newline(Cow::Borrowed("\n")),
                     Event::ValueDone(Cow::Borrowed(";\"")),
diff --git a/src/values.rs b/src/values.rs
index f2a05f8..ea1c999 100644
--- a/src/values.rs
+++ b/src/values.rs
@@ -16,9 +16,9 @@ impl<'a> Value<'a> {
             return Self::Boolean(bool);
         }
 
-        // if let Ok(int) = Integer::from_str(s) {
-        //     return Self::Integer(int);
-        // }
+        if let Ok(int) = Integer::from_str(s) {
+            return Self::Integer(int);
+        }
 
         // if let Ok(color) = Color::from_str(s) {
         //     return Self::Color(color);
@@ -32,6 +32,8 @@ impl<'a> Value<'a> {
     }
 }
 
+// todo display for value
+
 impl Serialize for Value<'_> {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -46,8 +48,6 @@ impl Serialize for Value<'_> {
     }
 }
 
-// todo display for value
-
 #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
 pub enum Boolean<'a> {
     True(TrueVariant<'a>),
@@ -108,9 +108,10 @@ impl<'a> TrueVariant<'a> {
 
 impl Display for TrueVariant<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Self::Explicit(v) => write!(f, "{}", v),
-            Self::Implicit => write!(f, "(implicit)"),
+        if let Self::Explicit(v) = self {
+            write!(f, "{}", v)
+        } else {
+            Ok(())
         }
     }
 }
@@ -163,12 +164,6 @@ pub struct Integer {
     suffix: Option<IntegerSuffix>,
 }
 
-impl Integer {
-    pub fn from_str(s: &str) -> Result<Self, ()> {
-        todo!()
-    }
-}
-
 impl Display for Integer {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", self.value)?;
@@ -193,7 +188,34 @@ impl Serialize for Integer {
     }
 }
 
-// todo from str for integer
+impl FromStr for Integer {
+    type Err = String;
+
+    fn from_str<'a>(s: &'a str) -> Result<Self, Self::Err> {
+        if let Ok(value) = s.parse() {
+            return Ok(Self {
+                value,
+                suffix: None,
+            });
+        }
+
+        // Assume we have a prefix at this point.
+
+        if s.len() <= 1 {
+            return Err(s.to_string());
+        }
+
+        let (number, suffix) = s.split_at(s.len() - 1);
+        if let (Ok(value), Ok(suffix)) = (number.parse(), suffix.parse()) {
+            Ok(Self {
+                value,
+                suffix: Some(suffix),
+            })
+        } else {
+            Err(s.to_string())
+        }
+    }
+}
 
 #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
 enum IntegerSuffix {
@@ -248,7 +270,7 @@ impl FromStr for IntegerSuffix {
     }
 }
 
-#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
+#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Default)]
 pub struct Color {
     foreground: Option<ColorValue>,
     background: Option<ColorValue>,
@@ -283,7 +305,54 @@ impl Serialize for Color {
     }
 }
 
-// impl fromstr for color
+pub enum FromColorErr {
+    TooManyColorValues,
+    InvalidColorOption,
+}
+
+impl FromStr for Color {
+    type Err = FromColorErr;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        enum ColorItem {
+            Value(ColorValue),
+            Attr(ColorAttribute),
+        }
+
+        let items = s.split_whitespace().filter_map(|s| {
+            if s.is_empty() {
+                return None;
+            }
+
+            Some(
+                ColorValue::from_str(s)
+                    .map(ColorItem::Value)
+                    .or_else(|_| ColorAttribute::from_str(s).map(ColorItem::Attr)),
+            )
+        });
+
+        let mut new_self = Self::default();
+        for item in items {
+            match item {
+                Ok(item) => match item {
+                    ColorItem::Value(v) => {
+                        if new_self.foreground.is_none() {
+                            new_self.foreground = Some(v);
+                        } else if new_self.background.is_none() {
+                            new_self.background = Some(v);
+                        } else {
+                            return Err(FromColorErr::TooManyColorValues);
+                        }
+                    }
+                    ColorItem::Attr(a) => new_self.attributes.push(a),
+                },
+                Err(_) => return Err(FromColorErr::InvalidColorOption),
+            }
+        }
+
+        Ok(new_self)
+    }
+}
 
 #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
 enum ColorValue {
@@ -484,3 +553,65 @@ impl FromStr for ColorAttribute {
         }
     }
 }
+
+#[cfg(test)]
+mod integer {
+    use super::*;
+
+    #[test]
+    fn from_str_no_suffix() {
+        assert_eq!(
+            Integer::from_str("1").unwrap(),
+            Integer {
+                value: 1,
+                suffix: None
+            }
+        );
+
+        assert_eq!(
+            Integer::from_str("-1").unwrap(),
+            Integer {
+                value: -1,
+                suffix: None
+            }
+        );
+    }
+
+    #[test]
+    fn from_str_with_suffix() {
+        assert_eq!(
+            Integer::from_str("1k").unwrap(),
+            Integer {
+                value: 1,
+                suffix: Some(IntegerSuffix::Kilo),
+            }
+        );
+
+        assert_eq!(
+            Integer::from_str("1m").unwrap(),
+            Integer {
+                value: 1,
+                suffix: Some(IntegerSuffix::Mega),
+            }
+        );
+
+        assert_eq!(
+            Integer::from_str("1g").unwrap(),
+            Integer {
+                value: 1,
+                suffix: Some(IntegerSuffix::Giga),
+            }
+        );
+    }
+
+    #[test]
+    fn invalid_from_str() {
+        assert!(Integer::from_str("").is_err());
+        assert!(Integer::from_str("-").is_err());
+        assert!(Integer::from_str("k").is_err());
+        assert!(Integer::from_str("m").is_err());
+        assert!(Integer::from_str("g").is_err());
+        assert!(Integer::from_str("123123123123123123123123").is_err());
+        assert!(Integer::from_str("gg").is_err());
+    }
+}
diff --git a/tests/parser_integration_tests.rs b/tests/parser_integration_tests.rs
index adea606..221aa91 100644
--- a/tests/parser_integration_tests.rs
+++ b/tests/parser_integration_tests.rs
@@ -39,12 +39,16 @@ fn whitespace(value: &'static str) -> Event<'static> {
     Event::Whitespace(Cow::Borrowed(value))
 }
 
+fn separator() -> Event<'static> {
+    Event::KeyValueSeparator
+}
+
 #[test]
 #[rustfmt::skip]
 fn personal_config() {
     let config = r#"[user]
         email = code@eddie.sh
-        name = Edward Shen
+        name = Foo Bar
 [core]
         autocrlf = input
 [push]
@@ -73,6 +77,7 @@ fn personal_config() {
             whitespace("        "),
             name("email"),
             whitespace(" "),
+            separator(),
             whitespace(" "),
             value("code@eddie.sh"),
             newline(),
@@ -80,8 +85,9 @@ fn personal_config() {
             whitespace("        "),
             name("name"),
             whitespace(" "),
+            separator(),
             whitespace(" "),
-            value("Edward Shen"),
+            value("Foo Bar"),
             newline(),
 
             gen_section_header("core", None),
@@ -90,6 +96,7 @@ fn personal_config() {
             whitespace("        "),
             name("autocrlf"),
             whitespace(" "),
+            separator(),
             whitespace(" "),
             value("input"),
             newline(),
@@ -100,6 +107,7 @@ fn personal_config() {
             whitespace("        "),
             name("default"),
             whitespace(" "),
+            separator(),
             whitespace(" "),
             value("simple"),
             newline(),
@@ -110,6 +118,7 @@ fn personal_config() {
             whitespace("        "),
             name("gpgsign"),
             whitespace(" "),
+            separator(),
             whitespace(" "),
             value("true"),
             newline(),
@@ -120,6 +129,7 @@ fn personal_config() {
             whitespace("        "),
             name("program"),
             whitespace(" "),
+            separator(),
             whitespace(" "),
             value("gpg"),
             newline(),
@@ -130,6 +140,7 @@ fn personal_config() {
             whitespace("        "),
             name("insteadOf"),
             whitespace(" "),
+            separator(),
             whitespace(" "),
             value("\"github://\""),
             newline(),
@@ -140,6 +151,7 @@ fn personal_config() {
             whitespace("        "),
             name("insteadOf"),
             whitespace(" "),
+            separator(),
             whitespace(" "),
             value("\"gitea://\""),
             newline(),
@@ -150,6 +162,7 @@ fn personal_config() {
             whitespace("        "),
             name("ff"),
             whitespace(" "),
+            separator(),
             whitespace(" "),
             value("only"),
             newline(),
@@ -160,6 +173,7 @@ fn personal_config() {
             whitespace("        "),
             name("defaultBranch"),
             whitespace(" "),
+            separator(),
             whitespace(" "),
             value("master"),
         ]