implemented Index.AddAll as git_index_add_all wrapper #125

2014-10-21T17:25:20-05:00

lucas-clemente commented

2014-10-21 17:25:20 -05:00

(Migrated from github.com)

The callback is currently ignored. I'm not sure how to implement it properly, probably you should take a look before merging.

carlosmn commented

2014-10-23 12:40:30 -05:00

(Migrated from github.com)

The callback would likely have to go through C in wrapper.c. What we'd have to do here is have libgit2 call a specific function which would take the callback parameter as its payload and then call that. We have the payload in the C API because we don't have closures there, but we shouldn't expose that for Go. If the user wants to have a context, they should use a closure in go. We then use this payload argument for the exposed version to know what to dispatch to. Something like

// export addAllCallback
func addAllCallback(cpath, cmatched_pathspec *C.char, payload unsafe.Pointer) {
    callback := IndexMatchedPathCallback(payload)
    return callback(C.GoString(cpath), C.GoString(cmatched_pathspec)
}

and you pass in addAllCallback as the callback to git_index_add_all() and the Go function callback as the payload.

The callback would likely have to go through C in `wrapper.c`. What we'd have to do here is have libgit2 call a specific function which would take the `callback` parameter as its payload and then call that. We have the payload in the C API because we don't have closures there, but we shouldn't expose that for Go. If the user wants to have a context, they should use a closure in go. We then use this payload argument for the exposed version to know what to dispatch to. Something like ``` go // export addAllCallback func addAllCallback(cpath, cmatched_pathspec *C.char, payload unsafe.Pointer) { callback := IndexMatchedPathCallback(payload) return callback(C.GoString(cpath), C.GoString(cmatched_pathspec) } ``` and you pass in `addAllCallback` as the callback to `git_index_add_all()` and the Go function `callback` as the payload.

lucas-clemente commented

2014-10-24 04:27:02 -05:00

(Migrated from github.com)

Okay, I think i've implemented callbacks the way you meant, could you please double-check? I also added UpdateAll and RemoveAll.

lucas-clemente commented

2014-10-24 04:28:46 -05:00

(Migrated from github.com)

Oh, and I also removed the payload parameter from the callback. I don't think it's necessary if you can just capture stuff within the closure.

lucas-clemente commented

2014-10-24 14:57:39 -05:00

(Migrated from github.com)

Sorry for taking a few tries, I didn't try go 1.1 before. I've rebased everything, fixed the thing with the const (you were of course right 😄), so it should hopefully be complete now :)

Sorry for taking a few tries, I didn't try go 1.1 before. I've rebased everything, fixed the thing with the const (you were of course right :smile:), so it should hopefully be complete now :)

carlosmn commented

2014-10-25 11:11:53 -05:00

(Migrated from github.com)

Neat, thanks!

Sign in to join this conversation.

No reviewers