CommitIterator now not hold branch as reference.

Make private var instead to prevent unexpected state change
from external influence. Uses enum instead of tuple
of stop and error to indicate state of Git command result.

SwiftGit2/CommitIterator.swift

@@ -7,61 +7,68 @@ import Result
 import libgit2
 
 public class CommitIterator: IteratorProtocol {
-    public typealias Element = Result<Commit, NSError>
+	public typealias Element = Result<Commit, NSError>
-    var repo: Repository
+	let repo: Repository
-    var branch: Branch
+	private var oid: git_oid
-    var revisionWalker: OpaquePointer? = nil
+	private var revisionWalker: OpaquePointer? = nil
-    var oid: git_oid
-    private var unsafeCommit: OpaquePointer? = nil
 
-    public init(repo: Repository, branch: Branch) {
+	private enum Next {
-        self.repo = repo
+		case over
-        self.branch = branch
+		case ok
-        self.oid = branch.oid.oid
+		case error(NSError)
-        setupRevisionWalker()
-    }
 
-    deinit {
+		init(_ result: Int32, name: String) {
-        git_revwalk_free(self.revisionWalker)
+			switch result {
-    }
+			case GIT_ITEROVER.rawValue:
+				self = .over
+			case GIT_OK.rawValue:
+				self = .ok
+			default:
+				self = .error(NSError(gitError: result, pointOfFailure: name))
+			}
+		}
+	}
 
-    private func setupRevisionWalker() {
+	init(repo: Repository, branch: Branch) {
-        git_revwalk_new(&revisionWalker, repo.pointer)
+		self.repo = repo
-        git_revwalk_sorting(revisionWalker, GIT_SORT_TOPOLOGICAL.rawValue)
+		self.oid = branch.oid.oid
-        git_revwalk_sorting(revisionWalker, GIT_SORT_TIME.rawValue)
+		setupRevisionWalker()
-        git_revwalk_push(revisionWalker, &oid)
+	}
-    }
 
-    private func result(withName name: String, from result: Int32) -> (stop: Bool, error: NSError?) {
+	deinit {
-        guard result == GIT_OK.rawValue else {
+		git_revwalk_free(self.revisionWalker)
-            if result == GIT_ITEROVER.rawValue {
+	}
-                return (stop: true, error: nil)
-            } else {
-                return (stop: false, error: NSError(gitError: result, pointOfFailure: name))
-            }
-        }
-        return (stop: false, error: nil)
-    }
 
-    public func next() -> Element? {
+	private func setupRevisionWalker() {
-        let revwalkGitResult = git_revwalk_next(&self.oid, self.revisionWalker)
+		git_revwalk_new(&revisionWalker, repo.pointer)
-        let revwalkResult = result(withName: "git_revwalk_next", from: revwalkGitResult)
+		git_revwalk_sorting(revisionWalker, GIT_SORT_TOPOLOGICAL.rawValue)
-        if revwalkResult.stop {
+		git_revwalk_sorting(revisionWalker, GIT_SORT_TIME.rawValue)
-            return nil
+		git_revwalk_push(revisionWalker, &oid)
-        } else if let error = revwalkResult.error {
+	}
-            return Result.failure(error)
+
-        }
+	private func next(withName name: String, from result: Int32) -> Next {
-        let lookupGitResult = git_commit_lookup(&self.unsafeCommit, self.repo.pointer, &self.oid)
+		if result == GIT_OK.rawValue || result == GIT_ITEROVER.rawValue {
-        let lookupResult = result(withName: "git_commit_lookup", from: lookupGitResult)
+			return Next(result, name: name)
-        if lookupResult.stop {
+		} else {
-            return nil
+			return Next.error(NSError(gitError: result, pointOfFailure: name))
-        } else if let error = lookupResult.error {
+		}
-            return Result.failure(error)
+	}
-        }
+
-        guard let commit = unsafeCommit else {
+	public func next() -> Element? {
-            return nil
+		var unsafeCommit: OpaquePointer? = nil
-        }
+		let revwalkGitResult = git_revwalk_next(&oid, revisionWalker)
-        git_commit_free(unsafeCommit)
+		let nextResult = next(withName: "git_revwalk_next", from: revwalkGitResult)
-        return Result.success(Commit(commit))
+		if case let .error(error) = nextResult {
-    }
+			return Result.failure(error)
+		} else if case .over = nextResult {
+			return nil
+		}
+		guard git_commit_lookup(&unsafeCommit, repo.pointer, &oid) == GIT_OK.rawValue,
+		      let unwrapCommit = unsafeCommit else {
+			return nil
+		}
+		let result: Element = Result.success(Commit(unwrapCommit))
+		git_commit_free(unsafeCommit)
+		return result
+	}
 }

SwiftGit2Tests/RepositorySpec.swift

@@ -92,18 +92,18 @@ class RepositorySpec: QuickSpec {
 			let env = ProcessInfo.processInfo.environment
 
 			if let privateRepo = env["SG2TestPrivateRepo"],
-				let gitUsername = env["SG2TestUsername"],
+			   let gitUsername = env["SG2TestUsername"],
-				let publicKey = env["SG2TestPublicKey"],
+			   let publicKey = env["SG2TestPublicKey"],
-				let privateKey = env["SG2TestPrivateKey"],
+			   let privateKey = env["SG2TestPrivateKey"],
-				let passphrase = env["SG2TestPassphrase"] {
+			   let passphrase = env["SG2TestPassphrase"] {
 
 				it("should be able to clone a remote repository requiring credentials") {
 					let remoteRepoURL = URL(string: privateRepo)
 					let localURL = self.temporaryURL(forPurpose: "private-remote-clone")
 					let credentials = Credentials.sshMemory(username: gitUsername,
-					                                        publicKey: publicKey,
+						publicKey: publicKey,
-					                                        privateKey: privateKey,
+						privateKey: privateKey,
-					                                        passphrase: passphrase)
+						passphrase: passphrase)
 
 					let cloneResult = Repository.clone(from: remoteRepoURL!, to: localURL, credentials: credentials)
 
@@ -601,18 +601,30 @@ class RepositorySpec: QuickSpec {
 			}
 		}
 
-		fdescribe("Repository.allCommits(in:)") {
+		describe("Repository.allCommits(in:)") {
 			it("should return all (9) commits") {
 				let repo = Fixtures.simpleRepository
 				let branches = repo.localBranches().value!
-                var count = 0
+				let expectedCount = 9
-                let expected = 9
+				let expectedMessages: [String] = [
+					"List branches in README\n",
+					"Create a README\n",
+					"Merge branch 'alphabetize'\n",
+					"Alphabetize branches\n",
+					"List new branches\n",
+					"List branches in README\n",
+					"Create a README\n",
+					"List branches in README\n",
+					"Create a README\n"
+				]
+				var commitMessages: [String] = []
 				for branch in branches {
-                    while repo.commits(in: branch).next() != nil {
+					while let commit = repo.commits(in: branch).next() {
-                        count += 1
+						commitMessages.append(commit.value!.message)
-                    }
+					}
 				}
-				expect(count).to(equal(expected))
+				expect(commitMessages.count).to(equal(expectedCount))
+				expect(commitMessages).to(equal(expectedMessages))
 			}
 		}
 	}