Hi Calvin
On 21/08/2024 19:46, Calvin Wan wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
>
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.
Are you suggesting something like "#define libgit_config_set
config_set"? I wouldn't be comfortable renaming config_set in git.git
just yet until config/config_set can be a standalone library by itself.
I was suggesting[1] adding
struct libgit_configset {
struct config_set set;
};
to public_symbol_export.c and rewriting the wrappers in that file to use
this struct e.g.
int libgit_configset_get_int(struct libgit_configset *cs,
const char *key, int *dest)
{
return git_configset_get_int(&cs.set, key, dest);
}
In public_symbol_export.h we'd then have
struct libgit_configset;
int libgit_configset_get_int(struct libgit_configset *,
const char *, int *);
If we want the symbol exports to be useful outside of the rust bindings
I think we need to namespace our types as well as our functions.
[1]
https://lore.kernel.org/git/5720d5b9-a850-4024-a1fd-54acc6b15a74@xxxxxxxxx
+ 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.
Having both get_cstr() and get_str() makes sense to me.
Just to be clear get_cstr() would return Vec<u8>? I'm far from a rust
expert but my understanding was that crates that wrap C libraries
generally avoid using CString in their API.
Best Wishes
Phillip
