don't expose 3 different diff foreach methods. use structures instead of pointers to structures for diff detail. add patch error code handling. trim excess data from diff structures.

diff.go

@@ -4,9 +4,11 @@ package git
 #include <git2.h>
 
 extern int _go_git_diff_foreach(git_diff *diff, int eachFile, int eachHunk, int eachLine, void *payload);
+extern void _go_git_setup_diff_notify_callbacks(git_diff_options* opts);
 */
 import "C"
 import (
+	"errors"
 	"runtime"
 	"unsafe"
 )
@@ -56,8 +58,8 @@ type DiffFile struct {
 	Mode  uint16
 }
 
-func newDiffFileFromC(file *C.git_diff_file) *DiffFile {
+func diffFileFromC(file *C.git_diff_file) DiffFile {
-	return &DiffFile{
+	return DiffFile{
 		Path:  C.GoString(file.path),
 		Oid:   newOidFromC(&file.id),
 		Size:  int(file.size),
@@ -70,17 +72,17 @@ type DiffDelta struct {
 	Status     Delta
 	Flags      DiffFlag
 	Similarity uint16
-	OldFile    *DiffFile
+	OldFile    DiffFile
-	NewFile    *DiffFile
+	NewFile    DiffFile
 }
 
-func newDiffDeltaFromC(delta *C.git_diff_delta) *DiffDelta {
+func diffDeltaFromC(delta *C.git_diff_delta) DiffDelta {
-	return &DiffDelta{
+	return DiffDelta{
 		Status:     Delta(delta.status),
 		Flags:      DiffFlag(delta.flags),
 		Similarity: uint16(delta.similarity),
-		OldFile:    newDiffFileFromC(&delta.old_file),
+		OldFile:    diffFileFromC(&delta.old_file),
-		NewFile:    newDiffFileFromC(&delta.new_file),
+		NewFile:    diffFileFromC(&delta.new_file),
 	}
 }
 
@@ -90,17 +92,15 @@ type DiffHunk struct {
 	NewStart int
 	NewLines int
 	Header   string
-	DiffDelta
 }
 
-func newDiffHunkFromC(delta *C.git_diff_delta, hunk *C.git_diff_hunk) *DiffHunk {
+func diffHunkFromC(delta *C.git_diff_delta, hunk *C.git_diff_hunk) DiffHunk {
-	return &DiffHunk{
+	return DiffHunk{
-		OldStart:  int(hunk.old_start),
+		OldStart: int(hunk.old_start),
-		OldLines:  int(hunk.old_lines),
+		OldLines: int(hunk.old_lines),
-		NewStart:  int(hunk.new_start),
+		NewStart: int(hunk.new_start),
-		NewLines:  int(hunk.new_lines),
+		NewLines: int(hunk.new_lines),
-		Header:    C.GoStringN(&hunk.header[0], C.int(hunk.header_len)),
+		Header:   C.GoStringN(&hunk.header[0], C.int(hunk.header_len)),
-		DiffDelta: *newDiffDeltaFromC(delta),
 	}
 }
 
@@ -110,17 +110,15 @@ type DiffLine struct {
 	NewLineno int
 	NumLines  int
 	Content   string
-	DiffHunk
 }
 
-func newDiffLineFromC(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line) *DiffLine {
+func diffLineFromC(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line) DiffLine {
-	return &DiffLine{
+	return DiffLine{
 		Origin:    DiffLineType(line.origin),
 		OldLineno: int(line.old_lineno),
 		NewLineno: int(line.new_lineno),
 		NumLines:  int(line.num_lines),
 		Content:   C.GoStringN(line.content, C.int(line.content_len)),
-		DiffHunk:  *newDiffHunkFromC(delta, hunk),
 	}
 }
 
@@ -128,6 +126,21 @@ type Diff struct {
 	ptr *C.git_diff
 }
 
+func (diff *Diff) NumDeltas() (int, error) {
+	if diff.ptr == nil {
+		return -1, ErrInvalid
+	}
+	return int(C.git_diff_num_deltas(diff.ptr)), nil
+}
+
+func (diff *Diff) GetDelta(index int) (DiffDelta, error) {
+	if diff.ptr == nil {
+		return DiffDelta{}, ErrInvalid
+	}
+	ptr := C.git_diff_get_delta(diff.ptr, C.size_t(index))
+	return diffDeltaFromC(ptr), nil
+}
+
 func newDiffFromC(ptr *C.git_diff) *Diff {
 	if ptr == nil {
 		return nil
@@ -158,20 +171,28 @@ type diffForEachData struct {
 	Error        error
 }
 
-type DiffForEachFileCallback func(*DiffDelta, float64) (DiffForEachHunkCallback, error)
+type DiffForEachFileCallback func(DiffDelta, float64) (DiffForEachHunkCallback, error)
 
-func (diff *Diff) ForEach(cbFile DiffForEachFileCallback, diffHunks bool, diffLines bool) error {
+type DiffDetail int
+
+const (
+	DiffDetailFiles DiffDetail = iota
+	DiffDetailHunks
+	DiffDetailLines
+)
+
+func (diff *Diff) ForEach(cbFile DiffForEachFileCallback, detail DiffDetail) error {
 	if diff.ptr == nil {
 		return ErrInvalid
 	}
 
 	intHunks := C.int(0)
-	if diffHunks {
+	if detail >= DiffDetailHunks {
 		intHunks = C.int(1)
 	}
 
 	intLines := C.int(0)
-	if diffLines {
+	if detail >= DiffDetailLines {
 		intLines = C.int(1)
 	}
 
@@ -191,7 +212,7 @@ func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, payload unsafe
 
 	data.HunkCallback = nil
 	if data.FileCallback != nil {
-		cb, err := data.FileCallback(newDiffDeltaFromC(delta), float64(progress))
+		cb, err := data.FileCallback(diffDeltaFromC(delta), float64(progress))
 		if err != nil {
 			data.Error = err
 			return -1
@@ -202,27 +223,7 @@ func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, payload unsafe
 	return 0
 }
 
-type DiffForEachHunkCallback func(*DiffHunk) (DiffForEachLineCallback, error)
+type DiffForEachHunkCallback func(DiffHunk) (DiffForEachLineCallback, error)
-
-func (diff *Diff) ForEachHunk(cb DiffForEachHunkCallback, diffLines bool) error {
-	if diff.ptr == nil {
-		return ErrInvalid
-	}
-	data := &diffForEachData{
-		HunkCallback: cb,
-	}
-
-	intLines := C.int(0)
-	if diffLines {
-		intLines = C.int(1)
-	}
-
-	ecode := C._go_git_diff_foreach(diff.ptr, 0, 1, intLines, unsafe.Pointer(data))
-	if ecode < 0 {
-		return data.Error
-	}
-	return nil
-}
 
 //export diffForEachHunkCb
 func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, payload unsafe.Pointer) int {
@@ -230,7 +231,7 @@ func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, payload u
 
 	data.LineCallback = nil
 	if data.HunkCallback != nil {
-		cb, err := data.HunkCallback(newDiffHunkFromC(delta, hunk))
+		cb, err := data.HunkCallback(diffHunkFromC(delta, hunk))
 		if err != nil {
 			data.Error = err
 			return -1
@@ -241,30 +242,14 @@ func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, payload u
 	return 0
 }
 
-type DiffForEachLineCallback func(*DiffLine) error
+type DiffForEachLineCallback func(DiffLine) error
-
-func (diff *Diff) ForEachLine(cb DiffForEachLineCallback) error {
-	if diff.ptr == nil {
-		return ErrInvalid
-	}
-
-	data := &diffForEachData{
-		LineCallback: cb,
-	}
-
-	ecode := C._go_git_diff_foreach(diff.ptr, 0, 0, 1, unsafe.Pointer(data))
-	if ecode < 0 {
-		return data.Error
-	}
-	return nil
-}
 
 //export diffForEachLineCb
 func diffForEachLineCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line, payload unsafe.Pointer) int {
 
 	data := (*diffForEachData)(payload)
 
-	err := data.LineCallback(newDiffLineFromC(delta, hunk, line))
+	err := data.LineCallback(diffLineFromC(delta, hunk, line))
 	if err != nil {
 		data.Error = err
 		return -1
@@ -273,25 +258,6 @@ func diffForEachLineCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.g
 	return 0
 }
 
-func (diff *Diff) NumDeltas() (int, error) {
-	if diff.ptr == nil {
-		return -1, ErrInvalid
-	}
-	return int(C.git_diff_num_deltas(diff.ptr)), nil
-}
-
-func (diff *Diff) GetDelta(index int) (*DiffDelta, error) {
-	if diff.ptr == nil {
-		return nil, ErrInvalid
-	}
-	ptr := C.git_diff_get_delta(diff.ptr, C.size_t(index))
-	if ptr == nil {
-		return nil, nil
-	}
-
-	return newDiffDeltaFromC(ptr), nil
-}
-
 func (diff *Diff) Patch(deltaIndex int) (*Patch, error) {
 	if diff.ptr == nil {
 		return nil, ErrInvalid
@@ -306,7 +272,109 @@ func (diff *Diff) Patch(deltaIndex int) (*Patch, error) {
 	return newPatchFromC(patchPtr), nil
 }
 
-func (v *Repository) DiffTreeToTree(oldTree, newTree *Tree) (*Diff, error) {
+type DiffOptionsFlag int
+
+const (
+	DiffNormal                 DiffOptionsFlag = C.GIT_DIFF_NORMAL
+	DiffReverse                                = C.GIT_DIFF_REVERSE
+	DiffIncludeIgnored                         = C.GIT_DIFF_INCLUDE_IGNORED
+	DiffRecurseIgnoredDirs                     = C.GIT_DIFF_RECURSE_IGNORED_DIRS
+	DiffIncludeUntracked                       = C.GIT_DIFF_INCLUDE_UNTRACKED
+	DiffRecurseUntracked                       = C.GIT_DIFF_RECURSE_UNTRACKED_DIRS
+	DiffIncludeUnmodified                      = C.GIT_DIFF_INCLUDE_UNMODIFIED
+	DiffIncludeTypeChange                      = C.GIT_DIFF_INCLUDE_TYPECHANGE
+	DiffIncludeTypeChangeTrees                 = C.GIT_DIFF_INCLUDE_TYPECHANGE_TREES
+	DiffIgnoreFilemode                         = C.GIT_DIFF_IGNORE_FILEMODE
+	DiffIgnoreSubmodules                       = C.GIT_DIFF_IGNORE_SUBMODULES
+	DiffIgnoreCase                             = C.GIT_DIFF_IGNORE_CASE
+
+	DiffDisablePathspecMatch    = C.GIT_DIFF_DISABLE_PATHSPEC_MATCH
+	DiffSkipBinaryCheck         = C.GIT_DIFF_SKIP_BINARY_CHECK
+	DiffEnableFastUntrackedDirs = C.GIT_DIFF_ENABLE_FAST_UNTRACKED_DIRS
+
+	DiffForceText   = C.GIT_DIFF_FORCE_TEXT
+	DiffForceBinary = C.GIT_DIFF_FORCE_BINARY
+
+	DiffIgnoreWhitespace       = C.GIT_DIFF_IGNORE_WHITESPACE
+	DiffIgnoreWhitespaceChange = C.GIT_DIFF_IGNORE_WHITESPACE_CHANGE
+	DiffIgnoreWitespaceEol     = C.GIT_DIFF_IGNORE_WHITESPACE_EOL
+
+	DiffShowUntrackedContent = C.GIT_DIFF_SHOW_UNTRACKED_CONTENT
+	DiffShowUnmodified       = C.GIT_DIFF_SHOW_UNMODIFIED
+	DiffPatience             = C.GIT_DIFF_PATIENCE
+	DiffMinimal              = C.GIT_DIFF_MINIMAL
+)
+
+type DiffNotifyCallback func(diffSoFar *Diff, deltaToAdd DiffDelta, matchedPathspec string) error
+
+type DiffOptions struct {
+	Flags            DiffOptionsFlag
+	IgnoreSubmodules SubmoduleIgnore
+	Pathspec         []string
+	NotifyCallback   DiffNotifyCallback
+
+	ContextLines   uint16
+	InterhunkLines uint16
+	IdAbbrev       uint16
+
+	MaxSize int
+
+	OldPrefix string
+	NewPrefix string
+}
+
+func DefaultDiffOptions() (DiffOptions, error) {
+	opts := C.git_diff_options{}
+	ecode := C.git_diff_init_options(&opts, C.GIT_DIFF_OPTIONS_VERSION)
+	if ecode < 0 {
+		return DiffOptions{}, MakeGitError(ecode)
+	}
+
+	return DiffOptions{
+		Flags:            DiffOptionsFlag(opts.flags),
+		IgnoreSubmodules: SubmoduleIgnore(opts.ignore_submodules),
+		Pathspec:         makeStringsFromCStrings(opts.pathspec.strings, int(opts.pathspec.count)),
+		ContextLines:     uint16(opts.context_lines),
+		InterhunkLines:   uint16(opts.interhunk_lines),
+		IdAbbrev:         uint16(opts.id_abbrev),
+		MaxSize:          int(opts.max_size),
+	}, nil
+}
+
+var (
+	ErrDeltaSkip = errors.New("Skip delta")
+)
+
+type diffNotifyData struct {
+	Callback DiffNotifyCallback
+	Diff     *Diff
+	Error    error
+}
+
+//export diffNotifyCb
+func diffNotifyCb(_diff_so_far unsafe.Pointer, delta_to_add *C.git_diff_delta, matched_pathspec *C.char, payload unsafe.Pointer) int {
+	diff_so_far := (*C.git_diff)(_diff_so_far)
+	data := (*diffNotifyData)(payload)
+	if data != nil {
+		if data.Diff == nil {
+			data.Diff = newDiffFromC(diff_so_far)
+		}
+
+		err := data.Callback(data.Diff, diffDeltaFromC(delta_to_add), C.GoString(matched_pathspec))
+
+		if err == ErrDeltaSkip {
+			return 1
+		} else if err != nil {
+			data.Error = err
+			return -1
+		} else {
+			return 0
+		}
+	}
+	return 0
+}
+
+func (v *Repository) DiffTreeToTree(oldTree, newTree *Tree, opts *DiffOptions) (*Diff, error) {
 	var diffPtr *C.git_diff
 	var oldPtr, newPtr *C.git_tree
 
@@ -318,10 +386,45 @@ func (v *Repository) DiffTreeToTree(oldTree, newTree *Tree) (*Diff, error) {
 		newPtr = newTree.gitObject.ptr
 	}
 
-	ecode := C.git_diff_tree_to_tree(&diffPtr, v.ptr, oldPtr, newPtr, nil)
+	cpathspec := C.git_strarray{}
+	var copts *C.git_diff_options
+	var notifyData *diffNotifyData
+	if opts != nil {
+		notifyData = &diffNotifyData{
+			Callback: opts.NotifyCallback,
+		}
+		if opts.Pathspec != nil {
+			cpathspec.count = C.size_t(len(opts.Pathspec))
+			cpathspec.strings = makeCStringsFromStrings(opts.Pathspec)
+			defer freeStrarray(&cpathspec)
+		}
+
+		copts = &C.git_diff_options{
+			version:           C.GIT_DIFF_OPTIONS_VERSION,
+			flags:             C.uint32_t(opts.Flags),
+			ignore_submodules: C.git_submodule_ignore_t(opts.IgnoreSubmodules),
+			pathspec:          cpathspec,
+			context_lines:     C.uint16_t(opts.ContextLines),
+			interhunk_lines:   C.uint16_t(opts.InterhunkLines),
+			id_abbrev:         C.uint16_t(opts.IdAbbrev),
+			max_size:          C.git_off_t(opts.MaxSize),
+		}
+
+		if opts.NotifyCallback != nil {
+			C._go_git_setup_diff_notify_callbacks(copts)
+			copts.notify_payload = unsafe.Pointer(notifyData)
+		}
+	}
+
+	ecode := C.git_diff_tree_to_tree(&diffPtr, v.ptr, oldPtr, newPtr, copts)
 	if ecode < 0 {
 		return nil, MakeGitError(ecode)
 	}
 
-	return newDiffFromC(diffPtr), nil
+	if notifyData != nil && notifyData.Diff != nil {
+		return notifyData.Diff, nil
+	} else {
+		return newDiffFromC(diffPtr), nil
+	}
+
 }

diff_test.go

@@ -2,13 +2,14 @@ package git
 
 import (
 	"errors"
+	"os"
 	"testing"
 )
 
 func TestDiffTreeToTree(t *testing.T) {
 	repo := createTestRepo(t)
 	defer repo.Free()
-	//defer os.RemoveAll(repo.Workdir())
+	defer os.RemoveAll(repo.Workdir())
 
 	_, originalTreeId := seedTestRepo(t, repo)
 	originalTree, err := repo.LookupTree(originalTreeId)
@@ -20,26 +21,37 @@ func TestDiffTreeToTree(t *testing.T) {
 	newTree, err := repo.LookupTree(newTreeId)
 	checkFatal(t, err)
 
-	diff, err := repo.DiffTreeToTree(originalTree, newTree)
+	callbackInvoked := false
+	opts := DiffOptions{
+		NotifyCallback: func(diffSoFar *Diff, delta DiffDelta, matchedPathSpec string) error {
+			callbackInvoked = true
+			return nil
+		},
+	}
+
+	diff, err := repo.DiffTreeToTree(originalTree, newTree, &opts)
 	checkFatal(t, err)
+	if !callbackInvoked {
+		t.Fatal("callback not invoked")
+	}
 
 	if diff == nil {
 		t.Fatal("no diff returned")
 	}
 
 	files := make([]string, 0)
-	hunks := make([]*DiffHunk, 0)
+	hunks := make([]DiffHunk, 0)
-	lines := make([]*DiffLine, 0)
+	lines := make([]DiffLine, 0)
-	err = diff.ForEach(func(file *DiffDelta, progress float64) (DiffForEachHunkCallback, error) {
+	err = diff.ForEach(func(file DiffDelta, progress float64) (DiffForEachHunkCallback, error) {
 		files = append(files, file.OldFile.Path)
-		return func(hunk *DiffHunk) (DiffForEachLineCallback, error) {
+		return func(hunk DiffHunk) (DiffForEachLineCallback, error) {
 			hunks = append(hunks, hunk)
-			return func(line *DiffLine) error {
+			return func(line DiffLine) error {
 				lines = append(lines, line)
 				return nil
 			}, nil
 		}, nil
-	}, true, true)
+	}, DiffDetailLines)
 
 	checkFatal(t, err)
 
@@ -73,9 +85,9 @@ func TestDiffTreeToTree(t *testing.T) {
 
 	errTest := errors.New("test error")
 
-	err = diff.ForEach(func(file *DiffDelta, progress float64) (DiffForEachHunkCallback, error) {
+	err = diff.ForEach(func(file DiffDelta, progress float64) (DiffForEachHunkCallback, error) {
 		return nil, errTest
-	}, false, false)
+	}, DiffDetailLines)
 
 	if err != errTest {
 		t.Fatal("Expected custom error to be returned")

patch.go

@@ -40,6 +40,9 @@ func (patch *Patch) String() (string, error) {
 		return "", ErrInvalid
 	}
 	var buf C.git_buf
-	C.git_patch_to_buf(&buf, patch.ptr)
+	ecode := C.git_patch_to_buf(&buf, patch.ptr)
+	if ecode < 0 {
+		return "", MakeGitError(ecode)
+	}
 	return C.GoString(buf.ptr), nil
 }

patch_test.go

@@ -20,7 +20,7 @@ func TestPatch(t *testing.T) {
 	newTree, err := repo.LookupTree(newTreeId)
 	checkFatal(t, err)
 
-	diff, err := repo.DiffTreeToTree(originalTree, newTree)
+	diff, err := repo.DiffTreeToTree(originalTree, newTree, nil)
 	checkFatal(t, err)
 
 	patch, err := diff.Patch(0)

push_test.go

@@ -1,7 +1,6 @@
 package git
 
 import (
-	"log"
 	"os"
 	"testing"
 	"time"
@@ -45,7 +44,6 @@ func Test_Push_ToRemote(t *testing.T) {
 	checkFatal(t, err)
 
 	err = push.StatusForeach(func(ref string, msg string) int {
-		log.Printf("%s -> %s", ref, msg)
 		return 0
 	})
 	checkFatal(t, err)

wrapper.c

@@ -45,6 +45,11 @@ int _go_git_diff_foreach(git_diff *diff, int eachFile, int eachHunk, int eachLin
 
 	return git_diff_foreach(diff, fcb, hcb, lcb, payload);
 }
+
+void _go_git_setup_diff_notify_callbacks(git_diff_options *opts) {
+	opts->notify_cb = (git_diff_notify_cb)diffNotifyCb;
+}
+
 void _go_git_setup_callbacks(git_remote_callbacks *callbacks) {
 	typedef int (*progress_cb)(const char *str, int len, void *data);
 	typedef int (*completion_cb)(git_remote_completion_type type, void *data);