[PATCH v3 0/3] libgit-rs: add get_bool() method to ConfigSet

["ions via GitGitGadget" <gitgitgadget@gmail.com>]

Purpose

This pull request introduces a get_bool() method to the ConfigSet module in
the libgit-rs library. The goal is to enhance the functionality of ConfigSet
by providing a way to fetch and handle boolean configuration values more
easily and consistently.

Implementation Details

• Added a get_bool() method to the ConfigSet module.

• The method retrieves configuration values as boolean values, ensuring
proper parsing and error handling.

• This addition simplifies the process of working with boolean
configurations for developers using the ConfigSet module.

Testing

• Added unit tests to verify the correctness of the get_bool() method.

• Tested edge cases to ensure robustness.

ionnss (3):
  po: fix escaped underscores in README.md
  libgit-rs: add get_bool() method to ConfigSet
  libgit-rs: add get_ulong() and get_pathname() methods

 contrib/libgit-rs/src/config.rs    | 83 +++++++++++++++++++++++++++++-
 contrib/libgit-rs/testdata/config3 |  2 +-
 contrib/libgit-rs/testdata/config4 | 13 +++++
 contrib/libgit-sys/src/lib.rs      | 24 ++++++++-
 po/README.md                       |  6 +--
 5 files changed, 121 insertions(+), 7 deletions(-)
 create mode 100644 contrib/libgit-rs/testdata/config4


base-commit: bb69721404348ea2db0a081c41ab6ebfe75bdec8
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1977%2Fionnss%2Fadd-rust-configset-get-bool-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1977/ionnss/add-rust-configset-get-bool-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1977

Range-diff vs v2:

 1:  d7810781fc = 1:  d7810781fc po: fix escaped underscores in README.md
 3:  43784e3ff9 ! 2:  479c263bc1 libgit-rs: address review feedback for get_bool()
     @@ Metadata
      Author: ionnss <zara.leonardo@gmail.com>
      
       ## Commit message ##
     -    libgit-rs: address review feedback for get_bool()
     +    libgit-rs: add get_bool() method to ConfigSet
      
     -    - Use git_configset_get_bool() C function instead of reimplementing parsing
     -    - Fix libgit_configset_get_bool() function signature in bindings
     -    - Improve .expect() error messages to be more descriptive
     -    - Add comprehensive boolean tests including edge cases (00, 100, 007)
     +    Add support for parsing boolean configuration values using Git's
     +    git_configset_get_bool() C function. This ensures consistent behavior
     +    with Git's native boolean parsing logic.
      
     -    This addresses feedback from Phillip Wood and Chris Torek about using
     -    Git's actual boolean parsing logic rather than duplicating it in Rust.
     +    The method handles all Git boolean formats (true/false, yes/no, on/off,
     +    1/0) and edge cases like "00" and "100" correctly.
     +
     +    Includes comprehensive tests for various boolean formats and edge cases.
      
          Signed-off-by: ionnss <zara.leonardo@gmail.com>
      
       ## contrib/libgit-rs/src/config.rs ##
      @@ contrib/libgit-rs/src/config.rs: impl ConfigSet {
     +             Some(owned_str)
               }
           }
     - 
     ++
      +    /// Load the value for the given key and attempt to parse it as a boolean. Dies with a fatal error
      +    /// if the value cannot be parsed. Returns None if the key is not present.
     -     pub fn get_bool(&mut self, key: &str) -> Option<bool> {
     --        let key = CString::new(key).expect("Couldn't convert key to CString");
     --        let mut val: *mut c_char = std::ptr::null_mut();
     ++    pub fn get_bool(&mut self, key: &str) -> Option<bool> {
      +        let key = CString::new(key).expect("config key should be valid CString");
      +        let mut val: c_int = 0;
     -         unsafe {
     --            if libgit_configset_get_string(self.0, key.as_ptr(), &mut val as *mut *mut c_char) != 0
     --            {
     ++        unsafe {
      +            if libgit_configset_get_bool(self.0, key.as_ptr(), &mut val as *mut c_int) != 0 {
     -                 return None;
     -             }
     --            let borrowed_str = CStr::from_ptr(val);
     --            let owned_str =
     --                String::from(borrowed_str.to_str().expect("Couldn't convert val to str"));
     --            free(val as *mut c_void); // Free the xstrdup()ed pointer from the C side
     --            match owned_str.to_lowercase().as_str() {
     --                "true" | "yes" | "on" | "1" => Some(true),
     --                "false" | "no" | "off" | "0" => Some(false),
     --                _ => None,
     --            }
     -         }
     ++                return None;
     ++            }
     ++        }
      +
      +        Some(val != 0)
     -     }
     ++    }
       }
       
     + impl Default for ConfigSet {
      @@ contrib/libgit-rs/src/config.rs: mod tests {
                   Path::new("testdata/config1"),
                   Path::new("testdata/config2"),
     @@ contrib/libgit-rs/src/config.rs: mod tests {
               assert_eq!(cs.get_int("trace2.eventNesting"), Some(3));
               // ConfigSet returns None for missing key
               assert_eq!(cs.get_string("foo.bar"), None);
     --        // Test boolean parsing
     --        assert_eq!(cs.get_bool("test.booleanValue"), Some(true));
      +        // Test boolean parsing - comprehensive tests
      +        assert_eq!(cs.get_bool("test.boolTrue"), Some(true));
      +        assert_eq!(cs.get_bool("test.boolFalse"), Some(false));
     @@ contrib/libgit-rs/src/config.rs: mod tests {
      +        assert_eq!(cs.get_bool("test.boolZero"), Some(false));
      +        assert_eq!(cs.get_bool("test.boolZeroZero"), Some(false)); // "00" → false
      +        assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true
     -+        assert_eq!(cs.get_bool("test.boolSeven"), Some(true)); // "007" → true
     -         // Test missing boolean key
     -         assert_eq!(cs.get_bool("missing.boolean"), None);
     ++        // Test missing boolean key
     ++        assert_eq!(cs.get_bool("missing.boolean"), None);
           }
     + }
      
       ## contrib/libgit-rs/testdata/config3 ##
      @@
       [trace2]
      -	eventNesting = 3
     --[test]
     --	booleanValue = true
      +	eventNesting = 3
       \ No newline at end of file
      
     @@ contrib/libgit-rs/testdata/config4 (new)
      +	boolZero = 0
      +	boolZeroZero = 00
      +	boolHundred = 100
     -+	boolSeven = 007
      
       ## contrib/libgit-sys/src/lib.rs ##
      @@ contrib/libgit-sys/src/lib.rs: extern "C" {
 2:  a5904a2ac0 ! 3:  1ac8d76819 libgit-rs: add get_bool() method to ConfigSet
     @@ Metadata
      Author: ionnss <zara.leonardo@gmail.com>
      
       ## Commit message ##
     -    libgit-rs: add get_bool() method to ConfigSet
     +    libgit-rs: add get_ulong() and get_pathname() methods
      
     -    Add support for parsing boolean configuration values in the Rust
     -    ConfigSet API. The method follows Git's standard boolean parsing
     -    rules, accepting true/yes/on/1 as true and false/no/off/0 as false.
     +    Expand the ConfigSet API with additional configuration value types:
      
     -    The implementation reuses the existing get_string() infrastructure
     -    and adds case-insensitive boolean parsing logic.
     +    - get_ulong(): Parse unsigned long integers for large numeric values
     +    - get_pathname(): Parse file paths, returning PathBuf for type safety
     +
     +    Both functions follow the same pattern as existing get_* methods,
     +    using Git's C functions for consistent parsing behavior.
     +
     +    Add comprehensive tests covering normal cases, edge cases, and
     +    error handling for all new functionality.
      
          Signed-off-by: ionnss <zara.leonardo@gmail.com>
      
       ## contrib/libgit-rs/src/config.rs ##
     +@@
     + use std::ffi::{c_void, CStr, CString};
     +-use std::path::Path;
     ++use std::path::{Path, PathBuf};
     + 
     + #[cfg(has_std__ffi__c_char)]
     +-use std::ffi::{c_char, c_int};
     ++use std::ffi::{c_char, c_int, c_ulong};
     + 
     + #[cfg(not(has_std__ffi__c_char))]
     + #[allow(non_camel_case_types)]
     +@@ contrib/libgit-rs/src/config.rs: type c_char = i8;
     + #[allow(non_camel_case_types)]
     + type c_int = i32;
     + 
     ++#[cfg(not(has_std__ffi__c_char))]
     ++#[allow(non_camel_case_types)]
     ++type c_ulong = u64;
     ++
     + use libgit_sys::*;
     + 
     + /// A ConfigSet is an in-memory cache for config-like files such as `.gitmodules` or `.gitconfig`.
      @@ contrib/libgit-rs/src/config.rs: impl ConfigSet {
     -             Some(owned_str)
     -         }
     + 
     +         Some(val != 0)
           }
      +
     -+    pub fn get_bool(&mut self, key: &str) -> Option<bool> {
     -+        let key = CString::new(key).expect("Couldn't convert key to CString");
     ++    /// Load the value for the given key and attempt to parse it as an unsigned long. Dies with a fatal error
     ++    /// if the value cannot be parsed. Returns None if the key is not present.
     ++    pub fn get_ulong(&mut self, key: &str) -> Option<u64> {
     ++        let key = CString::new(key).expect("config key should be valid CString");
     ++        let mut val: c_ulong = 0;
     ++        unsafe {
     ++            if libgit_configset_get_ulong(self.0, key.as_ptr(), &mut val as *mut c_ulong) != 0 {
     ++                return None;
     ++            }
     ++        }
     ++        Some(val as u64)
     ++    }
     ++
     ++    /// Load the value for the given key and attempt to parse it as a file path. Dies with a fatal error
     ++    /// if the value cannot be converted to a PathBuf. Returns None if the key is not present.
     ++    pub fn get_pathname(&mut self, key: &str) -> Option<PathBuf> {
     ++        let key = CString::new(key).expect("config key should be valid CString");
      +        let mut val: *mut c_char = std::ptr::null_mut();
      +        unsafe {
     -+            if libgit_configset_get_string(self.0, key.as_ptr(), &mut val as *mut *mut c_char) != 0
     ++            if libgit_configset_get_pathname(self.0, key.as_ptr(), &mut val as *mut *mut c_char)
     ++                != 0
      +            {
      +                return None;
      +            }
      +            let borrowed_str = CStr::from_ptr(val);
     -+            let owned_str =
     -+                String::from(borrowed_str.to_str().expect("Couldn't convert val to str"));
     ++            let owned_str = String::from(
     ++                borrowed_str
     ++                    .to_str()
     ++                    .expect("config path should be valid UTF-8"),
     ++            );
      +            free(val as *mut c_void); // Free the xstrdup()ed pointer from the C side
     -+            match owned_str.to_lowercase().as_str() {
     -+                "true" | "yes" | "on" | "1" => Some(true),
     -+                "false" | "no" | "off" | "0" => Some(false),
     -+                _ => None,
     -+            }
     ++            Some(PathBuf::from(owned_str))
      +        }
      +    }
       }
       
       impl Default for ConfigSet {
      @@ contrib/libgit-rs/src/config.rs: mod tests {
     -         assert_eq!(cs.get_int("trace2.eventNesting"), Some(3));
     -         // ConfigSet returns None for missing key
     -         assert_eq!(cs.get_string("foo.bar"), None);
     -+        // Test boolean parsing
     -+        assert_eq!(cs.get_bool("test.booleanValue"), Some(true));
     -+        // Test missing boolean key
     -+        assert_eq!(cs.get_bool("missing.boolean"), None);
     +         assert_eq!(cs.get_bool("test.boolHundred"), Some(true)); // "100" → true
     +         // Test missing boolean key
     +         assert_eq!(cs.get_bool("missing.boolean"), None);
     ++        // Test ulong parsing
     ++        assert_eq!(cs.get_ulong("test.ulongSmall"), Some(42));
     ++        assert_eq!(cs.get_ulong("test.ulongBig"), Some(4294967296)); // > 32-bit int
     ++        assert_eq!(cs.get_ulong("missing.ulong"), None);
     ++        // Test pathname parsing
     ++        assert_eq!(
     ++            cs.get_pathname("test.pathRelative"),
     ++            Some(PathBuf::from("./some/path"))
     ++        );
     ++        assert_eq!(
     ++            cs.get_pathname("test.pathAbsolute"),
     ++            Some(PathBuf::from("/usr/bin/git"))
     ++        );
     ++        assert_eq!(cs.get_pathname("missing.path"), None);
           }
       }
      
     - ## contrib/libgit-rs/testdata/config3 ##
     + ## contrib/libgit-rs/testdata/config4 ##
     +@@
     + 	boolZero = 0
     + 	boolZeroZero = 00
     + 	boolHundred = 100
     ++	ulongSmall = 42
     ++	ulongBig = 4294967296
     ++	pathRelative = ./some/path
     ++	pathAbsolute = /usr/bin/git
     +
     + ## contrib/libgit-sys/src/lib.rs ##
      @@
     - [trace2]
     - 	eventNesting = 3
     -+[test]
     -+	booleanValue = true
     + use std::ffi::c_void;
     + 
     + #[cfg(has_std__ffi__c_char)]
     +-use std::ffi::{c_char, c_int};
     ++use std::ffi::{c_char, c_int, c_ulong};
     + 
     + #[cfg(not(has_std__ffi__c_char))]
     + #[allow(non_camel_case_types)]
     +@@ contrib/libgit-sys/src/lib.rs: pub type c_char = i8;
     + #[allow(non_camel_case_types)]
     + pub type c_int = i32;
     + 
     ++#[cfg(not(has_std__ffi__c_char))]
     ++#[allow(non_camel_case_types)]
     ++pub type c_ulong = u64;
     ++
     + extern crate libz_sys;
     + 
     + #[allow(non_camel_case_types)]
     +@@ contrib/libgit-sys/src/lib.rs: extern "C" {
     +         dest: *mut c_int,
     +     ) -> c_int;
     + 
     ++    pub fn libgit_configset_get_ulong(
     ++        cs: *mut libgit_config_set,
     ++        key: *const c_char,
     ++        dest: *mut c_ulong,
     ++    ) -> c_int;
     ++
     ++    pub fn libgit_configset_get_pathname(
     ++        cs: *mut libgit_config_set,
     ++        key: *const c_char,
     ++        dest: *mut *mut c_char,
     ++    ) -> c_int;
     ++
     + }
     + 
     + #[cfg(test)]

-- 
gitgitgadget