remove Branch struct, unify reference iterators

branch.go

@@ -19,26 +19,27 @@ const (
 	BranchRemote            = C.GIT_BRANCH_REMOTE
 )
 
-type Branch struct {
+type branchIterator struct {
-	Reference
-}
-
-type BranchIterator struct {
 	ptr *C.git_branch_iterator
 }
 
-func newBranchIteratorFromC(ptr *C.git_branch_iterator) *BranchIterator {
+func newBranchIteratorFromC(ptr *C.git_branch_iterator) ReferenceIterator {
-	i := &BranchIterator{ptr: ptr}
+	i := &branchIterator{ptr: ptr}
-	runtime.SetFinalizer(i, (*BranchIterator).Free)
+	runtime.SetFinalizer(i, (*branchIterator).Free)
 	return i
 }
 
-func (i *BranchIterator) Next() (*Reference, error) {
+func (i *branchIterator) NextName() (string, error) {
-	ref, _, err := i.NextWithType()
+	ref, err := i.Next()
-	return ref, err
+	if err != nil {
+		return "", err
+	}
+	defer ref.Free()
+
+	return ref.Name(), err
 }
 
-func (i *BranchIterator) NextWithType() (*Reference, BranchType, error) {
+func (i *branchIterator) Next() (*Reference, error) {
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
@@ -49,20 +50,20 @@ func (i *BranchIterator) NextWithType() (*Reference, BranchType, error) {
 	ecode := C.git_branch_next(&refPtr, &refType, i.ptr)
 
 	if ecode == C.GIT_ITEROVER {
-		return nil, BranchLocal, ErrIterOver
+		return nil, ErrIterOver
 	} else if ecode < 0 {
-		return nil, BranchLocal, MakeGitError(ecode)
+		return nil, MakeGitError(ecode)
 	}
 
-	return newReferenceFromC(refPtr), BranchType(refType), nil
+	return newReferenceFromC(refPtr), nil
 }
 
-func (i *BranchIterator) Free() {
+func (i *branchIterator) Free() {
 	runtime.SetFinalizer(i, nil)
 	C.git_branch_iterator_free(i.ptr)
 }
 
-func (repo *Repository) NewBranchIterator(flags BranchType) (*BranchIterator, error) {
+func (repo *Repository) NewBranchIterator(flags BranchType) (ReferenceIterator, error) {
 
 	refType := C.git_branch_t(flags)
 	var ptr *C.git_branch_iterator
@@ -105,7 +106,7 @@ func (repo *Repository) CreateBranch(branchName string, target *Commit, force bo
 	return ref, nil
 }
 
-func (b *Branch) Delete() error {
+func (b *Reference) DeleteBranch() error {
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
@@ -116,8 +117,8 @@ func (b *Branch) Delete() error {
 	return nil
 }
 
-func (b *Branch) Move(newBranchName string, force bool, signature *Signature, msg string) (*Branch, error) {
+func (b *Reference) MoveBranch(newBranchName string, force bool, signature *Signature, msg string) (*Reference, error) {
-	newBranch := new(Branch)
+	var ptr *C.git_reference
 	cNewBranchName := C.CString(newBranchName)
 	cForce := cbool(force)
 
@@ -135,14 +136,14 @@ func (b *Branch) Move(newBranchName string, force bool, signature *Signature, ms
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ret := C.git_branch_move(&newBranch.ptr, b.ptr, cNewBranchName, cForce, cSignature, cmsg)
+	ret := C.git_branch_move(&ptr, b.ptr, cNewBranchName, cForce, cSignature, cmsg)
 	if ret < 0 {
 		return nil, MakeGitError(ret)
 	}
-	return newBranch, nil
+	return newReferenceFromC(ptr), nil
 }
 
-func (b *Branch) IsHead() (bool, error) {
+func (b *Reference) IsBranchHead() (bool, error) {
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
@@ -158,21 +159,22 @@ func (b *Branch) IsHead() (bool, error) {
 
 }
 
-func (repo *Repository) LookupBranch(branchName string, bt BranchType) (*Branch, error) {
+func (repo *Repository) LookupBranch(branchName string, bt BranchType) (*Reference, error) {
-	branch := new(Branch)
+	var ptr *C.git_reference
+
 	cName := C.CString(branchName)
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ret := C.git_branch_lookup(&branch.ptr, repo.ptr, cName, C.git_branch_t(bt))
+	ret := C.git_branch_lookup(&ptr, repo.ptr, cName, C.git_branch_t(bt))
 	if ret < 0 {
 		return nil, MakeGitError(ret)
 	}
-	return branch, nil
+	return newReferenceFromC(ptr), nil
 }
 
-func (b *Branch) Name() (string, error) {
+func (b *Reference) BranchName() (string, error) {
 	var cName *C.char
 	defer C.free(unsafe.Pointer(cName))
 
@@ -204,7 +206,7 @@ func (repo *Repository) RemoteName(canonicalBranchName string) (string, error) {
 	return C.GoString(nameBuf.ptr), nil
 }
 
-func (b *Branch) SetUpstream(upstreamName string) error {
+func (b *Reference) SetUpstream(upstreamName string) error {
 	cName := C.CString(upstreamName)
 
 	runtime.LockOSThread()
@@ -217,17 +219,17 @@ func (b *Branch) SetUpstream(upstreamName string) error {
 	return nil
 }
 
-func (b *Branch) Upstream() (*Branch, error) {
+func (b *Reference) Upstream() (*Reference, error) {
-	upstream := new(Branch)
 
+	var ptr *C.git_reference
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ret := C.git_branch_upstream(&upstream.ptr, b.ptr)
+	ret := C.git_branch_upstream(&ptr, b.ptr)
 	if ret < 0 {
 		return nil, MakeGitError(ret)
 	}
-	return upstream, nil
+	return newReferenceFromC(ptr), nil
 }
 
 func (repo *Repository) UpstreamName(canonicalBranchName string) (string, error) {

branch_test.go

@@ -4,7 +4,7 @@ import (
 	"testing"
 )
 
-func Test_List_Branches(t *testing.T) {
+func TestBranchIterator(t *testing.T) {
 
 	repo := createTestRepo(t)
 	seedTestRepo(t, repo)
@@ -25,4 +25,23 @@ func Test_List_Branches(t *testing.T) {
 	if err != ErrIterOver {
 		t.Fatal("expected iterover")
 	}
+
+	// test channel iterator
+
+	i, err = repo.NewBranchIterator(BranchLocal)
+	checkFatal(t, err)
+
+	list := make([]string, 0)
+	for ref := range ReferenceNameIteratorChannel(i) {
+		list = append(list, ref)
+	}
+
+	if len(list) != 1 {
+		t.Fatal("expected single match")
+	}
+
+	if list[0] != "refs/heads/master" {
+		t.Fatal("expected refs/heads/master")
+	}
+
 }

reference.go

@@ -185,12 +185,18 @@ func (v *Reference) Free() {
 	C.git_reference_free(v.ptr)
 }
 
-type ReferenceIterator struct {
+type ReferenceIterator interface {
+	Next() (*Reference, error)
+	NextName() (string, error)
+	Free()
+}
+
+type gitReferenceIterator struct {
 	ptr *C.git_reference_iterator
 }
 
 // NewReferenceIterator creates a new iterator over reference names
-func (repo *Repository) NewReferenceIterator() (*ReferenceIterator, error) {
+func (repo *Repository) NewReferenceIterator() (ReferenceIterator, error) {
 	var ptr *C.git_reference_iterator
 
 	runtime.LockOSThread()
@@ -201,15 +207,15 @@ func (repo *Repository) NewReferenceIterator() (*ReferenceIterator, error) {
 		return nil, MakeGitError(ret)
 	}
 
-	iter := &ReferenceIterator{ptr: ptr}
+	iter := &gitReferenceIterator{ptr: ptr}
-	runtime.SetFinalizer(iter, (*ReferenceIterator).Free)
+	runtime.SetFinalizer(iter, (*gitReferenceIterator).Free)
 	return iter, nil
 }
 
 // NewReferenceIteratorGlob creates an iterator over reference names
 // that match the speicified glob. The glob is of the usual fnmatch
 // type.
-func (repo *Repository) NewReferenceIteratorGlob(glob string) (*ReferenceIterator, error) {
+func (repo *Repository) NewReferenceIteratorGlob(glob string) (ReferenceIterator, error) {
 	cstr := C.CString(glob)
 	defer C.free(unsafe.Pointer(cstr))
 	var ptr *C.git_reference_iterator
@@ -222,14 +228,14 @@ func (repo *Repository) NewReferenceIteratorGlob(glob string) (*ReferenceIterato
 		return nil, MakeGitError(ret)
 	}
 
-	iter := &ReferenceIterator{ptr: ptr}
+	iter := &gitReferenceIterator{ptr: ptr}
-	runtime.SetFinalizer(iter, (*ReferenceIterator).Free)
+	runtime.SetFinalizer(iter, (*gitReferenceIterator).Free)
 	return iter, nil
 }
 
 // NextName retrieves the next reference name. If the iteration is over,
 // the returned error is git.ErrIterOver
-func (v *ReferenceIterator) NextName() (string, error) {
+func (v *gitReferenceIterator) NextName() (string, error) {
 	var ptr *C.char
 
 	runtime.LockOSThread()
@@ -249,7 +255,7 @@ func (v *ReferenceIterator) NextName() (string, error) {
 // Create a channel from the iterator. You can use range on the
 // returned channel to iterate over all the references names. The channel
 // will be closed in case any error is found.
-func (v *ReferenceIterator) NameIter() <-chan string {
+func ReferenceNameIteratorChannel(v ReferenceIterator) <-chan string {
 	ch := make(chan string)
 	go func() {
 		defer close(ch)
@@ -265,7 +271,7 @@ func (v *ReferenceIterator) NameIter() <-chan string {
 
 // Next retrieves the next reference. If the iterationis over, the
 // returned error is git.ErrIterOver
-func (v *ReferenceIterator) Next() (*Reference, error) {
+func (v *gitReferenceIterator) Next() (*Reference, error) {
 	var ptr *C.git_reference
 	ret := C.git_reference_next(&ptr, v.ptr)
 	if ret == ITEROVER {
@@ -278,10 +284,16 @@ func (v *ReferenceIterator) Next() (*Reference, error) {
 	return newReferenceFromC(ptr), nil
 }
 
+// Free the reference iterator
+func (v *gitReferenceIterator) Free() {
+	runtime.SetFinalizer(v, nil)
+	C.git_reference_iterator_free(v.ptr)
+}
+
 // Create a channel from the iterator. You can use range on the
 // returned channel to iterate over all the references names. The channel
 // will be closed in case any error is found.
-func (v *ReferenceIterator) Iter() <-chan *Reference {
+func ReferenceIteratorChannel(v ReferenceIterator) <-chan *Reference {
 	ch := make(chan *Reference)
 	go func() {
 		defer close(ch)
@@ -294,9 +306,3 @@ func (v *ReferenceIterator) Iter() <-chan *Reference {
 
 	return ch
 }
-
-// Free the reference iterator
-func (v *ReferenceIterator) Free() {
-	runtime.SetFinalizer(v, nil)
-	C.git_reference_iterator_free(v.ptr)
-}

reference_test.go

@@ -60,7 +60,7 @@ func TestRefModification(t *testing.T) {
 
 }
 
-func TestIterator(t *testing.T) {
+func TestReferenceIterator(t *testing.T) {
 	repo := createTestRepo(t)
 	defer os.RemoveAll(repo.Workdir())
 
@@ -138,7 +138,7 @@ func TestIterator(t *testing.T) {
 	// test the channel iteration
 	list = []string{}
 	iter, err = repo.NewReferenceIterator()
-	for name := range iter.NameIter() {
+	for name := range ReferenceNameIteratorChannel(iter) {
 		list = append(list, name)
 	}
 
@@ -152,7 +152,7 @@ func TestIterator(t *testing.T) {
 	}
 
 	list = []string{}
-	for name := range iter.NameIter() {
+	for name := range ReferenceNameIteratorChannel(iter) {
 		list = append(list, name)
 	}