From 5590078e6ff04be425b4a833adb44a0845c0b52f Mon Sep 17 00:00:00 2001 From: Jesse Ezell Date: Thu, 20 Mar 2014 01:27:03 -0700 Subject: [PATCH] remove channel based iteration for branch / ref. Add ReferenceNameIterator. All iterators use Next(). Remove interfaces. --- branch.go | 60 ++++++-------------------------- branch_test.go | 28 +++------------ reference.go | 89 +++++++++++++++++++---------------------------- reference_test.go | 31 +++-------------- 4 files changed, 55 insertions(+), 153 deletions(-) diff --git a/branch.go b/branch.go index 76f7bf0..bb01f91 100644 --- a/branch.go +++ b/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 { - i := &branchIterator{repo: repo, ptr: ptr} - runtime.SetFinalizer(i, (*branchIterator).Free) +func newBranchIteratorFromC(repo *Repository, ptr *C.git_branch_iterator) *BranchIterator { + i := &BranchIterator{repo: repo, ptr: ptr} + runtime.SetFinalizer(i, (*BranchIterator).Free) return i } -func (i *branchIterator) NextName() (string, error) { - b, err := i.NextBranch() - if err != nil { - return "", err - } - return b.Branch.Name() -} - -func (i *branchIterator) NextBranch() (BranchInfo, error) { +func (i *BranchIterator) Next() (*Branch, BranchType, 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) { - b, err := i.NextBranch() - if err != nil { - return nil, err - } - return b.Branch.Reference, err -} - -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) (*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 -} diff --git a/branch_test.go b/branch_test.go index 6c4a037..2b168f5 100644 --- a/branch_test.go +++ b/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") - } - if b.Type != BranchLocal { + } else if bt != 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") - } - } diff --git a/reference.go b/reference.go index a04d97a..58d48c0 100644 --- a/reference.go +++ b/reference.go @@ -184,23 +184,17 @@ func (v *Reference) Free() { C.git_reference_free(v.ptr) } -type NameIterator interface { - NextName() (string, error) - Free() -} - -type ReferenceIterator interface { - NameIterator - NextReference() (*Reference, error) -} - -type gitReferenceIterator struct { +type ReferenceIterator 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} - runtime.SetFinalizer(iter, (*gitReferenceIterator).Free) + iter := &ReferenceIterator{ptr: ptr, repo: repo} + 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} - runtime.SetFinalizer(iter, (*gitReferenceIterator).Free) + iter := &ReferenceIterator{ptr: ptr} + 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 -} diff --git a/reference_test.go b/reference_test.go index 911263a..bc970bf 100644 --- a/reference_test.go +++ b/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) {