Hi Josh
On 09/08/2024 23:41, Josh Steadmon wrote:
From: Calvin Wan <calvinwan@xxxxxxxxxx>
Wrap `struct config_set` and a few of its associated functions in
cgit-sys. Also introduce a higher-level "cgit" crate which provides a
more Rust-friendly interface to config_set structs.
Having an ergonamic interface is a really good idea. As far as the
naming goes I think the suggestion of "libgit-rs" is a good one.
diff --git a/contrib/cgit-rs/cgit-sys/public_symbol_export.h b/contrib/cgit-rs/cgit-sys/public_symbol_export.h
index 64332f30de..882c7932e8 100644
--- a/contrib/cgit-rs/cgit-sys/public_symbol_export.h
+++ b/contrib/cgit-rs/cgit-sys/public_symbol_export.h
@@ -9,6 +9,18 @@ void libgit_init_git(const char **argv);
int libgit_parse_maybe_bool(const char *val);
I'm suprised the compiler does not complain that 'struct config_set' is
not declared in this header - I was expecting to see
struct config_set;
before the function declarations. As I said in my comments on the last
patch I think we'd be better to namespace our types as well as our
functions in this library layer so this can be resued by other language
bindings.
> [...]
+ pub fn get_str(&mut self, key: &str) -> Option<CString> {
If we're adding an ergonomic api then having return CString isn't ideal.
I think the equivalent function in libgit2-rs has variants that return a
String which is convinent if the caller is expecting utf8 values or
Vec<u8> for non-utf8 values.
Best Wishes
Phillip
