Settings: add support for search paths #337
Conversation
|
Sweet. 👍 |
| if (error < 0) { | ||
| xfree(buf); | ||
| rugged_exception_raise(); | ||
| return Qnil; |
There was a problem hiding this comment.
I don't think you need the return here.
There was a problem hiding this comment.
If I don't return, I'm going to try to create a new string out of freed memory and then free it again.
There was a problem hiding this comment.
I suppose I could have an if-else which tries to reduce the duplication. Let me try.
There was a problem hiding this comment.
rugged_exception_raise will return.
There was a problem hiding this comment.
And your return Qnil; is never actually executed.
|
I went with these strings because the existing function only really seems to support using one value. Ideally we should be able to have Rugged::Settings['search_path']['global'] = '/tmp/foo'but that's a bit of work when writing it in C. The way I went with this in python is to have a low-level option getter/setter which is then wrapped by the much nicer to write python code. |
|
I actually prefer the way it's implemented now. The sub-hash API seems a bit awkward to me, because we'd have to figure out how to handle things like: |
|
That actually looks like a pretty neat future to have :) But yeah, it does present extra problems. |
Let the user set and get the config search paths
|
Silly long jumps. This version should now not leak and not have unreachable code. |
|
Third time's a charm. ❤️ |
Settings: add support for search paths
| len *= 2; | ||
| buf = xrealloc(buf, len); | ||
|
|
||
| error = git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, level, buf, len); |
There was a problem hiding this comment.
For another PR: @carlosmn can we change this opts API to take a pointer to gh_buf instead? It would make it much more usable.
There was a problem hiding this comment.
There is already a PR open in libgit2: libgit2/libgit2#2200
There was a problem hiding this comment.
Indeed, I opened the PR when I was adding this to pygit2.
Let the user set and get the config search paths
This should help alleviate some of the issues with libgit2/libgit2#2122 while not making the library ignore errors.