From: "ionnss via GitGitGadget" <gitgitgadget@gmail•com>
To: git@vger•kernel.org
Cc: Chris Torek <chris.torek@gmail•com>,
Phillip Wood <phillip.wood123@gmail•com>,
ions <zara.leonardo@gmail•com>, ionnss <zara.leonardo@gmail•com>
Subject: [PATCH v2 3/3] libgit-rs: address review feedback for get_bool()
Date: Sat, 27 Sep 2025 00:07:39 +0000 [thread overview]
Message-ID: <43784e3ff991929b5c314ad5b9ab6573e6ca48c4.1758931659.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1977.v2.git.1758931659.gitgitgadget@gmail.com>
From: ionnss <zara.leonardo@gmail•com>
- 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)
This addresses feedback from Phillip Wood and Chris Torek about using
Git's actual boolean parsing logic rather than duplicating it in Rust.
Signed-off-by: ionnss <zara.leonardo@gmail•com>
---
contrib/libgit-rs/src/config.rs | 33 ++++++++++++++++--------------
contrib/libgit-rs/testdata/config3 | 4 +---
contrib/libgit-rs/testdata/config4 | 10 +++++++++
contrib/libgit-sys/src/lib.rs | 6 ++++++
4 files changed, 35 insertions(+), 18 deletions(-)
create mode 100644 contrib/libgit-rs/testdata/config4
diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs
index 3f4a32c72d..b280b952b2 100644
--- a/contrib/libgit-rs/src/config.rs
+++ b/contrib/libgit-rs/src/config.rs
@@ -69,24 +69,18 @@ impl ConfigSet {
}
}
+ /// 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();
+ 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
- {
+ 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,
- }
}
+
+ Some(val != 0)
}
}
@@ -115,6 +109,7 @@ mod tests {
Path::new("testdata/config1"),
Path::new("testdata/config2"),
Path::new("testdata/config3"),
+ Path::new("testdata/config4"),
]);
// ConfigSet retrieves correct value
assert_eq!(cs.get_int("trace2.eventTarget"), Some(1));
@@ -122,8 +117,16 @@ 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));
+ assert_eq!(cs.get_bool("test.boolYes"), Some(true));
+ assert_eq!(cs.get_bool("test.boolNo"), Some(false));
+ assert_eq!(cs.get_bool("test.boolOne"), Some(true));
+ 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);
}
diff --git a/contrib/libgit-rs/testdata/config3 b/contrib/libgit-rs/testdata/config3
index 83a474ccef..3ea5b96f12 100644
--- a/contrib/libgit-rs/testdata/config3
+++ b/contrib/libgit-rs/testdata/config3
@@ -1,4 +1,2 @@
[trace2]
- eventNesting = 3
-[test]
- booleanValue = true
+ eventNesting = 3
\ No newline at end of file
diff --git a/contrib/libgit-rs/testdata/config4 b/contrib/libgit-rs/testdata/config4
new file mode 100644
index 0000000000..5b75385c38
--- /dev/null
+++ b/contrib/libgit-rs/testdata/config4
@@ -0,0 +1,10 @@
+[test]
+ boolTrue = true
+ boolFalse = false
+ boolYes = yes
+ boolNo = no
+ boolOne = 1
+ boolZero = 0
+ boolZeroZero = 00
+ boolHundred = 100
+ boolSeven = 007
diff --git a/contrib/libgit-sys/src/lib.rs b/contrib/libgit-sys/src/lib.rs
index 4bfc650450..b104fda8f6 100644
--- a/contrib/libgit-sys/src/lib.rs
+++ b/contrib/libgit-sys/src/lib.rs
@@ -43,6 +43,12 @@ extern "C" {
dest: *mut *mut c_char,
) -> c_int;
+ pub fn libgit_configset_get_bool(
+ cs: *mut libgit_config_set,
+ key: *const c_char,
+ dest: *mut c_int,
+ ) -> c_int;
+
}
#[cfg(test)]
--
gitgitgadget
next prev parent reply other threads:[~2025-09-27 0:07 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-25 11:44 [PATCH 0/2] libgit-rs: add get_bool() method to ConfigSet ions via GitGitGadget
2025-09-25 11:44 ` [PATCH 1/2] po: fix escaped underscores in README.md ionnss via GitGitGadget
2025-09-25 11:44 ` [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget
2025-09-26 6:43 ` Chris Torek
2025-09-26 9:58 ` Phillip Wood
2025-09-26 17:15 ` Junio C Hamano
2025-09-27 0:07 ` [PATCH v2 0/3] " ions via GitGitGadget
2025-09-27 0:07 ` [PATCH v2 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget
2025-09-27 0:07 ` [PATCH v2 2/3] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget
2025-09-27 0:07 ` ionnss via GitGitGadget [this message]
2025-09-27 2:01 ` [PATCH v2 0/3] " Junio C Hamano
2025-09-27 3:51 ` [PATCH v3 " ions via GitGitGadget
2025-09-27 3:51 ` [PATCH v3 1/3] po: fix escaped underscores in README.md ionnss via GitGitGadget
2025-09-29 13:26 ` Phillip Wood
2025-09-27 3:51 ` [PATCH v3 2/3] libgit-rs: add get_bool() method to ConfigSet ionnss via GitGitGadget
2025-09-29 13:23 ` Phillip Wood
2025-09-27 3:51 ` [PATCH v3 3/3] libgit-rs: add get_ulong() and get_pathname() methods ionnss via GitGitGadget
2025-09-29 13:23 ` Phillip Wood
2025-09-30 8:46 ` [PATCH v4] libgit-rs: add get_bool(), get_ulong(), " ions via GitGitGadget
2025-10-01 10:15 ` Phillip Wood
2025-10-06 21:20 ` brian m. carlson
2025-10-08 13:36 ` Phillip Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=43784e3ff991929b5c314ad5b9ab6573e6ca48c4.1758931659.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail$(echo .)com \
--cc=chris.torek@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=phillip.wood123@gmail$(echo .)com \
--cc=zara.leonardo@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox