Re: [PATCH v5 2/5] branch: let delete_branches warn instead of error on bulk refusal

[Junio C Hamano <gitster@pobox.com>]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Add two new parameters to delete_branches() and the helper
> check_branch_commit():
>
> * warn_only switches the per-branch refusal from a hard error
>   ("error: the branch 'X' is not fully merged" plus a four-line
>   hint about 'git branch -D X') to a one-line warning, and
>   causes the function to skip those branches without setting its
>   exit code. Each refused branch is still skipped from deletion.
> * n_not_merged, when non-NULL, is incremented for each branch
>   refused on the not-merged path, so a bulk caller can summarize
>   rather than print per-branch advice.
>
> All existing call sites pass 0 / NULL and so are unaffected. Both
> parameters are wired up so a bulk-deletion caller can suppress
> the noise normally appropriate for a one-shot 'git branch -d'.

Existing call sites are about "branch -d <other>" that allows the
other branch to be deleted if it is part of HEAD or if it is part of
its tracking branch, but should "branch --prune-merged" pay
attention to what branch happens to be checked out the same way (not
a rherotical question to hint that I do not think it should---I do
not have a strong opinion on this either way)?

> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>  builtin/branch.c | 29 ++++++++++++++++++++---------
>  1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index b3289a8875..1941f8a9ad 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -192,7 +192,8 @@ static int branch_merged(int kind, const char *name,
>  
>  static int check_branch_commit(const char *branchname, const char *refname,
>  			       const struct object_id *oid, struct commit *head_rev,
> -			       int kinds, int force)
> +			       int kinds, int force, int warn_only,
> +			       int *n_not_merged)
>  {
>  	struct commit *rev = lookup_commit_reference(the_repository, oid);
>  	if (!force && !rev) {
> @@ -200,10 +201,18 @@ static int check_branch_commit(const char *branchname, const char *refname,
>  		return -1;
>  	}
>  	if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
> -		error(_("the branch '%s' is not fully merged"), branchname);
> -		advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
> -				  _("If you are sure you want to delete it, "
> -				  "run 'git branch -D %s'"), branchname);
> +		if (warn_only) {
> +			warning(_("the branch '%s' is not fully merged"),
> +				branchname);
> +		} else {
> +			error(_("the branch '%s' is not fully merged"),
> +			      branchname);
> +			advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
> +					  _("If you are sure you want to delete it, "
> +					  "run 'git branch -D %s'"), branchname);
> +		}
> +		if (n_not_merged)
> +			(*n_not_merged)++;
>  		return -1;
>  	}
>  	return 0;
> @@ -219,7 +228,7 @@ static void delete_branch_config(const char *branchname)
>  }
>  
>  static int delete_branches(int argc, const char **argv, int force, int kinds,
> -			   int quiet)
> +			   int quiet, int warn_only, int *n_not_merged)
>  {
>  	struct commit *head_rev = NULL;
>  	struct object_id oid;
> @@ -309,8 +318,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
>  
>  		if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
>  		    check_branch_commit(bname.buf, name, &oid, head_rev, kinds,
> -					force)) {
> -			ret = 1;
> +					force, warn_only, n_not_merged)) {
> +			if (!warn_only)
> +				ret = 1;
>  			goto next;
>  		}
>  
> @@ -961,7 +971,8 @@ int cmd_branch(int argc,
>  	if (delete) {
>  		if (!argc)
>  			die(_("branch name required"));
> -		ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
> +		ret = delete_branches(argc, argv, delete > 1, filter.kind,
> +				      quiet, 0, NULL);
>  		goto out;
>  	} else if (forked) {
>  		ret = list_forked_branches(argc, argv);