branch: let --delete-merged default to every upstream

From: Harald Nordgren <[email protected]>

Cleaning up every branch whose work has landed upstream required typing '*/*' as the pattern.

Let a bare "git branch --delete-merged" consider every upstream. As with "--merged" without a commit, this applies only when the option comes last, so "git branch --dry-run --delete-merged" previews the cleanup.

builtin/branch.c +2 -0

+			.defval = (intptr_t) "**",
+		},

From: Harald Nordgren <[email protected]>

Cleaning up every branch whose work has landed upstream required typing '*/*' as the pattern.

Let a bare "git branch --delete-merged" consider every upstream. As with "--merged" without a commit, this applies only when the option comes last, so "git branch --dry-run --delete-merged" previews the cleanup.

Signed-off-by Harald Nordgren <[email protected]>

---
branch: let --delete-merged default to every upstream

A bare git branch --delete-merged now considers every upstream.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2428%2FHaraldNordgren%2Fbranch-delete-merged-default-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2428/HaraldNordgren/branch-delete-merged-default-v1
Pull-Request: https://github.com/git/git/pull/2428

  Documentation/git-branch.adoc |  6 ++++--
  builtin/branch.c              | 14 +++++++++++---
  t/t3200-branch.sh             | 25 ++++++++++++++++++++++---
  3 files changed, 37 insertions(+), 8 deletions(-)

Documentation/git-branch.adoc +4 -2

index bfdf459329..4a91ae6879 100644
@@ -25,6 +25,7 @@ git branch (-m|-M) [<old-branch>] <new-branch>
  git branch (-c|-C) [<old-branch>] <new-branch>
  git branch (-d|-D) [-r] <branch-name>...
  git branch --edit-description [<branch-name>]
+git branch [--dry-run] --delete-merged
  git branch [--dry-run] (--delete-merged <pattern>)... [<branch-pattern>...]
 
  DESCRIPTION

@@ -202,14 +203,15 @@ This option is only applicable in non-verbose mode.
  	Print the name of the current branch. In detached `HEAD` state,
  	nothing is printed.
 
-`--delete-merged <pattern>`::
+`--delete-merged [<pattern>]`::
  	Delete local branches whose configured upstream matches
  	_<pattern>_, but only when their tip is reachable from that
  	upstream. In other words, the work on the branch has already
  	landed on the upstream it tracks, so the local copy is no longer
  	needed. _<pattern>_ may name a ref, a remote (using the branch its
  	`HEAD` points at), or a shell-style glob. The option can be
-	repeated to widen the upstream match.
+	repeated to widen the upstream match. Without _<pattern>_, every
+	upstream matches.
  	Optional _<branch-pattern>_ arguments limit which local branches
  	are considered, e.g. `git branch --delete-merged 'origin/*'
  	'topic-*'`.

builtin/branch.c +11 -3

index a613148fc7..f2a4e117dc 100644
@@ -39,6 +39,7 @@ static const char * const builtin_branch_usage[] = {
  	N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
  	N_("git branch [<options>] [-r | -a] [--points-at]"),
  	N_("git branch [<options>] [-r | -a] [--format]"),
+	N_("git branch [<options>] --delete-merged"),
  	N_("git branch [<options>] (--delete-merged <pattern>)... "
  	   "[<branch-pattern>...]"),
  	NULL

@@ -1029,9 +1030,16 @@ int cmd_branch(int argc,
  		OPT_BOOL(0, "create-reflog", &reflog, N_("create the branch's reflog")),
  		OPT_BOOL(0, "edit-description", &edit_description,
  			 N_("edit the description for the branch")),
-		OPT_CALLBACK_F(0, "delete-merged", &delete_merged, N_("pattern"),
-			N_("delete merged branches whose upstream matches <pattern> (repeatable)"),
-			PARSE_OPT_NONEG, parse_opt_strvec),
+		{
+			.type = OPTION_CALLBACK,
+			.long_name = "delete-merged",
+			.value = &delete_merged,
+			.argh = N_("pattern"),
+			.help = N_("delete merged branches whose upstream matches <pattern> (repeatable)"),
+			.flags = PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG,
+			.callback = parse_opt_strvec,
+			.defval = (intptr_t) "**",
+		},
  		OPT_BOOL(0, "dry-run", &dry_run,
  			N_("with --delete-merged, only print which branches would be deleted")),
  		OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),

t/t3200-branch.sh +22 -3

index cdb6c6a634..e60f4794c8 100755
@@ -2133,9 +2133,28 @@ test_expect_success '--delete-merged result is independent of stacked branch nam
  	)
  '
 
-test_expect_success '--delete-merged requires a value' '
-	test_must_fail git -C forked branch --delete-merged 2>err &&
-	test_grep "requires a value" err
+test_expect_success '--delete-merged without a pattern matches every upstream' '
+	setup_repo_for_delete_merged &&
+	create_merged_branch merged &&
+	(
+		cd repo &&
+		git branch --track local-topic main &&
+		git checkout --detach &&
+
+		git branch --dry-run --delete-merged &&
+
+		check_branches <<-\EOF &&
+		local-topic
+		main
+		merged
+		EOF
+
+		git branch --delete-merged &&
+
+		check_branches <<-\EOF
+		main
+		EOF
+	)
  '
 
  test_expect_success '--delete-merged honours branch.<name>.deleteMerged=false' '

base-commit: a018953688f1b10bddf91bff8747068f5f4746a4

With hindsight maybe

        git branch --delete-merged [<upstream>...] -- [<branch>...]

and

        git branch --forked [<upstream>...] -- [<branch>...]

would have been a better design. That's the sort of design mistake that is much more likely to happen when a contributor sends an endless stream of patches because they're eager to get something merged, rather than engaging in a thoughtful discussion with the reviewer.

Quoted 8 lines

With hindsight maybe

         git branch --delete-merged [<upstream>...] -- [<branch>...]

and

         git branch --forked [<upstream>...] -- [<branch>...]

Quoted 4 lines

would have been a better design. That's the sort of design mistake that is much more likely to happen when a contributor sends an endless stream of patches because they're eager to get something merged, rather than engaging in a thoughtful discussion with the reviewer.

I appreciate all the help here, but it's not necessary to throw blame either way. It sours the collaboration.

Quoted 8 lines

Quoted 8 lines

With hindsight maybe

         git branch --delete-merged [<upstream>...] -- [<branch>...]

and

         git branch --forked [<upstream>...] -- [<branch>...]

Quoted 7 lines

Quoted 4 lines

would have been a better design. That's the sort of design mistake that is much more likely to happen when a contributor sends an endless stream of patches because they're eager to get something merged, rather than engaging in a thoughtful discussion with the reviewer.

I appreciate all the help here, but it's not necessary to throw blame either way. It sours the collaboration.

I'm sorry if it sounded like I was blaming you. Getting a patch series merged is a collaborative effort and any issues we discover later are the collective responsibility of all those involved. I do though think it is helpful to think about what we can do to try and avoid design mistakes and feel that spending a bit more time discussing things and a bit less time re-rolling patches would help. I also think that would likely end up with things getting merged sooner as, if we've had a reasonably detailed discussion about the design and implementation, we're likely to have ironed out a lot of potential problems earlier in the evolution of the patch series.

... it is helpful to think about what we can do to try and avoid design mistakes and feel that spending a bit more time discussing things and a bit less time re-rolling patches would help. I also think that would likely end up with things getting merged sooner ...