diff mbox series

[3/3] completion: fix bugs with slashes in remote names

Message ID 95ffa62df6ce394249a8ddabb84fb2b517825fe3.1740901525.git.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series completion: fix bugs with slashes in remote names | expand

Commit Message

David Mandelberg March 2, 2025, 7:45 a.m. UTC 
From: David Mandelberg <david@mandelberg.org>

Previously, some calls to for-each-ref passed fixed numbers of path
components to strip from refs, assuming that remote names had no slashes
in them. This made completions like:

git push github/dseomn :com<Tab>

Result in:

git push github/dseomn :dseomn/completion-remote-slash

With this patch, it instead results in:

git push github/dseomn :completion-remote-slash

In addition, the change to __git_dwim_remote_heads() restricts it to
only list remote branches, not HEAD. I think that actually improves the
completion where it's used though, since HEAD doesn't seem to be valid
anyway:

$ git switch HEAD
fatal: a branch is expected, got 'HEAD'

Signed-off-by: David Mandelberg <david@mandelberg.org>
---
 contrib/completion/git-completion.bash |  17 ++-
 t/t9902-completion.sh                  | 180 ++++++++++++++++++++++---
 2 files changed, 169 insertions(+), 28 deletions(-)

Comments

Phillip Wood March 2, 2025, 2:17 p.m. UTC | #1 
Hi David

On 02/03/2025 07:45, David Mandelberg via GitGitGadget wrote:
> From: David Mandelberg <david@mandelberg.org>
> 
> Previously, some calls to for-each-ref passed fixed numbers of path
> components to strip from refs, assuming that remote names had no slashes
> in them. This made completions like:
> 
> git push github/dseomn :com<Tab>
> 
> Result in:
> 
> git push github/dseomn :dseomn/completion-remote-slash
> 
> With this patch, it instead results in:
> 
> git push github/dseomn :completion-remote-slash

This sounds like a useful improvement and I like the idea, but I think 
running "git for-each-ref" once for each remote is not going to scale 
very well for people who have a lot of remotes. I think it would be 
better to try and strip "refs/remote/$remote/" outside of "git 
for-each-ref". I've not tested it but I think something like

local sed_cmd=
local remote
# ref names and therefore remote names cannot contain '*?[]^' so we
# only need to escape '.$/'. Using 'sort -r' means that if there is a
# remote called "github" and another called "github/alice" we will try
# and strip "github/alice" first.
for remote in $(__git_remotes | sort -r)
do
	remote="${remote//./\\./}"
	remote="${remote//\$/\\\$/}"
	remote="${remote//\//\\\//}"
	sed_cmd="${sed_cmd} -e s/^refs\/remotes\/$remote\/// -e t"
done
__git for-each-ref --format="$fer_pfx$sfx" \
	${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
	"refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" |
sed $sed_cmd | sort -u

should work and means we're only forking three extra processes no matter 
how many remotes the user has. I'm not sure if it changes the output 
order when GIT_COMPLETION_IGNORE_CASE is set though.

Best Wishes

Phillip

> In addition, the change to __git_dwim_remote_heads() restricts it to
> only list remote branches, not HEAD. I think that actually improves the
> completion where it's used though, since HEAD doesn't seem to be valid
> anyway:
> 
> $ git switch HEAD
> fatal: a branch is expected, got 'HEAD'
> 
> Signed-off-by: David Mandelberg <david@mandelberg.org>
> ---
>   contrib/completion/git-completion.bash |  17 ++-
>   t/t9902-completion.sh                  | 180 ++++++++++++++++++++++---
>   2 files changed, 169 insertions(+), 28 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 17c044f7d60..5f4407be37f 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -807,11 +807,15 @@ __git_dwim_remote_heads ()
>   	# employ the heuristic used by git checkout and git switch
>   	# Try to find a remote branch that cur_es the completion word
>   	# but only output if the branch name is unique
> -	__git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
> -		--sort="refname:strip=3" \
> -		${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
> -		"refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" | \
> -	uniq -u
> +	local remote
> +	for remote in $(__git_remotes); do
> +		local base="$(__git_escape_fnmatch "refs/remotes/$remote")"
> +		local strip="$(__git_count_path_components "$base")"
> +		__git for-each-ref \
> +			--format="$fer_pfx%(refname:strip=$strip)$sfx" \
> +			${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
> +			"$base/$cur_*" "$base/$cur_*/**"
> +	done | sort | uniq -u
>   }
>   
>   # Lists refs from the local (by default) or from a remote repository.
> @@ -917,7 +921,8 @@ __git_refs ()
>   			case "HEAD" in
>   			$match*|$umatch*)	echo "${pfx}HEAD$sfx" ;;
>   			esac
> -			__git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
> +			local strip="$(__git_count_path_components "refs/remotes/$remote")"
> +			__git for-each-ref --format="$fer_pfx%(refname:strip=$strip)$sfx" \
>   				${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
>   				"refs/remotes/$remote/$match*" \
>   				"refs/remotes/$remote/$match*/**"
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 0219408358c..0c41ef9d5b1 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -149,7 +149,8 @@ fi
>   test_expect_success 'setup for __git_find_repo_path/__gitdir tests' '
>   	mkdir -p subdir/subsubdir &&
>   	mkdir -p non-repo &&
> -	git init -b main otherrepo
> +	git init -b main otherrepo &&
> +	git init -b main slashrepo
>   '
>   
>   test_expect_success '__git_find_repo_path - from command line (through $__git_dir)' '
> @@ -681,6 +682,13 @@ test_expect_success 'setup for ref completion' '
>   	) &&
>   	git remote add other "$ROOT/otherrepo/.git" &&
>   	git fetch --no-tags other &&
> +	(
> +		cd slashrepo &&
> +		git commit --allow-empty -m initial &&
> +		git branch -m main branch/with/slash
> +	) &&
> +	git remote add remote/with/slash "$ROOT/slashrepo/.git" &&
> +	git fetch --no-tags remote/with/slash &&
>   	rm -f .git/FETCH_HEAD &&
>   	git init thirdrepo
>   '
> @@ -693,6 +701,8 @@ test_expect_success '__git_refs - simple' '
>   	other/HEAD
>   	other/branch-in-other
>   	other/main-in-other
> +	remote/with/slash/HEAD
> +	remote/with/slash/branch/with/slash
>   	matching-tag
>   	EOF
>   	(
> @@ -709,6 +719,8 @@ test_expect_success '__git_refs - full refs' '
>   	refs/remotes/other/HEAD
>   	refs/remotes/other/branch-in-other
>   	refs/remotes/other/main-in-other
> +	refs/remotes/remote/with/slash/HEAD
> +	refs/remotes/remote/with/slash/branch/with/slash
>   	refs/tags/matching-tag
>   	EOF
>   	(
> @@ -774,6 +786,19 @@ test_expect_success '__git_refs - configured remote' '
>   	test_cmp expected "$actual"
>   '
>   
> +test_expect_success '__git_refs - configured remote - with slash' '
> +	cat >expected <<-EOF &&
> +	HEAD
> +	HEAD
> +	branch/with/slash
> +	EOF
> +	(
> +		cur= &&
> +		__git_refs remote/with/slash >"$actual"
> +	) &&
> +	test_cmp expected "$actual"
> +'
> +
>   test_expect_success '__git_refs - configured remote - full refs' '
>   	cat >expected <<-EOF &&
>   	HEAD
> @@ -916,17 +941,19 @@ test_expect_success '__git_refs - unique remote branches for git checkout DWIMer
>   	other/ambiguous
>   	other/branch-in-other
>   	other/main-in-other
> -	remote/ambiguous
> -	remote/branch-in-remote
> +	remote/with/slash/HEAD
> +	remote/with/slash/ambiguous
> +	remote/with/slash/branch-in-remote
> +	remote/with/slash/branch/with/slash
>   	matching-tag
> -	HEAD
>   	branch-in-other
>   	branch-in-remote
> +	branch/with/slash
>   	main-in-other
>   	EOF
>   	for remote_ref in refs/remotes/other/ambiguous \
> -		refs/remotes/remote/ambiguous \
> -		refs/remotes/remote/branch-in-remote
> +		refs/remotes/remote/with/slash/ambiguous \
> +		refs/remotes/remote/with/slash/branch-in-remote
>   	do
>   		git update-ref $remote_ref main &&
>   		test_when_finished "git update-ref -d $remote_ref" || return 1
> @@ -946,6 +973,8 @@ test_expect_success '__git_refs - after --opt=' '
>   	other/HEAD
>   	other/branch-in-other
>   	other/main-in-other
> +	remote/with/slash/HEAD
> +	remote/with/slash/branch/with/slash
>   	matching-tag
>   	EOF
>   	(
> @@ -962,6 +991,8 @@ test_expect_success '__git_refs - after --opt= - full refs' '
>   	refs/remotes/other/HEAD
>   	refs/remotes/other/branch-in-other
>   	refs/remotes/other/main-in-other
> +	refs/remotes/remote/with/slash/HEAD
> +	refs/remotes/remote/with/slash/branch/with/slash
>   	refs/tags/matching-tag
>   	EOF
>   	(
> @@ -979,6 +1010,8 @@ test_expect_success '__git refs - excluding refs' '
>   	^other/HEAD
>   	^other/branch-in-other
>   	^other/main-in-other
> +	^remote/with/slash/HEAD
> +	^remote/with/slash/branch/with/slash
>   	^matching-tag
>   	EOF
>   	(
> @@ -995,6 +1028,8 @@ test_expect_success '__git refs - excluding full refs' '
>   	^refs/remotes/other/HEAD
>   	^refs/remotes/other/branch-in-other
>   	^refs/remotes/other/main-in-other
> +	^refs/remotes/remote/with/slash/HEAD
> +	^refs/remotes/remote/with/slash/branch/with/slash
>   	^refs/tags/matching-tag
>   	EOF
>   	(
> @@ -1022,6 +1057,8 @@ test_expect_success '__git_refs - do not filter refs unless told so' '
>   	other/branch-in-other
>   	other/main-in-other
>   	other/matching/branch-in-other
> +	remote/with/slash/HEAD
> +	remote/with/slash/branch/with/slash
>   	matching-tag
>   	matching/tag
>   	EOF
> @@ -1142,6 +1179,8 @@ test_expect_success '__git_complete_refs - simple' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	matching-tag Z
>   	EOF
>   	(
> @@ -1180,6 +1219,20 @@ test_expect_success '__git_complete_refs - remote' '
>   	test_cmp expected out
>   '
>   
> +test_expect_success '__git_complete_refs - remote - with slash' '
> +	sed -e "s/Z$//" >expected <<-EOF &&
> +	HEAD Z
> +	HEAD Z
> +	branch/with/slash Z
> +	EOF
> +	(
> +		cur= &&
> +		__git_complete_refs --remote=remote/with/slash &&
> +		print_comp
> +	) &&
> +	test_cmp expected out
> +'
> +
>   test_expect_success '__git_complete_refs - track' '
>   	sed -e "s/Z$//" >expected <<-EOF &&
>   	HEAD Z
> @@ -1188,9 +1241,11 @@ test_expect_success '__git_complete_refs - track' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	matching-tag Z
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main-in-other Z
>   	EOF
>   	(
> @@ -1235,6 +1290,8 @@ test_expect_success '__git_complete_refs - suffix' '
>   	other/HEAD.
>   	other/branch-in-other.
>   	other/main-in-other.
> +	remote/with/slash/HEAD.
> +	remote/with/slash/branch/with/slash.
>   	matching-tag.
>   	EOF
>   	(
> @@ -1260,6 +1317,20 @@ test_expect_success '__git_complete_fetch_refspecs - simple' '
>   	test_cmp expected out
>   '
>   
> +test_expect_success '__git_complete_fetch_refspecs - with slash' '
> +	sed -e "s/Z$//" >expected <<-EOF &&
> +	HEAD:HEAD Z
> +	HEAD:HEAD Z
> +	branch/with/slash:branch/with/slash Z
> +	EOF
> +	(
> +		cur= &&
> +		__git_complete_fetch_refspecs remote/with/slash &&
> +		print_comp
> +	) &&
> +	test_cmp expected out
> +'
> +
>   test_expect_success '__git_complete_fetch_refspecs - matching' '
>   	sed -e "s/Z$//" >expected <<-EOF &&
>   	branch-in-other:branch-in-other Z
> @@ -1340,8 +1411,8 @@ test_expect_success '__git_complete_worktree_paths with -C' '
>   
>   test_expect_success 'git switch - with no options, complete local branches and unique remote branch names for DWIM logic' '
>   	test_completion "git switch " <<-\EOF
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -1487,8 +1558,8 @@ test_expect_success 'git-bisect - existing view subcommand is recognized and ena
>   test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
>   	test_completion "git checkout " <<-\EOF
>   	HEAD Z
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -1496,6 +1567,8 @@ test_expect_success 'git checkout - completes refs and unique remote branches fo
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1515,8 +1588,8 @@ test_expect_success 'git switch - with GIT_COMPLETION_CHECKOUT_NO_GUESS=1, compl
>   
>   test_expect_success 'git switch - --guess overrides GIT_COMPLETION_CHECKOUT_NO_GUESS=1, complete local branches and unique remote names for DWIM logic' '
>   	GIT_COMPLETION_CHECKOUT_NO_GUESS=1 test_completion "git switch --guess " <<-\EOF
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -1525,8 +1598,8 @@ test_expect_success 'git switch - --guess overrides GIT_COMPLETION_CHECKOUT_NO_G
>   
>   test_expect_success 'git switch - a later --guess overrides previous --no-guess, complete local and remote unique branches for DWIM' '
>   	test_completion "git switch --no-guess --guess " <<-\EOF
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -1549,14 +1622,16 @@ test_expect_success 'git checkout - with GIT_COMPLETION_NO_GUESS=1 only complete
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
>   test_expect_success 'git checkout - --guess overrides GIT_COMPLETION_NO_GUESS=1, complete refs and unique remote branches for DWIM' '
>   	GIT_COMPLETION_CHECKOUT_NO_GUESS=1 test_completion "git checkout --guess " <<-\EOF
>   	HEAD Z
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -1564,6 +1639,8 @@ test_expect_success 'git checkout - --guess overrides GIT_COMPLETION_NO_GUESS=1,
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1576,14 +1653,16 @@ test_expect_success 'git checkout - with --no-guess, only completes refs' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
>   test_expect_success 'git checkout - a later --guess overrides previous --no-guess, complete refs and unique remote branches for DWIM' '
>   	test_completion "git checkout --no-guess --guess " <<-\EOF
>   	HEAD Z
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -1591,6 +1670,8 @@ test_expect_success 'git checkout - a later --guess overrides previous --no-gues
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1603,6 +1684,8 @@ test_expect_success 'git checkout - a later --no-guess overrides previous --gues
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1616,6 +1699,8 @@ test_expect_success 'git checkout - with checkout.guess = false, only completes
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1623,8 +1708,8 @@ test_expect_success 'git checkout - with checkout.guess = true, completes refs a
>   	test_config checkout.guess true &&
>   	test_completion "git checkout " <<-\EOF
>   	HEAD Z
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -1632,6 +1717,8 @@ test_expect_success 'git checkout - with checkout.guess = true, completes refs a
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1639,8 +1726,8 @@ test_expect_success 'git checkout - a later --guess overrides previous checkout.
>   	test_config checkout.guess false &&
>   	test_completion "git checkout --guess " <<-\EOF
>   	HEAD Z
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -1648,6 +1735,8 @@ test_expect_success 'git checkout - a later --guess overrides previous checkout.
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1661,6 +1750,8 @@ test_expect_success 'git checkout - a later --no-guess overrides previous checko
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1673,6 +1764,8 @@ test_expect_success 'git switch - with --detach, complete all references' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1685,6 +1778,8 @@ test_expect_success 'git checkout - with --detach, complete only references' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1857,6 +1952,8 @@ test_expect_success 'git switch - with -d, complete all references' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1869,6 +1966,8 @@ test_expect_success 'git checkout - with -d, complete only references' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1877,11 +1976,15 @@ test_expect_success 'git switch - with --track, complete only remote branches' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   	test_completion "git switch -t " <<-\EOF
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1890,11 +1993,15 @@ test_expect_success 'git checkout - with --track, complete only remote branches'
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   	test_completion "git checkout -t " <<-\EOF
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1914,6 +2021,8 @@ test_expect_success 'git checkout - with --no-track, complete only local referen
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1926,6 +2035,8 @@ test_expect_success 'git switch - with -c, complete all references' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1938,6 +2049,8 @@ test_expect_success 'git switch - with -C, complete all references' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1950,6 +2063,8 @@ test_expect_success 'git switch - with -c and --track, complete all references'
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1962,6 +2077,8 @@ test_expect_success 'git switch - with -C and --track, complete all references'
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1974,6 +2091,8 @@ test_expect_success 'git switch - with -c and --no-track, complete all reference
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1986,6 +2105,8 @@ test_expect_success 'git switch - with -C and --no-track, complete all reference
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -1998,6 +2119,8 @@ test_expect_success 'git checkout - with -b, complete all references' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -2010,6 +2133,8 @@ test_expect_success 'git checkout - with -B, complete all references' '
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -2022,6 +2147,8 @@ test_expect_success 'git checkout - with -b and --track, complete all references
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -2034,6 +2161,8 @@ test_expect_success 'git checkout - with -B and --track, complete all references
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -2046,6 +2175,8 @@ test_expect_success 'git checkout - with -b and --no-track, complete all referen
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -2058,13 +2189,15 @@ test_expect_success 'git checkout - with -B and --no-track, complete all referen
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
>   test_expect_success 'git switch - for -c, complete local branches and unique remote branches' '
>   	test_completion "git switch -c " <<-\EOF
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -2073,8 +2206,8 @@ test_expect_success 'git switch - for -c, complete local branches and unique rem
>   
>   test_expect_success 'git switch - for -C, complete local branches and unique remote branches' '
>   	test_completion "git switch -C " <<-\EOF
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -2111,8 +2244,8 @@ test_expect_success 'git switch - for -C with --no-track, complete local branche
>   
>   test_expect_success 'git checkout - for -b, complete local branches and unique remote branches' '
>   	test_completion "git checkout -b " <<-\EOF
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -2121,8 +2254,8 @@ test_expect_success 'git checkout - for -b, complete local branches and unique r
>   
>   test_expect_success 'git checkout - for -B, complete local branches and unique remote branches' '
>   	test_completion "git checkout -B " <<-\EOF
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -2159,8 +2292,8 @@ test_expect_success 'git checkout - for -B with --no-track, complete local branc
>   
>   test_expect_success 'git switch - with --orphan completes local branch names and unique remote branch names' '
>   	test_completion "git switch --orphan " <<-\EOF
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -2175,8 +2308,8 @@ test_expect_success 'git switch - --orphan with branch already provided complete
>   
>   test_expect_success 'git checkout - with --orphan completes local branch names and unique remote branch names' '
>   	test_completion "git checkout --orphan " <<-\EOF
> -	HEAD Z
>   	branch-in-other Z
> +	branch/with/slash Z
>   	main Z
>   	main-in-other Z
>   	matching-branch Z
> @@ -2192,6 +2325,8 @@ test_expect_success 'git checkout - --orphan with branch already provided comple
>   	other/HEAD Z
>   	other/branch-in-other Z
>   	other/main-in-other Z
> +	remote/with/slash/HEAD Z
> +	remote/with/slash/branch/with/slash Z
>   	EOF
>   '
>   
> @@ -2206,7 +2341,8 @@ test_expect_success 'git restore completes modified files' '
>   test_expect_success 'teardown after ref completion' '
>   	git branch -d matching-branch &&
>   	git tag -d matching-tag &&
> -	git remote remove other
> +	git remote remove other &&
> +	git remote remove remote/with/slash
>   '
>   
>
David Mandelberg March 2, 2025, 8:34 p.m. UTC | #2 
Op 2025-03-02 om 09:17 schreef Phillip Wood:
> Hi David
> 
> On 02/03/2025 07:45, David Mandelberg via GitGitGadget wrote:
>> From: David Mandelberg <david@mandelberg.org>
>>
>> Previously, some calls to for-each-ref passed fixed numbers of path
>> components to strip from refs, assuming that remote names had no slashes
>> in them. This made completions like:
>>
>> git push github/dseomn :com<Tab>
>>
>> Result in:
>>
>> git push github/dseomn :dseomn/completion-remote-slash
>>
>> With this patch, it instead results in:
>>
>> git push github/dseomn :completion-remote-slash
> 
> This sounds like a useful improvement and I like the idea, but I think 
> running "git for-each-ref" once for each remote is not going to scale 
> very well for people who have a lot of remotes. I think it would be 
> better to try and strip "refs/remote/$remote/" outside of "git for-each- 
> ref". I've not tested it but I think something like

Good point, I hadn't thought of that. Do you have a rough estimate of 
what "a lot of remotes" is? 100ish, maybe? I'd like to do some testing 
to get actual performance numbers before trying to optimize this, 
because I think the optimization has some drawbacks, see below.

If optimization is needed, another approach is to parallelize the forks:

{
	local fer_pids=
	for ...
	do
		__git for-each-ref ... &
		fer_pids="$fer_pids $!"
	done
	test -z "$fer_pids" || wait $fer_pids
} | sort | uniq -u

That might cause spikes in cpu/memory/disk usage that aren't ideal though.

> local sed_cmd=
> local remote
> # ref names and therefore remote names cannot contain '*?[]^' so we
> # only need to escape '.$/'. Using 'sort -r' means that if there is a
> # remote called "github" and another called "github/alice" we will try
> # and strip "github/alice" first.
> for remote in $(__git_remotes | sort -r)
> do
>      remote="${remote//./\\./}"
>      remote="${remote//\$/\\\$/}"
>      remote="${remote//\//\\\//}"

Just FYI since it took me hours to figure this out myself: I think this 
would break tests on macos because of an old version of bash that 
handles backslashes weirdly. I think removing the double quotes would 
work around that issue, and be safe because word splitting doesn't 
happen in assignments.

>      sed_cmd="${sed_cmd} -e s/^refs\/remotes\/$remote\/// -e t"
> done

Mostly just a note to myself if I end up using this idea: I think a 
space in $remote would break this because bash would split up the arg to 
sed. There's probably some way to fix that with extra escaping though?

> __git for-each-ref --format="$fer_pfx$sfx" \
>      ${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
>      "refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" |

This would search for $cur_ in the wrong place because * only matches 
one path component, right? Changing to ** might help, but then it would 
match in places more it shouldn't.

> sed $sed_cmd | sort -u
> 
> should work and means we're only forking three extra processes no matter 
> how many remotes the user has. I'm not sure if it changes the output 
> order when GIT_COMPLETION_IGNORE_CASE is set though.
> 
> Best Wishes
> 
> Phillip
Phillip Wood March 3, 2025, 4:36 p.m. UTC | #3 
Hi David

On 02/03/2025 20:34, David Mandelberg wrote:
> Op 2025-03-02 om 09:17 schreef Phillip Wood:
>> On 02/03/2025 07:45, David Mandelberg via GitGitGadget wrote:
>>> From: David Mandelberg <david@mandelberg.org>
>>>
>> This sounds like a useful improvement and I like the idea, but I think 
>> running "git for-each-ref" once for each remote is not going to scale 
>> very well for people who have a lot of remotes. I think it would be 
>> better to try and strip "refs/remote/$remote/" outside of "git for- 
>> each- ref". I've not tested it but I think something like
> 
> Good point, I hadn't thought of that. Do you have a rough estimate of 
> what "a lot of remotes" is? 100ish, maybe?

I'm not really sure what a "large" number looks like for people who 
track a lot of repositories. On windows forking is pretty slow (our 
shell-based test suite is incredibly slow on that platform) so I think 
it makes sense to try and avoid adding new processes.

> I'd like to do some testing 
> to get actual performance numbers before trying to optimize this, 
> because I think the optimization has some drawbacks, see below.
> 
> If optimization is needed, another approach is to parallelize the forks:
> 
> {
>      local fer_pids=
>      for ...
>      do
>          __git for-each-ref ... &
>          fer_pids="$fer_pids $!"
>      done
>      test -z "$fer_pids" || wait $fer_pids
> } | sort | uniq -u
> 
> That might cause spikes in cpu/memory/disk usage that aren't ideal though.

Yes, if there were a 100 remotes that's a bit of a fork-bomb.

>> local sed_cmd=
>> local remote
>> # ref names and therefore remote names cannot contain '*?[]^' so we
>> # only need to escape '.$/'. Using 'sort -r' means that if there is a
>> # remote called "github" and another called "github/alice" we will try
>> # and strip "github/alice" first.
>> for remote in $(__git_remotes | sort -r)
>> do
>>      remote="${remote//./\\./}"
>>      remote="${remote//\$/\\\$/}"
>>      remote="${remote//\//\\\//}"
> 
> Just FYI since it took me hours to figure this out myself: I think this 
> would break tests on macos because of an old version of bash that 
> handles backslashes weirdly. I think removing the double quotes would 
> work around that issue, and be safe because word splitting doesn't 
> happen in assignments.

Thanks, I'm not familiar with bash's extensions to parameter 
substitution. The completions can also but used under zsh 
(git-completion.zsh is pretty much abandoned I think) but it looks like 
bash and zsh agree on this expansion.

>>      sed_cmd="${sed_cmd} -e s/^refs\/remotes\/$remote\/// -e t"
>> done
> 
> Mostly just a note to myself if I end up using this idea: I think a 
> space in $remote would break this because bash would split up the arg to 
> sed. There's probably some way to fix that with extra escaping though?

I should have said in the comment above that ref names cannot contain 
space, tab, newline etc. so we don't have to worry about the shell's 
word splitting splitting the patterns. They cannot contain backslashes 
either.

>> __git for-each-ref --format="$fer_pfx$sfx" \
>>      ${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
>>      "refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" |
> 
> This would search for $cur_ in the wrong place because * only matches 
> one path component, right? Changing to ** might help, but then it would 
> match in places more it shouldn't.

Oh, good point I'd missed that. Could we change sed to run with '-n' and 
generate an expression that looks like

     -e s|^refs/remotes/$remote/\($cur_\)|\1|p -e t

with the appropriate escaping of $remote and $cur_ so that we only print 
the output of for-each-ref that matches $cur_?

Best Wishes

Phillip

>> sed $sed_cmd | sort -u
>>
>> should work and means we're only forking three extra processes no 
>> matter how many remotes the user has. I'm not sure if it changes the 
>> output order when GIT_COMPLETION_IGNORE_CASE is set though.
>>
>> Best Wishes
>>
>> Phillip
>
David Mandelberg March 3, 2025, 7:19 p.m. UTC | #4 
Op 2025-03-03 om 11:36 schreef phillip.wood123@gmail.com:
> I'm not really sure what a "large" number looks like for people who 
> track a lot of repositories. On windows forking is pretty slow (our 
> shell-based test suite is incredibly slow on that platform) so I think 
> it makes sense to try and avoid adding new processes.

Thanks, I didn't know about forking being slow on Windows.

Do you know if we need to worry about hitting the max number or length 
of arguments to a command? 
https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/limits.h.html 
under _POSIX_ARG_MAX says that at least 4KiB should be supported, which 
seems low enough to maybe cause trouble, but it's a minimum. 
https://www.in-ulm.de/~mascheck/various/argmax/ lists a bunch of values 
for different platforms, and from a brief scan it looks like all the 
remotely-modern platforms have values that are probably large enough for 
this.

>> I'd like to do some testing to get actual performance numbers before 
>> trying to optimize this, because I think the optimization has some 
>> drawbacks, see below.
>>
>> If optimization is needed, another approach is to parallelize the forks:
>>
>> {
>>      local fer_pids=
>>      for ...
>>      do
>>          __git for-each-ref ... &
>>          fer_pids="$fer_pids $!"
>>      done
>>      test -z "$fer_pids" || wait $fer_pids
>> } | sort | uniq -u
>>
>> That might cause spikes in cpu/memory/disk usage that aren't ideal 
>> though.
> 
> Yes, if there were a 100 remotes that's a bit of a fork-bomb.

I also realized after sending that email that it could run into issues 
with the number of available PIDs too. It could be improved by limiting 
to n child processes at a time and waiting for one to finish before 
starting the next, but that's more complication. I think your solution 
below is better.

>>> local sed_cmd=
>>> local remote
>>> # ref names and therefore remote names cannot contain '*?[]^' so we
>>> # only need to escape '.$/'. Using 'sort -r' means that if there is a
>>> # remote called "github" and another called "github/alice" we will try
>>> # and strip "github/alice" first.
>>> for remote in $(__git_remotes | sort -r)
>>> do
>>>      remote="${remote//./\\./}"
>>>      remote="${remote//\$/\\\$/}"
>>>      remote="${remote//\//\\\//}"
>>
>> Just FYI since it took me hours to figure this out myself: I think 
>> this would break tests on macos because of an old version of bash that 
>> handles backslashes weirdly. I think removing the double quotes would 
>> work around that issue, and be safe because word splitting doesn't 
>> happen in assignments.
> 
> Thanks, I'm not familiar with bash's extensions to parameter 
> substitution. The completions can also but used under zsh (git- 
> completion.zsh is pretty much abandoned I think) but it looks like bash 
> and zsh agree on this expansion.

The tests don't automatically run under zsh though, right? Maybe I 
should try to figure out how to do that in a separate patch...

>>>      sed_cmd="${sed_cmd} -e s/^refs\/remotes\/$remote\/// -e t"
>>> done
>>
>> Mostly just a note to myself if I end up using this idea: I think a 
>> space in $remote would break this because bash would split up the arg 
>> to sed. There's probably some way to fix that with extra escaping though?
> 
> I should have said in the comment above that ref names cannot contain 
> space, tab, newline etc. so we don't have to worry about the shell's 
> word splitting splitting the patterns. They cannot contain backslashes 
> either.

Good to know. Is https://git-scm.com/docs/git-check-ref-format the place 
I should have read to understand what escaping is needed and what isn't 
because of the limitations on ref names?

>>> __git for-each-ref --format="$fer_pfx$sfx" \
>>>      ${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
>>>      "refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" |
>>
>> This would search for $cur_ in the wrong place because * only matches 
>> one path component, right? Changing to ** might help, but then it 
>> would match in places more it shouldn't.
> 
> Oh, good point I'd missed that. Could we change sed to run with '-n' and 
> generate an expression that looks like
> 
>      -e s|^refs/remotes/$remote/\($cur_\)|\1|p -e t
> 
> with the appropriate escaping of $remote and $cur_ so that we only print 
> the output of for-each-ref that matches $cur_?

I think that would work. I was hoping to avoid sed, and I was thinking 
about using shell's ${parameter#word} to strip and test for matching 
prefixes. But I can't think of a way to do that with 
GIT_COMPLETION_IGNORE_CASE. Modern bash has ${parameter@L} to lowercase 
a string, but I don't think the version of bash on macos has that. With 
sed it would just be a matter of adding the i flag if needed.

I don't think this is an issue, but do we need to worry about putting 
remote names and $cur_ on the command line where any other user on the 
same system can read them? $cur_ was already on the command line before 
this patch, and remote names appear on other git command lines all the 
time, so I doubt they're meant to be secret.

Is a Helped-by trailer the right way to credit you for your suggestions 
on this?
SZEDER Gábor March 3, 2025, 7:33 p.m. UTC | #5 
On Sun, Mar 02, 2025 at 03:34:07PM -0500, David Mandelberg wrote:
> Op 2025-03-02 om 09:17 schreef Phillip Wood:
> > Hi David
> > 
> > On 02/03/2025 07:45, David Mandelberg via GitGitGadget wrote:
> > > From: David Mandelberg <david@mandelberg.org>
> > > 
> > > Previously, some calls to for-each-ref passed fixed numbers of path
> > > components to strip from refs, assuming that remote names had no slashes
> > > in them. This made completions like:
> > > 
> > > git push github/dseomn :com<Tab>
> > > 
> > > Result in:
> > > 
> > > git push github/dseomn :dseomn/completion-remote-slash
> > > 
> > > With this patch, it instead results in:
> > > 
> > > git push github/dseomn :completion-remote-slash
> > 
> > This sounds like a useful improvement and I like the idea, but I think
> > running "git for-each-ref" once for each remote is not going to scale
> > very well for people who have a lot of remotes. I think it would be
> > better to try and strip "refs/remote/$remote/" outside of "git for-each-
> > ref". I've not tested it but I think something like
> 
> Good point, I hadn't thought of that. Do you have a rough estimate of what
> "a lot of remotes" is? 100ish, maybe?

5.

In Git for Windows fork()-ing subshells and fork()+exec()-ing
processes is rather costly, about an order of magnitude slower than on
Linux.  The rough equivalent of the body of your loop, with two
subshells and a git process:

  time { a=$(echo 1) ; b=$(echo 2) ; git for-each-ref >/dev/null ; }

takes on average 0.17s on a windows box I have access to (with fully
packed refs, and merely 4 refs in total).  So guess at about 4-5
remotes it would take over a second to react to my TABs...

I would rather try to go in the opposite direction to see whether 'git
for-each-ref' could be taught to strip the "refs/remote/$remote/"
prefix with a format specifier option like '%(refname:strip=remote)'.

That would surely be faster than any shell filtering we might come up
with, and would also save us from the trouble of escaping glob and/or
regex metacharacters for shell/sed pattern matching.

> I'd like to do some testing to get
> actual performance numbers before trying to optimize this, because I think
> the optimization has some drawbacks, see below.
> 
> If optimization is needed, another approach is to parallelize the forks:
> 
> {
> 	local fer_pids=
> 	for ...
> 	do
> 		__git for-each-ref ... &
> 		fer_pids="$fer_pids $!"
> 	done
> 	test -z "$fer_pids" || wait $fer_pids
> } | sort | uniq -u
> 
> That might cause spikes in cpu/memory/disk usage that aren't ideal though.

Please don't do this, the completion script must not forkbomb the
system.
Todd Zullinger March 3, 2025, 7:43 p.m. UTC | #6 
David Mandelberg wrote:
> I think that would work. I was hoping to avoid sed, and I was thinking about
> using shell's ${parameter#word} to strip and test for matching prefixes. But
> I can't think of a way to do that with GIT_COMPLETION_IGNORE_CASE. Modern
> bash has ${parameter@L} to lowercase a string, but I don't think the version
> of bash on macos has that. With sed it would just be a matter of adding the
> i flag if needed.

I haven't followed this discussion closely, but you can
also use ${parameter,,} to do parameter case modification.

Sadly, it looks like those were added in bash-4.0, so Apple
will have kept their users from benefiting from that to
avoid GPLv3 code. :/

I don't have an ancient bash-3.x like MacOS to test, but
maybe they've backported some features?

Though if you do end up using this, ${parameter,,} will work
in a much wider range of bash version that ${parameter@L}
which covers various releases in the OS's we aim to target
(e.g. RHEL 8.x, which has bash-4.4).
David Mandelberg March 3, 2025, 7:49 p.m. UTC | #7 
Op 2025-03-03 om 14:33 schreef SZEDER Gábor:
> On Sun, Mar 02, 2025 at 03:34:07PM -0500, David Mandelberg wrote:
>> Good point, I hadn't thought of that. Do you have a rough estimate of what
>> "a lot of remotes" is? 100ish, maybe?
> 
> 5.
> 
> In Git for Windows fork()-ing subshells and fork()+exec()-ing
> processes is rather costly, about an order of magnitude slower than on
> Linux.  The rough equivalent of the body of your loop, with two
> subshells and a git process:
> 
>    time { a=$(echo 1) ; b=$(echo 2) ; git for-each-ref >/dev/null ; }
> 
> takes on average 0.17s on a windows box I have access to (with fully
> packed refs, and merely 4 refs in total).  So guess at about 4-5
> remotes it would take over a second to react to my TABs...

Oh wow, that's much slower than I expected, thanks for the numbers!

> I would rather try to go in the opposite direction to see whether 'git
> for-each-ref' could be taught to strip the "refs/remote/$remote/"
> prefix with a format specifier option like '%(refname:strip=remote)'.
> 
> That would surely be faster than any shell filtering we might come up
> with, and would also save us from the trouble of escaping glob and/or
> regex metacharacters for shell/sed pattern matching.

That doesn't fix the issue of needing to anchor the $cur_ part of the 
pattern after each refs/remote/$remote/ prefix. It might be possible to 
use something like %(refname:strip=remote) to find all remote branches 
with the remote stripped, then filter them for $cur_ in the shell or 
with sed though.

What are the expectations around mixing versions of git itself and files 
in contrib? Can a single patch series add a feature to for-each-ref and 
use it in git-completion.bash, or does the feature in for-each-ref need 
to exist for some time before git-completion.bash can use it?
Junio C Hamano March 3, 2025, 8:29 p.m. UTC | #8 
phillip.wood123@gmail.com writes:

>>      local fer_pids=
>>      for ...
>>      do
>>          __git for-each-ref ... &
>>          fer_pids="$fer_pids $!"
>>      done
>>      test -z "$fer_pids" || wait $fer_pids
>> } | sort | uniq -u
>> That might cause spikes in cpu/memory/disk usage that aren't ideal
>> though.
>
> Yes, if there were a 100 remotes that's a bit of a fork-bomb.

And they are all competing for a shared common resource for read
access.

I wonder if a single invocation of for-each-ref for refs/remotes/
hierarchy would be sufficient?  That is, the calling shell script
can set up a shell function "foo" and then

    eval $(for-each-ref --format='foo %(refname)' --shell refs/remotes/)

and "foo" can do its thing avoiding as much forks as possible.  As
you are allowed bash-ism in completion script, you do not have to
limit yourself to POSIX parameter substitution but can use substring
expansion ${parameter:offset:length} and pattern substitution
${parameter/pattern/string} and other kind of bash exotics, as well
as shell arrays (e.g., define an array __git__remote_branches[] for
each remote, which may contain the remote-tracking branches for the
remote).
diff mbox series

Patch

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 17c044f7d60..5f4407be37f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -807,11 +807,15 @@  __git_dwim_remote_heads ()
 	# employ the heuristic used by git checkout and git switch
 	# Try to find a remote branch that cur_es the completion word
 	# but only output if the branch name is unique
-	__git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
-		--sort="refname:strip=3" \
-		${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
-		"refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" | \
-	uniq -u
+	local remote
+	for remote in $(__git_remotes); do
+		local base="$(__git_escape_fnmatch "refs/remotes/$remote")"
+		local strip="$(__git_count_path_components "$base")"
+		__git for-each-ref \
+			--format="$fer_pfx%(refname:strip=$strip)$sfx" \
+			${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
+			"$base/$cur_*" "$base/$cur_*/**"
+	done | sort | uniq -u
 }
 
 # Lists refs from the local (by default) or from a remote repository.
@@ -917,7 +921,8 @@  __git_refs ()
 			case "HEAD" in
 			$match*|$umatch*)	echo "${pfx}HEAD$sfx" ;;
 			esac
-			__git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
+			local strip="$(__git_count_path_components "refs/remotes/$remote")"
+			__git for-each-ref --format="$fer_pfx%(refname:strip=$strip)$sfx" \
 				${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
 				"refs/remotes/$remote/$match*" \
 				"refs/remotes/$remote/$match*/**"
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 0219408358c..0c41ef9d5b1 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -149,7 +149,8 @@  fi
 test_expect_success 'setup for __git_find_repo_path/__gitdir tests' '
 	mkdir -p subdir/subsubdir &&
 	mkdir -p non-repo &&
-	git init -b main otherrepo
+	git init -b main otherrepo &&
+	git init -b main slashrepo
 '
 
 test_expect_success '__git_find_repo_path - from command line (through $__git_dir)' '
@@ -681,6 +682,13 @@  test_expect_success 'setup for ref completion' '
 	) &&
 	git remote add other "$ROOT/otherrepo/.git" &&
 	git fetch --no-tags other &&
+	(
+		cd slashrepo &&
+		git commit --allow-empty -m initial &&
+		git branch -m main branch/with/slash
+	) &&
+	git remote add remote/with/slash "$ROOT/slashrepo/.git" &&
+	git fetch --no-tags remote/with/slash &&
 	rm -f .git/FETCH_HEAD &&
 	git init thirdrepo
 '
@@ -693,6 +701,8 @@  test_expect_success '__git_refs - simple' '
 	other/HEAD
 	other/branch-in-other
 	other/main-in-other
+	remote/with/slash/HEAD
+	remote/with/slash/branch/with/slash
 	matching-tag
 	EOF
 	(
@@ -709,6 +719,8 @@  test_expect_success '__git_refs - full refs' '
 	refs/remotes/other/HEAD
 	refs/remotes/other/branch-in-other
 	refs/remotes/other/main-in-other
+	refs/remotes/remote/with/slash/HEAD
+	refs/remotes/remote/with/slash/branch/with/slash
 	refs/tags/matching-tag
 	EOF
 	(
@@ -774,6 +786,19 @@  test_expect_success '__git_refs - configured remote' '
 	test_cmp expected "$actual"
 '
 
+test_expect_success '__git_refs - configured remote - with slash' '
+	cat >expected <<-EOF &&
+	HEAD
+	HEAD
+	branch/with/slash
+	EOF
+	(
+		cur= &&
+		__git_refs remote/with/slash >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
 test_expect_success '__git_refs - configured remote - full refs' '
 	cat >expected <<-EOF &&
 	HEAD
@@ -916,17 +941,19 @@  test_expect_success '__git_refs - unique remote branches for git checkout DWIMer
 	other/ambiguous
 	other/branch-in-other
 	other/main-in-other
-	remote/ambiguous
-	remote/branch-in-remote
+	remote/with/slash/HEAD
+	remote/with/slash/ambiguous
+	remote/with/slash/branch-in-remote
+	remote/with/slash/branch/with/slash
 	matching-tag
-	HEAD
 	branch-in-other
 	branch-in-remote
+	branch/with/slash
 	main-in-other
 	EOF
 	for remote_ref in refs/remotes/other/ambiguous \
-		refs/remotes/remote/ambiguous \
-		refs/remotes/remote/branch-in-remote
+		refs/remotes/remote/with/slash/ambiguous \
+		refs/remotes/remote/with/slash/branch-in-remote
 	do
 		git update-ref $remote_ref main &&
 		test_when_finished "git update-ref -d $remote_ref" || return 1
@@ -946,6 +973,8 @@  test_expect_success '__git_refs - after --opt=' '
 	other/HEAD
 	other/branch-in-other
 	other/main-in-other
+	remote/with/slash/HEAD
+	remote/with/slash/branch/with/slash
 	matching-tag
 	EOF
 	(
@@ -962,6 +991,8 @@  test_expect_success '__git_refs - after --opt= - full refs' '
 	refs/remotes/other/HEAD
 	refs/remotes/other/branch-in-other
 	refs/remotes/other/main-in-other
+	refs/remotes/remote/with/slash/HEAD
+	refs/remotes/remote/with/slash/branch/with/slash
 	refs/tags/matching-tag
 	EOF
 	(
@@ -979,6 +1010,8 @@  test_expect_success '__git refs - excluding refs' '
 	^other/HEAD
 	^other/branch-in-other
 	^other/main-in-other
+	^remote/with/slash/HEAD
+	^remote/with/slash/branch/with/slash
 	^matching-tag
 	EOF
 	(
@@ -995,6 +1028,8 @@  test_expect_success '__git refs - excluding full refs' '
 	^refs/remotes/other/HEAD
 	^refs/remotes/other/branch-in-other
 	^refs/remotes/other/main-in-other
+	^refs/remotes/remote/with/slash/HEAD
+	^refs/remotes/remote/with/slash/branch/with/slash
 	^refs/tags/matching-tag
 	EOF
 	(
@@ -1022,6 +1057,8 @@  test_expect_success '__git_refs - do not filter refs unless told so' '
 	other/branch-in-other
 	other/main-in-other
 	other/matching/branch-in-other
+	remote/with/slash/HEAD
+	remote/with/slash/branch/with/slash
 	matching-tag
 	matching/tag
 	EOF
@@ -1142,6 +1179,8 @@  test_expect_success '__git_complete_refs - simple' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	matching-tag Z
 	EOF
 	(
@@ -1180,6 +1219,20 @@  test_expect_success '__git_complete_refs - remote' '
 	test_cmp expected out
 '
 
+test_expect_success '__git_complete_refs - remote - with slash' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	HEAD Z
+	HEAD Z
+	branch/with/slash Z
+	EOF
+	(
+		cur= &&
+		__git_complete_refs --remote=remote/with/slash &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
 test_expect_success '__git_complete_refs - track' '
 	sed -e "s/Z$//" >expected <<-EOF &&
 	HEAD Z
@@ -1188,9 +1241,11 @@  test_expect_success '__git_complete_refs - track' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	matching-tag Z
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main-in-other Z
 	EOF
 	(
@@ -1235,6 +1290,8 @@  test_expect_success '__git_complete_refs - suffix' '
 	other/HEAD.
 	other/branch-in-other.
 	other/main-in-other.
+	remote/with/slash/HEAD.
+	remote/with/slash/branch/with/slash.
 	matching-tag.
 	EOF
 	(
@@ -1260,6 +1317,20 @@  test_expect_success '__git_complete_fetch_refspecs - simple' '
 	test_cmp expected out
 '
 
+test_expect_success '__git_complete_fetch_refspecs - with slash' '
+	sed -e "s/Z$//" >expected <<-EOF &&
+	HEAD:HEAD Z
+	HEAD:HEAD Z
+	branch/with/slash:branch/with/slash Z
+	EOF
+	(
+		cur= &&
+		__git_complete_fetch_refspecs remote/with/slash &&
+		print_comp
+	) &&
+	test_cmp expected out
+'
+
 test_expect_success '__git_complete_fetch_refspecs - matching' '
 	sed -e "s/Z$//" >expected <<-EOF &&
 	branch-in-other:branch-in-other Z
@@ -1340,8 +1411,8 @@  test_expect_success '__git_complete_worktree_paths with -C' '
 
 test_expect_success 'git switch - with no options, complete local branches and unique remote branch names for DWIM logic' '
 	test_completion "git switch " <<-\EOF
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -1487,8 +1558,8 @@  test_expect_success 'git-bisect - existing view subcommand is recognized and ena
 test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
 	test_completion "git checkout " <<-\EOF
 	HEAD Z
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -1496,6 +1567,8 @@  test_expect_success 'git checkout - completes refs and unique remote branches fo
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1515,8 +1588,8 @@  test_expect_success 'git switch - with GIT_COMPLETION_CHECKOUT_NO_GUESS=1, compl
 
 test_expect_success 'git switch - --guess overrides GIT_COMPLETION_CHECKOUT_NO_GUESS=1, complete local branches and unique remote names for DWIM logic' '
 	GIT_COMPLETION_CHECKOUT_NO_GUESS=1 test_completion "git switch --guess " <<-\EOF
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -1525,8 +1598,8 @@  test_expect_success 'git switch - --guess overrides GIT_COMPLETION_CHECKOUT_NO_G
 
 test_expect_success 'git switch - a later --guess overrides previous --no-guess, complete local and remote unique branches for DWIM' '
 	test_completion "git switch --no-guess --guess " <<-\EOF
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -1549,14 +1622,16 @@  test_expect_success 'git checkout - with GIT_COMPLETION_NO_GUESS=1 only complete
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
 test_expect_success 'git checkout - --guess overrides GIT_COMPLETION_NO_GUESS=1, complete refs and unique remote branches for DWIM' '
 	GIT_COMPLETION_CHECKOUT_NO_GUESS=1 test_completion "git checkout --guess " <<-\EOF
 	HEAD Z
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -1564,6 +1639,8 @@  test_expect_success 'git checkout - --guess overrides GIT_COMPLETION_NO_GUESS=1,
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1576,14 +1653,16 @@  test_expect_success 'git checkout - with --no-guess, only completes refs' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
 test_expect_success 'git checkout - a later --guess overrides previous --no-guess, complete refs and unique remote branches for DWIM' '
 	test_completion "git checkout --no-guess --guess " <<-\EOF
 	HEAD Z
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -1591,6 +1670,8 @@  test_expect_success 'git checkout - a later --guess overrides previous --no-gues
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1603,6 +1684,8 @@  test_expect_success 'git checkout - a later --no-guess overrides previous --gues
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1616,6 +1699,8 @@  test_expect_success 'git checkout - with checkout.guess = false, only completes
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1623,8 +1708,8 @@  test_expect_success 'git checkout - with checkout.guess = true, completes refs a
 	test_config checkout.guess true &&
 	test_completion "git checkout " <<-\EOF
 	HEAD Z
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -1632,6 +1717,8 @@  test_expect_success 'git checkout - with checkout.guess = true, completes refs a
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1639,8 +1726,8 @@  test_expect_success 'git checkout - a later --guess overrides previous checkout.
 	test_config checkout.guess false &&
 	test_completion "git checkout --guess " <<-\EOF
 	HEAD Z
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -1648,6 +1735,8 @@  test_expect_success 'git checkout - a later --guess overrides previous checkout.
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1661,6 +1750,8 @@  test_expect_success 'git checkout - a later --no-guess overrides previous checko
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1673,6 +1764,8 @@  test_expect_success 'git switch - with --detach, complete all references' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1685,6 +1778,8 @@  test_expect_success 'git checkout - with --detach, complete only references' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1857,6 +1952,8 @@  test_expect_success 'git switch - with -d, complete all references' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1869,6 +1966,8 @@  test_expect_success 'git checkout - with -d, complete only references' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1877,11 +1976,15 @@  test_expect_success 'git switch - with --track, complete only remote branches' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 	test_completion "git switch -t " <<-\EOF
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1890,11 +1993,15 @@  test_expect_success 'git checkout - with --track, complete only remote branches'
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 	test_completion "git checkout -t " <<-\EOF
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1914,6 +2021,8 @@  test_expect_success 'git checkout - with --no-track, complete only local referen
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1926,6 +2035,8 @@  test_expect_success 'git switch - with -c, complete all references' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1938,6 +2049,8 @@  test_expect_success 'git switch - with -C, complete all references' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1950,6 +2063,8 @@  test_expect_success 'git switch - with -c and --track, complete all references'
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1962,6 +2077,8 @@  test_expect_success 'git switch - with -C and --track, complete all references'
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1974,6 +2091,8 @@  test_expect_success 'git switch - with -c and --no-track, complete all reference
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1986,6 +2105,8 @@  test_expect_success 'git switch - with -C and --no-track, complete all reference
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -1998,6 +2119,8 @@  test_expect_success 'git checkout - with -b, complete all references' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -2010,6 +2133,8 @@  test_expect_success 'git checkout - with -B, complete all references' '
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -2022,6 +2147,8 @@  test_expect_success 'git checkout - with -b and --track, complete all references
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -2034,6 +2161,8 @@  test_expect_success 'git checkout - with -B and --track, complete all references
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -2046,6 +2175,8 @@  test_expect_success 'git checkout - with -b and --no-track, complete all referen
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -2058,13 +2189,15 @@  test_expect_success 'git checkout - with -B and --no-track, complete all referen
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
 test_expect_success 'git switch - for -c, complete local branches and unique remote branches' '
 	test_completion "git switch -c " <<-\EOF
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -2073,8 +2206,8 @@  test_expect_success 'git switch - for -c, complete local branches and unique rem
 
 test_expect_success 'git switch - for -C, complete local branches and unique remote branches' '
 	test_completion "git switch -C " <<-\EOF
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -2111,8 +2244,8 @@  test_expect_success 'git switch - for -C with --no-track, complete local branche
 
 test_expect_success 'git checkout - for -b, complete local branches and unique remote branches' '
 	test_completion "git checkout -b " <<-\EOF
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -2121,8 +2254,8 @@  test_expect_success 'git checkout - for -b, complete local branches and unique r
 
 test_expect_success 'git checkout - for -B, complete local branches and unique remote branches' '
 	test_completion "git checkout -B " <<-\EOF
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -2159,8 +2292,8 @@  test_expect_success 'git checkout - for -B with --no-track, complete local branc
 
 test_expect_success 'git switch - with --orphan completes local branch names and unique remote branch names' '
 	test_completion "git switch --orphan " <<-\EOF
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -2175,8 +2308,8 @@  test_expect_success 'git switch - --orphan with branch already provided complete
 
 test_expect_success 'git checkout - with --orphan completes local branch names and unique remote branch names' '
 	test_completion "git checkout --orphan " <<-\EOF
-	HEAD Z
 	branch-in-other Z
+	branch/with/slash Z
 	main Z
 	main-in-other Z
 	matching-branch Z
@@ -2192,6 +2325,8 @@  test_expect_success 'git checkout - --orphan with branch already provided comple
 	other/HEAD Z
 	other/branch-in-other Z
 	other/main-in-other Z
+	remote/with/slash/HEAD Z
+	remote/with/slash/branch/with/slash Z
 	EOF
 '
 
@@ -2206,7 +2341,8 @@  test_expect_success 'git restore completes modified files' '
 test_expect_success 'teardown after ref completion' '
 	git branch -d matching-branch &&
 	git tag -d matching-tag &&
-	git remote remove other
+	git remote remove other &&
+	git remote remove remote/with/slash
 '