Re: [PATCH 2/2] libgit-rs: add get_bool() method to ConfigSet
From: Chris Torek <hidden>
Date: 2025-09-26 06:43:41
A bit minor, and I'm not a real Rust programmer, but: On Thu, Sep 25, 2025 at 4:44 AM ionnss via GitGitGadget [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: ionnss <redacted> 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. The implementation reuses the existing get_string() infrastructure and adds case-insensitive boolean parsing logic. Signed-off-by: ionnss <redacted> --- contrib/libgit-rs/src/config.rs | 24 ++++++++++++++++++++++++ contrib/libgit-rs/testdata/config3 | 2 ++ 2 files changed, 26 insertions(+)diff --git a/contrib/libgit-rs/src/config.rs b/contrib/libgit-rs/src/config.rs index 6bf04845c8..3f4a32c72d 100644 --- a/contrib/libgit-rs/src/config.rs +++ b/contrib/libgit-rs/src/config.rs@@ -68,6 +68,26 @@ impl ConfigSet { Some(owned_str) } } + + pub fn get_bool(&mut self, key: &str) -> Option<bool> { + let key = CString::new(key).expect("Couldn't convert key to CString");
The string argument for `.expect` should be phrased in
a more positive manner in terms of what is expected,
since failure will cause a panic. So, something like:
let key = CString::new(key).expect("boolean key should be valid CString");
which would produce, e.g.,
panic: boolean key should be valid CString: ... details of key ...
A similar rule applies to the later `.expect`.
Chris