remove channel based iteration for branch / ref. Add ReferenceNameIterator. All iterators use Next(). Remove interfaces.

branch.go

@@ -27,36 +27,23 @@ func (r *Reference) Branch() *Branch {
 	return &Branch{Reference: r}
 }
 
-type branchIterator struct {
+type BranchIterator struct {
 	ptr  *C.git_branch_iterator
 	repo *Repository
 }
 
-type BranchIterator interface {
-	ReferenceIterator
-	NextBranch() (BranchInfo, error)
-}
-
 type BranchInfo struct {
 	Branch *Branch
 	Type   BranchType
 }
 
-func newBranchIteratorFromC(repo *Repository, ptr *C.git_branch_iterator) BranchIterator {
+func newBranchIteratorFromC(repo *Repository, ptr *C.git_branch_iterator) *BranchIterator {
-	i := &branchIterator{repo: repo, ptr: ptr}
+	i := &BranchIterator{repo: repo, ptr: ptr}
-	runtime.SetFinalizer(i, (*branchIterator).Free)
+	runtime.SetFinalizer(i, (*BranchIterator).Free)
 	return i
 }
 
-func (i *branchIterator) NextName() (string, error) {
+func (i *BranchIterator) Next() (*Branch, BranchType, error) {
-	b, err := i.NextBranch()
-	if err != nil {
-		return "", err
-	}
-	return b.Branch.Name()
-}
-
-func (i *branchIterator) NextBranch() (BranchInfo, error) {
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
@@ -67,30 +54,22 @@ func (i *branchIterator) NextBranch() (BranchInfo, error) {
 	ecode := C.git_branch_next(&refPtr, &refType, i.ptr)
 
 	if ecode == C.GIT_ITEROVER {
-		return BranchInfo{}, ErrIterOver
+		return nil, BranchLocal, ErrIterOver
 	} else if ecode < 0 {
-		return BranchInfo{}, MakeGitError(ecode)
+		return nil, BranchLocal, MakeGitError(ecode)
 	}
 
 	branch := newReferenceFromC(refPtr).Branch()
 
-	return BranchInfo{branch, BranchType(refType)}, nil
+	return branch, BranchType(refType), nil
 }
 
-func (i *branchIterator) NextReference() (*Reference, error) {
+func (i *BranchIterator) Free() {
-	b, err := i.NextBranch()
-	if err != nil {
-		return nil, err
-	}
-	return b.Branch.Reference, err
-}
-
-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) (*BranchIterator, error) {
 
 	refType := C.git_branch_t(flags)
 	var ptr *C.git_branch_iterator
@@ -144,7 +123,7 @@ func (b *Branch) Delete() error {
 	return nil
 }
 
-func (b *Branch) MoveBranch(newBranchName string, force bool, signature *Signature, msg string) (*Branch, error) {
+func (b *Branch) Move(newBranchName string, force bool, signature *Signature, msg string) (*Branch, error) {
 	var ptr *C.git_reference
 	cNewBranchName := C.CString(newBranchName)
 	cForce := cbool(force)
@@ -275,20 +254,3 @@ func (repo *Repository) UpstreamName(canonicalBranchName string) (string, error)
 
 	return C.GoString(nameBuf.ptr), nil
 }
-
-// Create a channel from the iterator. You can use range on the
-// returned channel to iterate over all the branches. The channel
-// will be closed in case any error is found.
-func BranchIteratorChannel(v BranchIterator) <-chan BranchInfo {
-	ch := make(chan BranchInfo)
-	go func() {
-		defer close(ch)
-		b, err := v.NextBranch()
-		for err == nil {
-			ch <- b
-			b, err = v.NextBranch()
-		}
-	}()
-
-	return ch
-}

branch_test.go

@@ -12,35 +12,15 @@ func TestBranchIterator(t *testing.T) {
 	i, err := repo.NewBranchIterator(BranchLocal)
 	checkFatal(t, err)
 
-	b, err := i.NextBranch()
+	b, bt, err := i.Next()
 	checkFatal(t, err)
-	if name, _ := b.Branch.Name(); name != "master" {
+	if name, _ := b.Name(); name != "master" {
 		t.Fatalf("expected master")
-	}
+	} else if bt != BranchLocal {
-	if b.Type != BranchLocal {
 		t.Fatalf("expected BranchLocal, not %v", t)
 	}
-	b, err = i.NextBranch()
+	b, bt, err = i.Next()
 	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 NameIteratorChannel(i) {
-		list = append(list, ref)
-	}
-
-	if len(list) != 1 {
-		t.Fatal("expected single match")
-	}
-
-	if list[0] != "master" {
-		t.Fatal("expected master")
-	}
-
 }

reference.go

@@ -184,23 +184,17 @@ func (v *Reference) Free() {
 	C.git_reference_free(v.ptr)
 }
 
-type NameIterator interface {
+type ReferenceIterator struct {
-	NextName() (string, error)
-	Free()
-}
-
-type ReferenceIterator interface {
-	NameIterator
-	NextReference() (*Reference, error)
-}
-
-type gitReferenceIterator struct {
 	ptr  *C.git_reference_iterator
 	repo *Repository
 }
 
+type ReferenceNameIterator struct {
+	*ReferenceIterator
+}
+
 // 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()
@@ -211,15 +205,32 @@ func (repo *Repository) NewReferenceIterator() (ReferenceIterator, error) {
 		return nil, MakeGitError(ret)
 	}
 
-	iter := &gitReferenceIterator{ptr: ptr, repo: repo}
+	iter := &ReferenceIterator{ptr: ptr, repo: repo}
-	runtime.SetFinalizer(iter, (*gitReferenceIterator).Free)
+	runtime.SetFinalizer(iter, (*ReferenceIterator).Free)
+	return iter, nil
+}
+
+// NewReferenceIterator creates a new bane iterator over reference names
+func (repo *Repository) NewReferenceNameIterator() (*ReferenceIterator, error) {
+	var ptr *C.git_reference_iterator
+
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	ret := C.git_reference_iterator_new(&ptr, repo.ptr)
+	if ret < 0 {
+		return nil, MakeGitError(ret)
+	}
+
+	iter := &ReferenceIterator{ptr: ptr, repo: repo}
+	runtime.SetFinalizer(iter, (*ReferenceIterator).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
@@ -232,14 +243,18 @@ func (repo *Repository) NewReferenceIteratorGlob(glob string) (ReferenceIterator
 		return nil, MakeGitError(ret)
 	}
 
-	iter := &gitReferenceIterator{ptr: ptr}
+	iter := &ReferenceIterator{ptr: ptr}
-	runtime.SetFinalizer(iter, (*gitReferenceIterator).Free)
+	runtime.SetFinalizer(iter, (*ReferenceIterator).Free)
 	return iter, nil
 }
 
+func (i *ReferenceIterator) Names() *ReferenceNameIterator {
+	return &ReferenceNameIterator{i}
+}
+
 // NextName retrieves the next reference name. If the iteration is over,
 // the returned error is git.ErrIterOver
-func (v *gitReferenceIterator) NextName() (string, error) {
+func (v *ReferenceNameIterator) Next() (string, error) {
 	var ptr *C.char
 
 	runtime.LockOSThread()
@@ -256,26 +271,9 @@ func (v *gitReferenceIterator) NextName() (string, error) {
 	return C.GoString(ptr), nil
 }
 
-// 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 NameIteratorChannel(v NameIterator) <-chan string {
-	ch := make(chan string)
-	go func() {
-		defer close(ch)
-		name, err := v.NextName()
-		for err == nil {
-			ch <- name
-			name, err = v.NextName()
-		}
-	}()
-
-	return ch
-}
-
 // Next retrieves the next reference. If the iterationis over, the
 // returned error is git.ErrIterOver
-func (v *gitReferenceIterator) NextReference() (*Reference, error) {
+func (v *ReferenceIterator) Next() (*Reference, error) {
 	var ptr *C.git_reference
 	ret := C.git_reference_next(&ptr, v.ptr)
 	if ret == ITEROVER {
@@ -289,24 +287,7 @@ func (v *gitReferenceIterator) NextReference() (*Reference, error) {
 }
 
 // Free the reference iterator
-func (v *gitReferenceIterator) Free() {
+func (v *ReferenceIterator) 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 ReferenceIteratorChannel(v ReferenceIterator) <-chan *Reference {
-	ch := make(chan *Reference)
-	go func() {
-		defer close(ch)
-		name, err := v.NextReference()
-		for err == nil {
-			ch <- name
-			name, err = v.NextReference()
-		}
-	}()
-
-	return ch
-}

reference_test.go

@@ -106,10 +106,11 @@ func TestReferenceIterator(t *testing.T) {
 	}
 
 	// test some manual iteration
-	name, err := iter.NextName()
+	nameIter := iter.Names()
+	name, err := nameIter.Next()
 	for err == nil {
 		list = append(list, name)
-		name, err = iter.NextName()
+		name, err = nameIter.Next()
 	}
 	if err != ErrIterOver {
 		t.Fatal("Iteration not over")
@@ -122,10 +123,10 @@ func TestReferenceIterator(t *testing.T) {
 	iter, err = repo.NewReferenceIterator()
 	checkFatal(t, err)
 	count := 0
-	_, err = iter.NextReference()
+	_, err = iter.Next()
 	for err == nil {
 		count++
-		_, err = iter.NextReference()
+		_, err = iter.Next()
 	}
 	if err != ErrIterOver {
 		t.Fatal("Iteration not over")
@@ -135,28 +136,6 @@ func TestReferenceIterator(t *testing.T) {
 		t.Fatalf("Wrong number of references returned %v", count)
 	}
 
-	// test the channel iteration
-	list = []string{}
-	iter, err = repo.NewReferenceIterator()
-	for name := range NameIteratorChannel(iter) {
-		list = append(list, name)
-	}
-
-	sort.Strings(list)
-	compareStringList(t, expected, list)
-
-	iter, err = repo.NewReferenceIteratorGlob("refs/heads/t*")
-	expected = []string{
-		"refs/heads/three",
-		"refs/heads/two",
-	}
-
-	list = []string{}
-	for name := range NameIteratorChannel(iter) {
-		list = append(list, name)
-	}
-
-	compareStringList(t, expected, list)
 }
 
 func TestUtil(t *testing.T) {