All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Harald Nordgren via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: Harald Nordgren <haraldnordgren@gmail.com>
Subject: Re: [PATCH] branch: let --delete-merged default to every upstream
Date: Sun, 4 Oct 2026 10:55:06 +0100	[thread overview]
Message-ID: <ae47baff-daaa-4b78-97e9-94faebb8e694@gmail.com> (raw)
In-Reply-To: <pull.2428.git.git.1790960147943.gitgitgadget@gmail.com>

Hi Harald

On 02/10/2026 17:55, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
> 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.

PARSE_OPT_LASTARG_DEFAULT is a usability footgun that I think it is best 
to avoid. If we ever wanted to add a new option that worked with 
"--delete-merged" (for example to control whether it looked for branches 
that had been squashed) then

	git branch --delete-merged --foo

would behave differently to

	git branch --foo --delete-merged

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.

Thanks

Phillip

> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>      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(-)
> 
> diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
> index bfdf459329..4a91ae6879 100644
> --- a/Documentation/git-branch.adoc
> +++ b/Documentation/git-branch.adoc
> @@ -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-*'`.
> diff --git a/builtin/branch.c b/builtin/branch.c
> index a613148fc7..f2a4e117dc 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -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),
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index cdb6c6a634..e60f4794c8 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -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


  parent reply	other threads:[~2026-10-04  9:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-02 16:55 [PATCH] branch: let --delete-merged default to every upstream Harald Nordgren via GitGitGadget
2026-10-02 17:11 ` Junio C Hamano
2026-10-04  8:34 ` [PATCH v2] " Harald Nordgren via GitGitGadget
2026-10-04  9:55 ` Phillip Wood [this message]
2026-10-04 13:16   ` [PATCH] " Harald Nordgren
2026-10-04 18:42     ` Phillip Wood
2026-10-04 18:50       ` Harald Nordgren
2026-10-05  4:27       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae47baff-daaa-4b78-97e9-94faebb8e694@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=haraldnordgren@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.