From d78036fe24c9c746d3513f9e97f463f995aef0fd Mon Sep 17 00:00:00 2001 From: Jesse Ezell Date: Thu, 20 Mar 2014 22:54:18 -0700 Subject: [PATCH] refactor and cleanup code --- diff.go | 41 +++++++++++++++++++++++------------------ diff_test.go | 30 +++++++++++++++++++++++------- git_test.go | 3 ++- patch.go | 8 ++++---- patch_test.go | 34 ++++++++++++++++++++++++++++++++++ repository.go | 2 +- wrapper.c | 4 +--- 7 files changed, 88 insertions(+), 34 deletions(-) create mode 100644 patch_test.go diff --git a/diff.go b/diff.go index 31000bc..014cff5 100644 --- a/diff.go +++ b/diff.go @@ -16,7 +16,7 @@ type DiffFlag int const ( DiffFlagBinary DiffFlag = C.GIT_DIFF_FLAG_BINARY DiffFlagNotBinary = C.GIT_DIFF_FLAG_NOT_BINARY - DiffFlagValidOid = C.GIT_DIFF_FLAG_VALID_OID + DiffFlagValidOid = C.GIT_DIFF_FLAG_VALID_ID ) type Delta int @@ -59,7 +59,7 @@ type DiffFile struct { func newDiffFileFromC(file *C.git_diff_file) *DiffFile { return &DiffFile{ Path: C.GoString(file.path), - Oid: newOidFromC(&file.oid), + Oid: newOidFromC(&file.id), Size: int(file.size), Flags: DiffFlag(file.flags), Mode: uint16(file.mode), @@ -142,7 +142,7 @@ func newDiffFromC(ptr *C.git_diff) *Diff { } func (diff *Diff) Free() error { - if diff.ptr != nil { + if diff.ptr == nil { return ErrInvalid } runtime.SetFinalizer(diff, nil) @@ -150,20 +150,22 @@ func (diff *Diff) Free() error { return nil } +type DiffForEachFileCallback func(*DiffDelta) error + type diffForEachFileData struct { Callback DiffForEachFileCallback Error error } func (diff *Diff) ForEachFile(cb DiffForEachFileCallback) error { - if diff.ptr != nil { + if diff.ptr == nil { return ErrInvalid } data := &diffForEachFileData{ Callback: cb, } - ecode := C._go_git_diff_foreach(diff.ptr, 1, 0, 0, unsafe.Pointer(&data)) + ecode := C._go_git_diff_foreach(diff.ptr, 1, 0, 0, unsafe.Pointer(data)) if ecode < 0 { return data.Error } @@ -172,7 +174,7 @@ func (diff *Diff) ForEachFile(cb DiffForEachFileCallback) error { //export diffForEachFileCb func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, payload unsafe.Pointer) int { - data := *diffForEachFileData(payload) + data := (*diffForEachFileData)(payload) err := data.Callback(newDiffDeltaFromC(delta)) if err != nil { @@ -191,7 +193,7 @@ type diffForEachHunkData struct { type DiffForEachHunkCallback func(*DiffHunk) error func (diff *Diff) ForEachHunk(cb DiffForEachHunkCallback) error { - if diff.ptr != nil { + if diff.ptr == nil { return ErrInvalid } data := &diffForEachHunkData{ @@ -206,10 +208,10 @@ func (diff *Diff) ForEachHunk(cb DiffForEachHunkCallback) error { //export diffForEachHunkCb func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, payload unsafe.Pointer) int { - data := *diffForEachHunkData(payload) + data := (*diffForEachHunkData)(payload) err := data.Callback(newDiffHunkFromC(delta, hunk)) - if err < 0 { + if err != nil { data.Error = err return -1 } @@ -225,7 +227,7 @@ type diffForEachLineData struct { type DiffForEachLineCallback func(*DiffLine) error func (diff *Diff) ForEachLine(cb DiffForEachLineCallback) error { - if diff.ptr != nil { + if diff.ptr == nil { return ErrInvalid } @@ -243,7 +245,7 @@ func (diff *Diff) ForEachLine(cb DiffForEachLineCallback) 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 := *diffForEachLineData(payload) + data := (*diffForEachLineData)(payload) err := data.Callback(newDiffLineFromC(delta, hunk, line)) if err != nil { @@ -255,26 +257,26 @@ func diffForEachLineCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.g } func (diff *Diff) NumDeltas() (int, error) { - if diff.ptr != nil { + 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 { + if diff.ptr == nil { return nil, ErrInvalid } ptr := C.git_diff_get_delta(diff.ptr, C.size_t(index)) if ptr == nil { - return nil + return nil, nil } return newDiffDeltaFromC(ptr), nil } func (diff *Diff) Patch(deltaIndex int) (*Patch, error) { - if diff.ptr != nil { + if diff.ptr == nil { return nil, ErrInvalid } var patchPtr *C.git_patch @@ -287,7 +289,7 @@ func (diff *Diff) Patch(deltaIndex int) (*Patch, error) { return newPatchFromC(patchPtr), nil } -func (v *Repository) DiffTreeToTree(oldTree, newTree *Tree) *Diff { +func (v *Repository) DiffTreeToTree(oldTree, newTree *Tree) (*Diff, error) { var diffPtr *C.git_diff var oldPtr, newPtr *C.git_tree @@ -299,7 +301,10 @@ func (v *Repository) DiffTreeToTree(oldTree, newTree *Tree) *Diff { newPtr = newTree.gitObject.ptr } - C.git_diff_tree_to_tree(&diffPtr, v.ptr, oldPtr, newPtr, nil) + ecode := C.git_diff_tree_to_tree(&diffPtr, v.ptr, oldPtr, newPtr, nil) + if ecode < 0 { + return nil, MakeGitError(ecode) + } - return newDiff(diffPtr) + return newDiffFromC(diffPtr), nil } diff --git a/diff_test.go b/diff_test.go index 15aabc5..6ddd433 100644 --- a/diff_test.go +++ b/diff_test.go @@ -1,41 +1,57 @@ package git import ( + "errors" "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) checkFatal(t, err) - updateReadme(t, repo, "file changed\n") - _, newTreeId := seedTestRepo(t, repo) + _, newTreeId := updateReadme(t, repo, "file changed\n") + newTree, err := repo.LookupTree(newTreeId) checkFatal(t, err) - diff, err := repo.DiffTreeToTree(originalTreeId, newTreeId) + diff, err := repo.DiffTreeToTree(originalTree, newTree) checkFatal(t, err) + if diff == nil { + t.Fatal("no diff returned") + } + files := make([]string, 0) - err := diff.ForEachFile(func(file *DiffFile) error { - files = append(files, file.Path) + err = diff.ForEachFile(func(file *DiffDelta) error { + files = append(files, file.OldFile.Path) return nil }) checkFatal(t, err) - if len(files) != 0 { + if len(files) != 1 { t.Fatal("Incorrect number of files in diff") } if files[0] != "README" { t.Fatal("File in diff was expected to be README") } + + errTest := errors.New("test error") + + err = diff.ForEachFile(func(file *DiffDelta) error { + return errTest + }) + + if err != errTest { + t.Fatal("Expected custom error to be returned") + } + } diff --git a/git_test.go b/git_test.go index e6372fb..6a3aeaa 100644 --- a/git_test.go +++ b/git_test.go @@ -2,6 +2,7 @@ package git import ( "io/ioutil" + "path" "testing" "time" ) @@ -66,7 +67,7 @@ func updateReadme(t *testing.T, repo *Repository, content string) (*Oid, *Oid) { } tmpfile := "README" - err = ioutil.WriteFile(repo.Path()+"/"+tmpfile, []byte(content), 0644) + err = ioutil.WriteFile(path.Join(path.Dir(path.Dir(repo.Path())), tmpfile), []byte(content), 0644) checkFatal(t, err) idx, err := repo.Index() diff --git a/patch.go b/patch.go index f2016c4..880f088 100644 --- a/patch.go +++ b/patch.go @@ -35,10 +35,10 @@ func (patch *Patch) Free() error { } func (patch *Patch) String() (string, error) { - if diff.ptr != nil { + if patch.ptr == nil { return "", ErrInvalid } - var cptr *C.char - C.git_patch_to_str(&cptr, patch.ptr) - return C.GoString(cptr), nil + var buf C.git_buf + C.git_patch_to_buf(&buf, patch.ptr) + return C.GoString(buf.ptr), nil } diff --git a/patch_test.go b/patch_test.go new file mode 100644 index 0000000..f816068 --- /dev/null +++ b/patch_test.go @@ -0,0 +1,34 @@ +package git + +import ( + "strings" + "testing" +) + +func TestPatch(t *testing.T) { + repo := createTestRepo(t) + defer repo.Free() + //defer os.RemoveAll(repo.Workdir()) + + _, originalTreeId := seedTestRepo(t, repo) + originalTree, err := repo.LookupTree(originalTreeId) + + checkFatal(t, err) + + _, newTreeId := updateReadme(t, repo, "file changed\n") + + newTree, err := repo.LookupTree(newTreeId) + checkFatal(t, err) + + diff, err := repo.DiffTreeToTree(originalTree, newTree) + checkFatal(t, err) + + patch, err := diff.Patch(0) + checkFatal(t, err) + + patchStr, err := patch.String() + checkFatal(t, err) + if strings.Index(patchStr, "diff --git a/README b/README\nindex 257cc56..820734a 100644\n--- a/README\n+++ b/README\n@@ -1 +1 @@\n-foo\n+file changed") == -1 { + t.Fatalf("patch was bad") + } +} diff --git a/repository.go b/repository.go index 50053d9..3d6e59a 100644 --- a/repository.go +++ b/repository.go @@ -154,7 +154,7 @@ func (v *Repository) Head() (*Reference, error) { ecode := C.git_repository_head(&ptr, v.ptr) if ecode < 0 { - return nil, LastError() + return nil, MakeGitError(ecode) } return newReferenceFromC(ptr), nil diff --git a/wrapper.c b/wrapper.c index affa8d6..5d68df5 100644 --- a/wrapper.c +++ b/wrapper.c @@ -25,7 +25,6 @@ int _go_git_odb_foreach(git_odb *db, void *payload) return git_odb_foreach(db, (git_odb_foreach_cb)&odbForEachCb, payload); } -<<<<<<< HEAD int _go_git_diff_foreach(git_diff *diff, int eachFile, int eachHunk, int eachLine, void *payload) { git_diff_file_cb fcb = NULL; @@ -45,7 +44,7 @@ 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_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); @@ -82,6 +81,5 @@ int _go_git_blob_create_fromchunks(git_oid *id, void *payload) { return git_blob_create_fromchunks(id, repo, hintpath, _go_blob_chunk_cb, payload); ->>>>>>> 2811845a1287d949a74b8ed80a5791fd8875002a } /* EOF */