fix: Properly handle boolean values such that a is true but a= is false. (#450)

This is even consistent when no booleans are used, such that `a` has no
value as if it is not present, it's only available for booleans which
must be specified.

Author: Byron

git-config/src/file/access/read_only.rs

@@ -40,7 +40,7 @@ impl<'event> File<'event> {
     /// let config = r#"
     ///     [core]
     ///         a = 10k
-    ///         c
+    ///         c = false
     /// "#;
     /// let git_config = git_config::File::try_from(config)?;
     /// // You can either use the turbofish to determine the type...

git-config/src/file/mutable/section.rs

@@ -92,7 +92,8 @@ impl<'a, 'event> SectionMut<'a, 'event> {
                 self.push(key, Some(value.into()));
                 None
             }
-            Some((_, value_range)) => {
+            Some((key_range, value_range)) => {
+                let value_range = value_range.unwrap_or(key_range.end - 1..key_range.end);
                 let range_start = value_range.start;
                 let ret = self.remove_internal(value_range);
                 self.section

git-config/src/file/section/body.rs

@@ -20,6 +20,7 @@ impl<'event> Body<'event> {
     pub fn value(&self, key: impl AsRef<str>) -> Option<Cow<'_, BStr>> {
         let key = Key::from_str_unchecked(key.as_ref());
         let (_key_range, range) = self.key_and_value_range_by(&key)?;
+        let range = range?;
         let mut concatenated = BString::default();
 
         for event in &self.0[range] {
@@ -109,9 +110,10 @@ impl<'event> Body<'event> {
         &self.0
     }
 
-    /// Returns the the range containing the value events for the `key`.
-    /// If the value is not found, then this returns an empty range.
-    pub(crate) fn key_and_value_range_by(&self, key: &Key<'_>) -> Option<(Range<usize>, Range<usize>)> {
+    /// Returns the the range containing the value events for the `key`, with value range being `None` if there is no key-value separator
+    /// and only a 'fake' Value event with an empty string in side.
+    /// If the value is not found, `None` is returned.
+    pub(crate) fn key_and_value_range_by(&self, key: &Key<'_>) -> Option<(Range<usize>, Option<Range<usize>>)> {
         let mut value_range = Range::default();
         let mut key_start = None;
         for (i, e) in self.0.iter().enumerate().rev() {
@@ -140,7 +142,8 @@ impl<'event> Body<'event> {
             // value end needs to be offset by one so that the last value's index
             // is included in the range
             let value_range = value_range.start..value_range.end + 1;
-            (key_start..value_range.end, value_range)
+            let key_range = key_start..value_range.end;
+            (key_range, (value_range.start != key_start + 1).then(|| value_range))
         })
     }
 }

git-config/src/lib.rs

@@ -22,12 +22,6 @@
 //! - Legacy headers like `[section.subsection]` are supposed to be turned into to lower case and compared
 //!   case-sensitively. We keep its case and compare case-insensitively.
 //!
-//! # Limitations
-//!
-//! - Keys like `a.b` are interpreted as true but `a.b = ` are interpreted as false in `git`.
-//!   This library, however, considers both to be true due to its inability to differentiate them at present.
-//!   Even to though the information is available, the API doesn't expose it.
-//!
 //! [^1]: When read values do not need normalization and it wasn't parsed in 'owned' mode.
 //!
 //! [`git-config` files]: https://git-scm.com/docs/git-config#_configuration_file

git-config/src/parse/nom/mod.rs

@@ -288,7 +288,9 @@ fn config_value<'a>(i: &'a [u8], dispatch: &mut impl FnMut(Event<'a>)) -> IResul
         let (i, newlines) = value_impl(i, dispatch)?;
         Ok((i, newlines))
     } else {
-        // TODO: remove this and fix everything, or else we can't fix the boolean limitation
+        // This is a special way of denoting 'empty' values which a lot of code depends on.
+        // Hence, rather to fix this everywhere else, leave it here and fix it where it matters, namely
+        // when it's about differentiating between a missing key-vaue separator, and one followed by emptiness.
         dispatch(Event::Value(Cow::Borrowed("".into())));
         Ok((i, 0))
     }

git-config/tests/file/access/read_only.rs

@@ -41,12 +41,12 @@ fn get_value_for_all_provided_values() -> crate::Result {
         assert!(!config.boolean("core", None, "bool-explicit").expect("exists")?);
 
         assert!(
-            !config.value::<Boolean>("core", None, "bool-implicit")?.0,
+            config.value::<Boolean>("core", None, "bool-implicit").is_err(),
             "this cannot work like in git as the value isn't there for us"
         );
         assert!(
-            !config.boolean("core", None, "bool-implicit").expect("present")?,
-            "this should work, but doesn't yet"
+            config.boolean("core", None, "bool-implicit").expect("present")?,
+            "this should work"
         );
 
         assert_eq!(config.string("doesnt", None, "exist"), None);
@@ -177,11 +177,11 @@ fn get_value_looks_up_all_sections_before_failing() -> crate::Result {
 
 #[test]
 fn section_names_are_case_insensitive() -> crate::Result {
-    let config = "[core] bool-implicit";
+    let config = "[core] a=true";
     let file = File::try_from(config)?;
     assert_eq!(
-        file.value::<Boolean>("core", None, "bool-implicit").unwrap(),
-        file.value::<Boolean>("CORE", None, "bool-implicit").unwrap()
+        file.value::<Boolean>("core", None, "a").unwrap(),
+        file.value::<Boolean>("CORE", None, "a").unwrap()
     );
 
     Ok(())
@@ -209,13 +209,13 @@ fn single_section() {
     assert_eq!(first_value, cow_str("b"));
 
     assert!(
-        config.raw_value("core", None, "c").is_ok(),
-        "value is considered false as it is without '=', so it's like not present, BUT this parses strangely which needs fixing (see TODO nom parse)"
+        config.raw_value("core", None, "c").is_err(),
+        "value is considered false as it is without '=', so it's like not present"
     );
 
     assert!(
-        !config.boolean("core", None, "c").expect("present").unwrap(),
-        "asking for a boolean is true true, as per git rules, but doesn't work yet"
+        config.boolean("core", None, "c").expect("present").unwrap(),
+        "asking for a boolean is true true, as per git rules"
     );
 }