branch: clarify documentation around branches

2 files changed, 99 insertions, 84 deletions

diff --git a/include/git2/branch.h b/include/git2/branch.h
index 8a4ce29a9..de1d1621b 100644
--- a/include/git2/branch.h
+++ b/include/git2/branch.h
@@ -75,9 +75,9 @@ GIT_EXTERN(int) git_branch_create_from_annotated(
 /**
  * Delete an existing branch reference.
  *
- * If the branch is successfully deleted, the passed reference
- * object will be invalidated. The reference must be freed manually
- * by the user.
+ * Note that if the deletion succeeds, the reference object will not
+ * be valid anymore, and should be freed immediately by the user using
+ * `git_reference_free()`.
  *
  * @param branch A valid reference representing a branch
  * @return 0 on success, or an error code.
@@ -145,17 +145,14 @@ GIT_EXTERN(int) git_branch_move(
  * Lookup a branch by its name in a repository.
  *
  * The generated reference must be freed by the user.
- *
  * The branch name will be checked for validity.
- * See `git_tag_create()` for rules about valid names.
  *
- * @param out pointer to the looked-up branch reference
+ * @see git_tag_create for rules about valid names.
  *
+ * @param out pointer to the looked-up branch reference
  * @param repo the repository to look up the branch
- *
  * @param branch_name Name of the branch to be looked-up;
  * this name is validated for consistency.
- *
  * @param branch_type Type of the considered branch. This should
  * be valued with either GIT_BRANCH_LOCAL or GIT_BRANCH_REMOTE.
  *
@@ -169,65 +166,74 @@ GIT_EXTERN(int) git_branch_lookup(
 	git_branch_t branch_type);
 
 /**
- * Return the name of the given local or remote branch.
+ * Get the branch name
+ *
+ * Given a reference object, this will check that it really is a branch (ie.
+ * it lives under "refs/heads/" or "refs/remotes/"), and return the branch part
+ * of it.
  *
- * The name of the branch matches the definition of the name
- * for git_branch_lookup. That is, if the returned name is given
- * to git_branch_lookup() then the reference is returned that
- * was given to this function.
+ * @param out Pointer to the abbreviated reference name.
+ *        Owned by ref, do not free.
  *
- * @param out where the pointer of branch name is stored;
- * this is valid as long as the ref is not freed.
- * @param ref the reference ideally pointing to a branch
+ * @param ref A reference object, ideally pointing to a branch
  *
- * @return 0 on success; otherwise an error code (e.g., if the
- *  ref is no local or remote branch).
+ * @return 0 on success; GIT_EINVALID if the reference isn't either a local or
+ *         remote branch, otherwise an error code.
  */
 GIT_EXTERN(int) git_branch_name(
 		const char **out,
 		const git_reference *ref);
 
 /**
- * Return the reference supporting the remote tracking branch,
- * given a local branch reference.
+ * Get the upstream of a branch
+ *
+ * Given a reference, this will return a new reference object corresponding
+ * to its remote tracking branch. The reference must be a local branch.
  *
- * @param out Pointer where to store the retrieved
- * reference.
+ * @see git_branch_upstream_name for details on the resolution.
  *
+ * @param out Pointer where to store the retrieved reference.
  * @param branch Current underlying reference of the branch.
  *
  * @return 0 on success; GIT_ENOTFOUND when no remote tracking
- * reference exists, otherwise an error code.
+ *         reference exists, otherwise an error code.
  */
 GIT_EXTERN(int) git_branch_upstream(
 	git_reference **out,
 	const git_reference *branch);
 
 /**
- * Set the upstream configuration for a given local branch
+ * Set a branch's upstream branch
  *
- * @param branch the branch to configure
+ * This will update the configuration to set the branch named `name` as the upstream of `branch`.
+ * Pass a NULL name to unset the upstream information.
  *
- * @param upstream_name remote-tracking or local branch to set as
- * upstream. Pass NULL to unset.
+ * @note the actual tracking reference must have been already created for the
+ * operation to succeed.
  *
- * @return 0 or an error code
+ * @param branch the branch to configure
+ * @param branch_name remote-tracking or local branch to set as upstream.
+ *
+ * @return 0 on success; GIT_ENOTFOUND if there's no branch named `branch_name`
+ *         or an error code
  */
-GIT_EXTERN(int) git_branch_set_upstream(git_reference *branch, const char *upstream_name);
+GIT_EXTERN(int) git_branch_set_upstream(
+	git_reference *branch,
+	const char *branch_name);
 
 /**
- * Return the name of the reference supporting the remote tracking branch,
- * given the name of a local branch reference.
+ * Get the upstream name of a branch
  *
- * @param out Pointer to the user-allocated git_buf which will be
- * filled with the name of the reference.
- *
- * @param repo the repository where the branches live
+ * Given a local branch, this will return its remote-tracking branch information,
+ * as a full reference name, ie. "feature/nice" would become
+ * "refs/remote/origin/feature/nice", depending on that branch's configuration.
  *
+ * @param out the buffer into which the name will be written.
+ * @param repo the repository where the branches live.
  * @param refname reference name of the local branch.
  *
- * @return 0, GIT_ENOTFOUND when no remote tracking reference exists,
- *     otherwise an error code.
+ * @return 0 on success, GIT_ENOTFOUND when no remote tracking reference exists,
+ *         or an error code.
  */
 GIT_EXTERN(int) git_branch_upstream_name(
 	git_buf *out,
@@ -235,50 +241,55 @@ GIT_EXTERN(int) git_branch_upstream_name(
 	const char *refname);
 
 /**
- * Determine if the current local branch is pointed at by HEAD.
+ * Determine if HEAD points to the given branch
  *
- * @param branch Current underlying reference of the branch.
+ * @param branch A reference to a local branch.
  *
- * @return 1 if HEAD points at the branch, 0 if it isn't,
- * error code otherwise.
+ * @return 1 if HEAD points at the branch, 0 if it isn't, or a negative value
+ * 		   as an error code.
  */
 GIT_EXTERN(int) git_branch_is_head(
 	const git_reference *branch);
 
 /**
- * Determine if the current branch is checked out in any linked
- * repository.
+ * Determine if any HEAD points to the current branch
  *
- * @param branch Reference to the branch.
+ * This will iterate over all known linked repositories (usually in the form of
+ * worktrees) and report whether any HEAD is pointing at the current branch.
  *
- * @return 1 if branch is checked out, 0 if it isn't,
- * error code otherwise.
+ * @param branch A reference to a local branch.
+ *
+ * @return 1 if branch is checked out, 0 if it isn't, an error code otherwise.
  */
 GIT_EXTERN(int) git_branch_is_checked_out(
 	const git_reference *branch);
 
 /**
- * Return the name of remote that the remote tracking branch belongs to.
+ * Find the remote name of a remote-tracking branch
  *
- * @param out Pointer to the user-allocated git_buf which will be filled with the name of the remote.
+ * This will return the name of the remote whose fetch refspec is matching
+ * the given branch. E.g. given a branch "refs/remotes/test/master", it will
+ * extract the "test" part. If refspecs from multiple remotes match,
+ * the function will return GIT_EAMBIGUOUS.
  *
+ * @param out The buffer into which the name will be written.
  * @param repo The repository where the branch lives.
+ * @param refname complete name of the remote tracking branch.
  *
- * @param canonical_branch_name name of the remote tracking branch.
- *
- * @return 0, GIT_ENOTFOUND
- *     when no remote matching remote was found,
- *     GIT_EAMBIGUOUS when the branch maps to several remotes,
- *     otherwise an error code.
+ * @return 0 on success, GIT_ENOTFOUND when no matching remote was found,
+ *         GIT_EAMBIGUOUS when the branch maps to several remotes,
+ *         otherwise an error code.
  */
 GIT_EXTERN(int) git_branch_remote_name(
 	git_buf *out,
 	git_repository *repo,
-	const char *canonical_branch_name);
-
+	const char *refname);
 
 /**
- * Retrieve the name of the upstream remote of a local branch
+ * Retrieve the upstream remote of a local branch
+ *
+ * This will return the currently configured "branch.*.remote" for a given
+ * branch. This branch must be local.
  *
  * @param buf the buffer into which to write the name
  * @param repo the repository in which to look
diff --git a/src/branch.c b/src/branch.c
index a93db7e5b..8926c8956 100644
--- a/src/branch.c
+++ b/src/branch.c
@@ -597,35 +597,36 @@ on_error:
 	return -1;
 }
 
-int git_branch_set_upstream(git_reference *branch, const char *upstream_name)
+int git_branch_set_upstream(git_reference *branch, const char *branch_name)
 {
-	git_buf key = GIT_BUF_INIT, value = GIT_BUF_INIT;
+	git_buf key = GIT_BUF_INIT, remote_name = GIT_BUF_INIT, merge_refspec = GIT_BUF_INIT;
 	git_reference *upstream;
 	git_repository *repo;
 	git_remote *remote = NULL;
 	git_config *config;
-	const char *name, *shortname;
+	const char *refname, *shortname;
 	int local, error;
 	const git_refspec *fetchspec;
 
-	name = git_reference_name(branch);
-	if (!git_reference__is_branch(name))
-		return not_a_local_branch(name);
+	refname = git_reference_name(branch);
+	if (!git_reference__is_branch(refname))
+		return not_a_local_branch(refname);
 
 	if (git_repository_config__weakptr(&config, git_reference_owner(branch)) < 0)
 		return -1;
 
-	shortname = name + strlen(GIT_REFS_HEADS_DIR);
+	shortname = refname + strlen(GIT_REFS_HEADS_DIR);
 
-	if (upstream_name == NULL)
+	/* We're unsetting, delegate and bail-out */
+	if (branch_name == NULL)
 		return unset_upstream(config, shortname);
 
 	repo = git_reference_owner(branch);
 
-	/* First we need to figure out whether it's a branch or remote-tracking */
-	if (git_branch_lookup(&upstream, repo, upstream_name, GIT_BRANCH_LOCAL) == 0)
+	/* First we need to resolve name to a branch */
+	if (git_branch_lookup(&upstream, repo, branch_name, GIT_BRANCH_LOCAL) == 0)
 		local = 1;
-	else if (git_branch_lookup(&upstream, repo, upstream_name, GIT_BRANCH_REMOTE) == 0)
+	else if (git_branch_lookup(&upstream, repo, branch_name, GIT_BRANCH_REMOTE) == 0)
 		local = 0;
 	else {
 		git_error_set(GIT_ERROR_REFERENCE,
@@ -634,60 +635,63 @@ int git_branch_set_upstream(git_reference *branch, const char *upstream_name)
 	}
 
 	/*
-	 * If it's local, the remote is "." and the branch name is
-	 * simply the refname. Otherwise we need to figure out what
-	 * the remote-tracking branch's name on the remote is and use
-	 * that.
+	 * If it's a local-tracking branch, its remote is "." (as "the local
+	 * repository"), and the branch name is simply the refname.
+	 * Otherwise we need to figure out what the remote-tracking branch's
+	 * name on the remote is and use that.
 	 */
 	if (local)
-		error = git_buf_puts(&value, ".");
+		error = git_buf_puts(&remote_name, ".");
 	else
-		error = git_branch_remote_name(&value, repo, git_reference_name(upstream));
+		error = git_branch_remote_name(&remote_name, repo, git_reference_name(upstream));
 
 	if (error < 0)
 		goto on_error;
 
+	/* Update the upsteam branch config with the new name */
 	if (git_buf_printf(&key, "branch.%s.remote", shortname) < 0)
 		goto on_error;
 
-	if (git_config_set_string(config, git_buf_cstr(&key), git_buf_cstr(&value)) < 0)
+	if (git_config_set_string(config, git_buf_cstr(&key), git_buf_cstr(&remote_name)) < 0)
 		goto on_error;
 
 	if (local) {
-		git_buf_clear(&value);
-		if (git_buf_puts(&value, git_reference_name(upstream)) < 0)
+		/* A local branch uses the upstream refname directly */
+		if (git_buf_puts(&merge_refspec, git_reference_name(upstream)) < 0)
 			goto on_error;
 	} else {
-		/* Get the remoe-tracking branch's refname in its repo */
-		if (git_remote_lookup(&remote, repo, git_buf_cstr(&value)) < 0)
+		/* We transform the upstream branch name according to the remote's refspecs */
+		if (git_remote_lookup(&remote, repo, git_buf_cstr(&remote_name)) < 0)
 			goto on_error;
 
 		fetchspec = git_remote__matching_dst_refspec(remote, git_reference_name(upstream));
-		git_buf_clear(&value);
-		if (!fetchspec || git_refspec_rtransform(&value, fetchspec, git_reference_name(upstream)) < 0)
+		if (!fetchspec || git_refspec_rtransform(&merge_refspec, fetchspec, git_reference_name(upstream)) < 0)
 			goto on_error;
 
 		git_remote_free(remote);
 		remote = NULL;
 	}
 
+	/* Update the merge branch config with the refspec */
 	git_buf_clear(&key);
 	if (git_buf_printf(&key, "branch.%s.merge", shortname) < 0)
 		goto on_error;
 
-	if (git_config_set_string(config, git_buf_cstr(&key), git_buf_cstr(&value)) < 0)
+	if (git_config_set_string(config, git_buf_cstr(&key), git_buf_cstr(&merge_refspec)) < 0)
 		goto on_error;
 
 	git_reference_free(upstream);
 	git_buf_dispose(&key);
-	git_buf_dispose(&value);
+	git_buf_dispose(&remote_name);
+	git_buf_dispose(&merge_refspec);
 
 	return 0;
 
 on_error:
 	git_reference_free(upstream);
 	git_buf_dispose(&key);
-	git_buf_dispose(&value);
+	git_buf_dispose(&remote_name);
+	git_buf_dispose(&merge_refspec);
 	git_remote_free(remote);
 
 	return -1;