From 155f641683f09ec1a9511f42867ae3f278a6800b Mon Sep 17 00:00:00 2001 From: Jesse Ezell Date: Fri, 21 Mar 2014 22:16:26 -0700 Subject: [PATCH] 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 | 279 ++++++++++++++++++++++++++++++++++---------------- diff_test.go | 32 ++++-- patch.go | 5 +- patch_test.go | 2 +- push_test.go | 2 - wrapper.c | 5 + 6 files changed, 223 insertions(+), 102 deletions(-) diff --git a/diff.go b/diff.go index c34d043..68ea9af 100644 --- a/diff.go +++ b/diff.go @@ -4,9 +4,11 @@ package git #include 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 { - return &DiffFile{ +func diffFileFromC(file *C.git_diff_file) 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 - NewFile *DiffFile + OldFile DiffFile + NewFile DiffFile } -func newDiffDeltaFromC(delta *C.git_diff_delta) *DiffDelta { - return &DiffDelta{ +func diffDeltaFromC(delta *C.git_diff_delta) DiffDelta { + return DiffDelta{ Status: Delta(delta.status), Flags: DiffFlag(delta.flags), Similarity: uint16(delta.similarity), - OldFile: newDiffFileFromC(&delta.old_file), - NewFile: newDiffFileFromC(&delta.new_file), + OldFile: diffFileFromC(&delta.old_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 { - return &DiffHunk{ - OldStart: int(hunk.old_start), - OldLines: int(hunk.old_lines), - NewStart: int(hunk.new_start), - NewLines: int(hunk.new_lines), - Header: C.GoStringN(&hunk.header[0], C.int(hunk.header_len)), - DiffDelta: *newDiffDeltaFromC(delta), +func diffHunkFromC(delta *C.git_diff_delta, hunk *C.git_diff_hunk) DiffHunk { + return DiffHunk{ + OldStart: int(hunk.old_start), + OldLines: int(hunk.old_lines), + NewStart: int(hunk.new_start), + NewLines: int(hunk.new_lines), + Header: C.GoStringN(&hunk.header[0], C.int(hunk.header_len)), } } @@ -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 { - return &DiffLine{ +func diffLineFromC(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line) 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) - -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 -} +type DiffForEachHunkCallback func(DiffHunk) (DiffForEachLineCallback, error) //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 - -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 -} +type DiffForEachLineCallback func(DiffLine) error //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 --git a/diff_test.go b/diff_test.go index f3a1ea6..b688294 100644 --- a/diff_test.go +++ b/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) - lines := make([]*DiffLine, 0) - err = diff.ForEach(func(file *DiffDelta, progress float64) (DiffForEachHunkCallback, error) { + hunks := make([]DiffHunk, 0) + lines := make([]DiffLine, 0) + 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") diff --git a/patch.go b/patch.go index d927109..0665501 100644 --- a/patch.go +++ b/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 } diff --git a/patch_test.go b/patch_test.go index f816068..569eac2 100644 --- a/patch_test.go +++ b/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) diff --git a/push_test.go b/push_test.go index c1e6a22..65f4dd2 100644 --- a/push_test.go +++ b/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) diff --git a/wrapper.c b/wrapper.c index 5d68df5..a8b1432 100644 --- a/wrapper.c +++ b/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);