Merge pull request #782 from nulltoken/topic/branch-foreach

Branch foreach

include/git2/branch.h

@@ -72,15 +72,7 @@ GIT_EXTERN(int) git_branch_delete(
 		git_branch_t branch_type);
 
 /**
- * Fill a list with all the branches in the Repository
- *
- * The string array will be filled with the names of the
- * matching branches; these values are owned by the user and
- * should be free'd manually when no longer needed, using
- * `git_strarray_free`.
- *
- * @param branch_names Pointer to a git_strarray structure
- * where the branch names will be stored.
+ * Loop over all the branches and issue a callback for each one.
  *
  * @param repo Repository where to find the branches.
  *
@@ -88,12 +80,21 @@ GIT_EXTERN(int) git_branch_delete(
  * listing. Valid values are GIT_BRANCH_LOCAL, GIT_BRANCH_REMOTE
  * or a combination of the two.
  *
+ * @param branch_cb Callback to invoke per found branch.
+ *
+ * @param payload Extra parameter to callback function.
+ *
  * @return 0 or an error code.
  */
-GIT_EXTERN(int) git_branch_list(
-		git_strarray *branch_names,
+GIT_EXTERN(int) git_branch_foreach(
 		git_repository *repo,
-		unsigned int list_flags);
+		unsigned int list_flags,
+		int (*branch_cb)(
+			const char *branch_name,
+			git_branch_t branch_type,
+			void *payload),
+		void *payload
+);
 
 /**
  * Move/rename an existing branch reference.

src/branch.c

@@ -141,46 +141,46 @@ on_error:
 }
 
 typedef struct {
-	git_vector *branchlist;
+	int (*branch_cb)(
+			const char *branch_name,
+			git_branch_t branch_type,
+			void *payload);
+	void *callback_payload;
 	unsigned int branch_type;
-} branch_filter_data;
+} branch_foreach_filter;
 
-static int branch_list_cb(const char *branch_name, void *payload)
+static int branch_foreach_cb(const char *branch_name, void *payload)
 {
-	branch_filter_data *filter = (branch_filter_data *)payload;
+	branch_foreach_filter *filter = (branch_foreach_filter *)payload;
 
-	if (filter->branch_type & GIT_BRANCH_LOCAL && git__prefixcmp(branch_name, GIT_REFS_HEADS_DIR) == 0) {
-		return git_vector_insert(filter->branchlist, git__strdup(branch_name +strlen(GIT_REFS_HEADS_DIR)));
-  } else if (filter->branch_type & GIT_BRANCH_REMOTE && git__prefixcmp(branch_name, GIT_REFS_REMOTES_DIR) == 0) {
-		return git_vector_insert(filter->branchlist, git__strdup(branch_name+strlen(GIT_REFS_DIR)));
-  }
+	if (filter->branch_type & GIT_BRANCH_LOCAL &&
+		git__prefixcmp(branch_name, GIT_REFS_HEADS_DIR) == 0)
+		return filter->branch_cb(branch_name + strlen(GIT_REFS_HEADS_DIR), GIT_BRANCH_LOCAL, filter->callback_payload);
+
+	if (filter->branch_type & GIT_BRANCH_REMOTE &&
+		git__prefixcmp(branch_name, GIT_REFS_REMOTES_DIR) == 0)
+		return filter->branch_cb(branch_name + strlen(GIT_REFS_REMOTES_DIR), GIT_BRANCH_REMOTE, filter->callback_payload);
 
 	return 0;
 }
 
-int git_branch_list(git_strarray *branch_names, git_repository *repo, unsigned int list_flags)
+int git_branch_foreach(
+		git_repository *repo,
+		unsigned int list_flags,
+		int (*branch_cb)(
+			const char *branch_name,
+			git_branch_t branch_type,
+			void *payload),
+		void *payload
+)
 {
-	int error;
-	branch_filter_data filter;
-	git_vector branchlist;
-
-	assert(branch_names && repo);
+	branch_foreach_filter filter;
 
-	if (git_vector_init(&branchlist, 8, NULL) < 0)
-		return -1;
-
-	filter.branchlist = &branchlist;
+	filter.branch_cb = branch_cb;
 	filter.branch_type = list_flags;
+	filter.callback_payload = payload;
 
-	error = git_reference_foreach(repo, GIT_REF_LISTALL, &branch_list_cb, (void *)&filter);
-	if (error < 0) {
-		git_vector_free(&branchlist);
-		return -1;
-	}
-
-	branch_names->strings = (char **)branchlist.contents;
-	branch_names->count = branchlist.length;
-	return 0;
+	return git_reference_foreach(repo, GIT_REF_LISTALL, &branch_foreach_cb, (void *)&filter);
 }
 
 int git_branch_move(git_repository *repo, const char *old_branch_name, const char *new_branch_name, int force)

tests-clar/refs/branches/listall.c → tests-clar/refs/branches/foreach.c

@@ -3,10 +3,9 @@
 #include "branch.h"
 
 static git_repository *repo;
-static git_strarray branch_list;
 static git_reference *fake_remote;
 
-void test_refs_branches_listall__initialize(void)
+void test_refs_branches_foreach__initialize(void)
 {
 	git_oid id;
 
@@ -17,62 +16,106 @@ void test_refs_branches_listall__initialize(void)
 	cl_git_pass(git_reference_create_oid(&fake_remote, repo, "refs/remotes/nulltoken/master", &id, 0));
 }
 
-void test_refs_branches_listall__cleanup(void)
+void test_refs_branches_foreach__cleanup(void)
 {
-	git_strarray_free(&branch_list);
 	git_reference_free(fake_remote);
 	git_repository_free(repo);
 
 	cl_fixture_cleanup("testrepo.git");
 }
 
+static int count_branch_list_cb(const char *branch_name, git_branch_t branch_type, void *payload)
+{
+	int *count = (int *)payload;
+
+	(*count)++;
+
+	return 0;
+}
+
 static void assert_retrieval(unsigned int flags, unsigned int expected_count)
 {
-	cl_git_pass(git_branch_list(&branch_list, repo, flags));
+	int count = 0;
 
-	cl_assert_equal_i(branch_list.count, expected_count);
+	cl_git_pass(git_branch_foreach(repo, flags, count_branch_list_cb, &count));
+
+	cl_assert_equal_i(expected_count, count);
 }
 
-void test_refs_branches_listall__retrieve_all_branches(void)
+void test_refs_branches_foreach__retrieve_all_branches(void)
 {
 	assert_retrieval(GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, 9);
 }
 
-void test_refs_branches_listall__retrieve_remote_branches(void)
+void test_refs_branches_foreach__retrieve_remote_branches(void)
 {
 	assert_retrieval(GIT_BRANCH_REMOTE, 2);
 }
 
-void test_refs_branches_listall__retrieve_local_branches(void)
+void test_refs_branches_foreach__retrieve_local_branches(void)
 {
 	assert_retrieval(GIT_BRANCH_LOCAL, 7);
 }
 
-static void assert_branch_list_contains(git_strarray *branches, const char* expected_branch_name)
+struct expectations {
+	const char *branch_name;
+	int encounters;
+};
+
+static void assert_branch_has_been_found(struct expectations *findings, const char* expected_branch_name)
 {
-	unsigned int i;
+	int pos = 0;
 
-	for (i = 0; i < branches->count; i++) {
-		if (strcmp(expected_branch_name, branches->strings[i]) == 0)
+	while (findings[pos].branch_name)
+	{
+		if (strcmp(expected_branch_name, findings[pos].branch_name) == 0) {
+			cl_assert_equal_i(1, findings[pos].encounters);
 			return;
+		}
+
+		pos++;
 	}
 
 	cl_fail("expected branch not found in list.");
 }
 
+static int contains_branch_list_cb(const char *branch_name, git_branch_t branch_type, void *payload)
+{
+	int pos = 0;
+
+	struct expectations *exp = (struct expectations *)payload;
+
+	while (exp[pos].branch_name)
+	{
+		if (strcmp(branch_name, exp[pos].branch_name) == 0)
+			exp[pos].encounters++;
+		
+		pos++;
+	}
+
+	return 0;
+}
+
 /*
  * $ git branch -r
  *  nulltoken/HEAD -> nulltoken/master
  *  nulltoken/master
  */
-void test_refs_branches_listall__retrieve_remote_symbolic_HEAD_when_present(void)
+void test_refs_branches_foreach__retrieve_remote_symbolic_HEAD_when_present(void)
 {
+	struct expectations exp[] = {
+		{ "nulltoken/HEAD", 0 },
+		{ "nulltoken/master", 0 },
+		{ NULL, 0 }
+	};
+
 	git_reference_free(fake_remote);
 	cl_git_pass(git_reference_create_symbolic(&fake_remote, repo, "refs/remotes/nulltoken/HEAD", "refs/remotes/nulltoken/master", 0));
 
-	cl_git_pass(git_branch_list(&branch_list, repo, GIT_BRANCH_REMOTE));
-	
-	cl_assert_equal_i(3, branch_list.count);
-	assert_branch_list_contains(&branch_list, "remotes/nulltoken/HEAD");
-	assert_branch_list_contains(&branch_list, "remotes/nulltoken/master");
+	assert_retrieval(GIT_BRANCH_REMOTE, 3);
+
+	cl_git_pass(git_branch_foreach(repo, GIT_BRANCH_REMOTE, contains_branch_list_cb, &exp));
+
+	assert_branch_has_been_found(exp, "nulltoken/HEAD");
+	assert_branch_has_been_found(exp, "nulltoken/HEAD");
 }