finish raw value queries for gitconfig

src/config.rs

@@ -13,8 +13,11 @@ enum LookupTreeNode<'a> {
 
 #[derive(Debug, PartialEq, Eq)]
 pub enum GitConfigError<'a> {
+    /// The requested section does not exist.
     SectionDoesNotExist(&'a str),
+    /// The requested subsection does not exist.
     SubSectionDoesNotExist(Option<&'a str>),
+    /// The key does not exist in the requested section.
     KeyDoesNotExist(&'a str),
 }
 
@@ -50,10 +53,6 @@ impl<'a> GitConfig<'a> {
 
         for event in parser.into_iter() {
             match event {
-                e @ Event::Comment(_) => match maybe_section {
-                    Some(ref mut section) => section.push(e),
-                    None => new_self.front_matter_events.push(e),
-                },
                 Event::SectionHeader(header) => {
                     new_self.push_section(
                         &mut current_section_name,
@@ -73,33 +72,24 @@ impl<'a> GitConfig<'a> {
                         .section_header_separators
                         .insert(SectionId(new_self.section_id_counter), header.separator);
                 }
-                e @ Event::Key(_) => maybe_section
+                e @ Event::Key(_)
+                | e @ Event::Value(_)
+                | e @ Event::ValueNotDone(_)
+                | e @ Event::ValueDone(_) => maybe_section
                     .as_mut()
                     .expect("Got a section-only event before a section")
                     .push(e),
-                e @ Event::Value(_) => maybe_section
+                e @ Event::Comment(_) | e @ Event::Newline(_) | e @ Event::Whitespace(_) => {
-                    .as_mut()
+                    match maybe_section {
-                    .expect("Got a section-only event before a section")
-                    .push(e),
-                e @ Event::Newline(_) => match maybe_section {
                         Some(ref mut section) => section.push(e),
                         None => new_self.front_matter_events.push(e),
-                },
+                    }
-                e @ Event::ValueNotDone(_) => maybe_section
+                }
-                    .as_mut()
-                    .expect("Got a section-only event before a section")
-                    .push(e),
-                e @ Event::ValueDone(_) => maybe_section
-                    .as_mut()
-                    .expect("Got a section-only event before a section")
-                    .push(e),
-                e @ Event::Whitespace(_) => match maybe_section {
-                    Some(ref mut section) => section.push(e),
-                    None => new_self.front_matter_events.push(e),
-                },
             }
         }
 
+        // The last section doesn't get pushed since we only push if there's a
+        // new section header, so we need to call push one more time.
         new_self.push_section(
             &mut current_section_name,
             &mut current_subsection_name,
@@ -178,11 +168,14 @@ impl<'a> GitConfig<'a> {
     /// ```
     /// # use serde_git_config::config::GitConfig;
     /// # let git_config = GitConfig::from_str("[core]a=b\n[core]\na=c\na=d").unwrap();
-    /// assert_eq!(git_config.get_raw_single_value("core", None, "a"), Ok("d"));
+    /// assert_eq!(git_config.get_raw_value("core", None, "a"), Ok("d"));
     /// ```
     ///
-    /// The the resolution is as follows
+    /// # Errors
-    pub fn get_raw_single_value<'b>(
+    ///
+    /// This function will return an error if the key is not in the requested
+    /// section and subsection.
+    pub fn get_raw_value<'b>(
         &self,
         section_name: &'b str,
         subsection_name: Option<&'b str>,
@@ -191,16 +184,13 @@ impl<'a> GitConfig<'a> {
         // Note: cannot wrap around the raw_multi_value method because we need
         // to guarantee that the highest section id is used (so that we follow
         // the "last one wins" resolution strategy by `git-config`).
-        let section_id = self
+        let section_id = self.get_section_id_by_name_and_subname(section_name, subsection_name)?;
-            .get_section_id_by_name_and_subname(section_name, subsection_name)
-            .ok_or(GitConfigError::SubSectionDoesNotExist(subsection_name))?;
 
         // section_id is guaranteed to exist in self.sections, else we have a
         // violated invariant.
         let events = self.sections.get(&section_id).unwrap();
         let mut found_key = false;
         let mut latest_value = None;
-        // logic needs fixing for last one wins rule
         for event in events {
             match event {
                 Event::Key(event_key) if *event_key == key => found_key = true,
@@ -219,10 +209,13 @@ impl<'a> GitConfig<'a> {
         &'a self,
         section_name: &'b str,
         subsection_name: Option<&'b str>,
-    ) -> Option<SectionId> {
+    ) -> Result<SectionId, GitConfigError<'b>> {
         self.get_section_ids_by_name_and_subname(section_name, subsection_name)
-            .map(|vec| vec.into_iter().max())
+            .map(|vec| {
-            .flatten()
+                // 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()
+            })
     }
 
     pub fn get_raw_multi_value<'b>(
@@ -232,26 +225,33 @@ impl<'a> GitConfig<'a> {
         key: &'b str,
     ) -> Result<Vec<&'a str>, GitConfigError<'b>> {
         let values = self
-            .get_section_ids_by_name_and_subname(section_name, subsection_name)
+            .get_section_ids_by_name_and_subname(section_name, subsection_name)?
-            .ok_or(GitConfigError::SubSectionDoesNotExist(subsection_name))?
             .iter()
             .map(|section_id| {
                 let mut found_key = false;
+                let mut events = vec![];
                 // section_id is guaranteed to exist in self.sections, else we have a
                 // violated invariant.
                 for event in self.sections.get(section_id).unwrap() {
                     match event {
                         Event::Key(event_key) if *event_key == key => found_key = true,
-                        Event::Value(v) if found_key => return Ok(*v),
+                        Event::Value(v) if found_key => {
+                            events.push(*v);
+                            found_key = false;
+                        }
                         _ => (),
                     }
                 }
 
+                if events.is_empty() {
                     Err(GitConfigError::KeyDoesNotExist(key))
+                } else {
+                    Ok(events)
+                }
             })
             .filter_map(Result::ok)
+            .flatten()
             .collect::<Vec<_>>();
-
         if values.is_empty() {
             Err(GitConfigError::KeyDoesNotExist(key))
         } else {
@@ -263,34 +263,39 @@ impl<'a> GitConfig<'a> {
         &'a self,
         section_name: &'b str,
         subsection_name: Option<&'b str>,
-    ) -> Option<Vec<SectionId>> {
+    ) -> Result<&[SectionId], GitConfigError<'b>> {
-        let section_ids = self.section_lookup_tree.get(section_name)?;
+        let section_ids = self
-        if let Some(subsect_name) = subsection_name {
+            .section_lookup_tree
+            .get(section_name)
+            .ok_or(GitConfigError::SectionDoesNotExist(section_name))?;
         let mut maybe_ids = None;
+        // Don't simplify if and matches here -- the for loop currently needs
+        // `n + 1` checks, while the if and matches will result in the for loop
+        // needing `2n` checks.
+        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);
                     break;
                 }
             }
-            maybe_ids.map(|vec| vec.clone())
         } else {
-            let mut maybe_ids = None;
             for node in section_ids {
                 if let LookupTreeNode::Terminal(subsection_lookup) = node {
-                    maybe_ids = subsection_lookup.iter().max();
+                    maybe_ids = Some(subsection_lookup);
                     break;
                 }
             }
-            maybe_ids.map(|v| vec![*v])
         }
+        maybe_ids
+            .map(Vec::as_slice)
+            .ok_or(GitConfigError::SubSectionDoesNotExist(subsection_name))
     }
 }
 
 #[cfg(test)]
-mod git_config {
+mod from_parser {
-    mod from_parser {
+    use super::*;
-        use super::super::*;
 
     #[test]
     fn parse_empty() {
@@ -336,12 +341,246 @@ mod git_config {
     }
 
     #[test]
-        fn parse_single_subsection() {}
+    fn parse_single_subsection() {
+        let config = GitConfig::from_str("[core.subsec]\na=b\nc=d").unwrap();
+        let expected_separators = {
+            let mut map = HashMap::new();
+            map.insert(SectionId(0), Some("."));
+            map
+        };
+        assert_eq!(config.section_header_separators, expected_separators);
+        assert_eq!(config.section_id_counter, 1);
+        let expected_lookup_tree = {
+            let mut tree = HashMap::new();
+            let mut inner_tree = HashMap::new();
+            inner_tree.insert("subsec", vec![SectionId(0)]);
+            tree.insert("core", vec![LookupTreeNode::NonTerminal(inner_tree)]);
+            tree
+        };
+        assert_eq!(config.section_lookup_tree, expected_lookup_tree);
+        let expected_sections = {
+            let mut sections = HashMap::new();
+            sections.insert(
+                SectionId(0),
+                vec![
+                    Event::Newline("\n"),
+                    Event::Key("a"),
+                    Event::Value("b"),
+                    Event::Newline("\n"),
+                    Event::Key("c"),
+                    Event::Value("d"),
+                ],
+            );
+            sections
+        };
+        assert_eq!(config.sections, expected_sections);
+    }
 
     #[test]
-        fn parse_multiple_sections() {}
+    fn parse_multiple_sections() {
+        let config = GitConfig::from_str("[core]\na=b\nc=d\n[other]e=f").unwrap();
+        let expected_separators = {
+            let mut map = HashMap::new();
+            map.insert(SectionId(0), None);
+            map.insert(SectionId(1), None);
+            map
+        };
+        assert_eq!(config.section_header_separators, expected_separators);
+        assert_eq!(config.section_id_counter, 2);
+        let expected_lookup_tree = {
+            let mut tree = HashMap::new();
+            tree.insert("core", vec![LookupTreeNode::Terminal(vec![SectionId(0)])]);
+            tree.insert("other", vec![LookupTreeNode::Terminal(vec![SectionId(1)])]);
+            tree
+        };
+        assert_eq!(config.section_lookup_tree, expected_lookup_tree);
+        let expected_sections = {
+            let mut sections = HashMap::new();
+            sections.insert(
+                SectionId(0),
+                vec![
+                    Event::Newline("\n"),
+                    Event::Key("a"),
+                    Event::Value("b"),
+                    Event::Newline("\n"),
+                    Event::Key("c"),
+                    Event::Value("d"),
+                    Event::Newline("\n"),
+                ],
+            );
+            sections.insert(SectionId(1), vec![Event::Key("e"), Event::Value("f")]);
+            sections
+        };
+        assert_eq!(config.sections, expected_sections);
+    }
 
     #[test]
-        fn parse_multiple_duplicate_sections() {}
+    fn parse_multiple_duplicate_sections() {
+        let config = GitConfig::from_str("[core]\na=b\nc=d\n[core]e=f").unwrap();
+        let expected_separators = {
+            let mut map = HashMap::new();
+            map.insert(SectionId(0), None);
+            map.insert(SectionId(1), None);
+            map
+        };
+        assert_eq!(config.section_header_separators, expected_separators);
+        assert_eq!(config.section_id_counter, 2);
+        let expected_lookup_tree = {
+            let mut tree = HashMap::new();
+            tree.insert(
+                "core",
+                vec![LookupTreeNode::Terminal(vec![SectionId(0), SectionId(1)])],
+            );
+            tree
+        };
+        assert_eq!(config.section_lookup_tree, expected_lookup_tree);
+        let expected_sections = {
+            let mut sections = HashMap::new();
+            sections.insert(
+                SectionId(0),
+                vec![
+                    Event::Newline("\n"),
+                    Event::Key("a"),
+                    Event::Value("b"),
+                    Event::Newline("\n"),
+                    Event::Key("c"),
+                    Event::Value("d"),
+                    Event::Newline("\n"),
+                ],
+            );
+            sections.insert(SectionId(1), vec![Event::Key("e"), Event::Value("f")]);
+            sections
+        };
+        assert_eq!(config.sections, expected_sections);
+    }
+}
+
+#[cfg(test)]
+mod get_raw_value {
+    use super::*;
+
+    #[test]
+    fn single_section() {
+        let config = GitConfig::from_str("[core]\na=b\nc=d").unwrap();
+        assert_eq!(config.get_raw_value("core", None, "a"), Ok("b"));
+        assert_eq!(config.get_raw_value("core", None, "c"), Ok("d"));
+    }
+
+    #[test]
+    fn last_one_wins_respected_in_section() {
+        let config = GitConfig::from_str("[core]\na=b\na=d").unwrap();
+        assert_eq!(config.get_raw_value("core", None, "a"), Ok("d"));
+    }
+
+    #[test]
+    fn last_one_wins_respected_across_section() {
+        let config = GitConfig::from_str("[core]\na=b\n[core]\na=d").unwrap();
+        assert_eq!(config.get_raw_value("core", None, "a"), Ok("d"));
+    }
+
+    #[test]
+    fn section_not_found() {
+        let config = GitConfig::from_str("[core]\na=b\nc=d").unwrap();
+        assert_eq!(
+            config.get_raw_value("foo", None, "a"),
+            Err(GitConfigError::SectionDoesNotExist("foo"))
+        );
+    }
+
+    #[test]
+    fn subsection_not_found() {
+        let config = GitConfig::from_str("[core]\na=b\nc=d").unwrap();
+        assert_eq!(
+            config.get_raw_value("core", Some("a"), "a"),
+            Err(GitConfigError::SubSectionDoesNotExist(Some("a")))
+        );
+    }
+
+    #[test]
+    fn key_not_found() {
+        let config = GitConfig::from_str("[core]\na=b\nc=d").unwrap();
+        assert_eq!(
+            config.get_raw_value("core", None, "aaaaaa"),
+            Err(GitConfigError::KeyDoesNotExist("aaaaaa"))
+        );
+    }
+
+    #[test]
+    fn subsection_must_be_respected() {
+        let config = GitConfig::from_str("[core]a=b\n[core.a]a=c").unwrap();
+        assert_eq!(config.get_raw_value("core", None, "a"), Ok("b"));
+        assert_eq!(config.get_raw_value("core", Some("a"), "a"), Ok("c"));
+    }
+}
+
+#[cfg(test)]
+mod get_raw_multi_value {
+    use super::*;
+
+    #[test]
+    fn single_value_is_identical_to_single_value_query() {
+        let config = GitConfig::from_str("[core]\na=b\nc=d").unwrap();
+        assert_eq!(
+            vec![config.get_raw_value("core", None, "a").unwrap()],
+            config.get_raw_multi_value("core", None, "a").unwrap()
+        );
+    }
+
+    #[test]
+    fn multi_value_in_section() {
+        let config = GitConfig::from_str("[core]\na=b\na=c").unwrap();
+        assert_eq!(
+            config.get_raw_multi_value("core", None, "a").unwrap(),
+            vec!["b", "c"]
+        );
+    }
+
+    #[test]
+    fn multi_value_across_sections() {
+        let config = GitConfig::from_str("[core]\na=b\na=c\n[core]a=d").unwrap();
+        assert_eq!(
+            config.get_raw_multi_value("core", None, "a").unwrap(),
+            vec!["b", "c", "d"]
+        );
+    }
+
+    #[test]
+    fn section_not_found() {
+        let config = GitConfig::from_str("[core]\na=b\nc=d").unwrap();
+        assert_eq!(
+            config.get_raw_multi_value("foo", None, "a"),
+            Err(GitConfigError::SectionDoesNotExist("foo"))
+        );
+    }
+
+    #[test]
+    fn subsection_not_found() {
+        let config = GitConfig::from_str("[core]\na=b\nc=d").unwrap();
+        assert_eq!(
+            config.get_raw_multi_value("core", Some("a"), "a"),
+            Err(GitConfigError::SubSectionDoesNotExist(Some("a")))
+        );
+    }
+
+    #[test]
+    fn key_not_found() {
+        let config = GitConfig::from_str("[core]\na=b\nc=d").unwrap();
+        assert_eq!(
+            config.get_raw_multi_value("core", None, "aaaaaa"),
+            Err(GitConfigError::KeyDoesNotExist("aaaaaa"))
+        );
+    }
+
+    #[test]
+    fn subsection_must_be_respected() {
+        let config = GitConfig::from_str("[core]a=b\n[core.a]a=c").unwrap();
+        assert_eq!(
+            config.get_raw_multi_value("core", None, "a").unwrap(),
+            vec!["b"]
+        );
+        assert_eq!(
+            config.get_raw_multi_value("core", Some("a"), "a").unwrap(),
+            vec!["c"]
+        );
     }
 }

src/parser.rs

@@ -1,11 +1,13 @@
 //! This module handles parsing a `git-config`. Generally speaking, you want to
-//! use a higher abstraction unless you have some explicit reason to work with
+//! use a higher abstraction such as [`GitConfig`] unless you have some explicit
-//! events instead.
+//! reason to work with events instead.
 //!
 //! The general workflow for interacting with this is to use one of the
 //! `parse_from_*` function variants. These will return a [`Parser`] on success,
 //! which can be converted into an [`Event`] iterator. The [`Parser`] also has
 //! additional methods for accessing leading comments or events by section.
+//!
+//! [`GitConfig`]: crate::config::GitConfig
 
 use nom::bytes::complete::{escaped, tag, take_till, take_while};
 use nom::character::complete::{char, none_of, one_of};
@@ -616,18 +618,17 @@ fn take_common<'a, F: Fn(char) -> bool>(i: &'a str, f: F) -> IResult<&'a str, &'
         Ok((i, v))
     }
 }
+
 #[cfg(test)]
-mod parse {
+fn fully_consumed<T>(t: T) -> (&'static str, T) {
-    use super::*;
-
-    fn fully_consumed<T>(t: T) -> (&'static str, T) {
     ("", t)
-    }
+}
 
-    fn gen_section_header(
+#[cfg(test)]
+fn gen_section_header(
     name: &str,
     subsection: impl Into<Option<(&'static str, &'static str)>>,
-    ) -> ParsedSectionHeader<'_> {
+) -> ParsedSectionHeader<'_> {
     if let Some((separator, subsection_name)) = subsection.into() {
         ParsedSectionHeader {
             name,
@@ -641,10 +642,10 @@ mod parse {
             subsection_name: None,
         }
     }
-    }
+}
 
-    mod comments {
+#[cfg(test)]
-        use super::super::*;
+mod comments {
     use super::*;
 
     #[test]
@@ -679,10 +680,10 @@ mod parse {
             })
         );
     }
-    }
+}
 
-    mod section_headers {
+#[cfg(test)]
-        use super::super::*;
+mod section_headers {
     use super::*;
 
     #[test]
@@ -750,10 +751,10 @@ mod parse {
             fully_consumed(gen_section_header("hello", (" ", "]")))
         );
     }
-    }
+}
 
-    mod config_name {
+#[cfg(test)]
-        use super::super::*;
+mod config_name {
     use super::*;
 
     #[test]
@@ -771,10 +772,10 @@ mod parse {
     fn cannot_be_empty() {
         assert!(config_name("").is_err())
     }
-    }
+}
 
-    mod value_no_continuation {
+#[cfg(test)]
-        use super::super::*;
+mod value_no_continuation {
     use super::*;
 
     #[test]
@@ -849,10 +850,10 @@ mod parse {
     fn garbage_after_continution_is_err() {
         assert!(value_impl("hello \\afwjdls").is_err());
     }
-    }
+}
 
-    mod value_continuation {
+#[cfg(test)]
-        use super::super::*;
+mod value_continuation {
     use super::*;
 
     #[test]
@@ -910,10 +911,10 @@ mod parse {
             )
         )
     }
-    }
+}
 
-    mod section {
+#[cfg(test)]
-        use super::super::*;
+mod section {
     use super::*;
 
     #[test]
@@ -1088,5 +1089,4 @@ mod parse {
             })
         );
     }
-    }
 }