libgit-rs: add get_bool() method to ConfigSet

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 <[email protected]>

## 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.

## 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 <[email protected]>

## 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.

## 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