Subject: Re: [PATCH] completion(switch/checkout): treat --track and -t the same

From: Junio C Hamano <[email-masked]>
Date: Fri, 08 Sep 2023 08:51:57 -0700

"Johannes Schindelin via GitGitGadget" <[email-masked]>
writes:

> From: Johannes Schindelin <[email-masked]>
>
> When `git switch --track ` is to be completed, only remote refs are
> eligible because that is what the `--track` option targets.

OK.  Presumably that is the same for "checkout --track".

> And when the short-hand `-t` is used instead, the same _should_ happen.

That sounds sensible.  The code change for _git_checkout() and
_git_switch() is surprisingly simple ;-)  "-t" was not handled any
specially at all and fell through to "refs" complation.

> Let's make it so.

Sounds good.

> Note that the bug exists both in the completions of `switch` and
> `completion`, even if it manifests in slightly different ways: While
> the completion of `git switch -t ` will not even look at remote refs,
> the completion of `git checkout -t ` will look at both remote _and_
> local refs. Both should look only at remote refs.

Correct.

> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 133ec92bfae..745dc901fbe 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1607,7 +1607,7 @@ _git_checkout ()
>  
>  		if [ -n "$(__git_find_on_cmdline "-b -B -d --detach --orphan")" ]; then
>  			__git_complete_refs --mode="refs"
> -		elif [ -n "$(__git_find_on_cmdline "--track")" ]; then
> +		elif [ -n "$(__git_find_on_cmdline "-t --track")" ]; then
>  			__git_complete_refs --mode="remote-heads"
>  		else
>  			__git_complete_refs $dwim_opt --mode="refs"
> @@ -2514,7 +2514,7 @@ _git_switch ()
>  
>  		if [ -n "$(__git_find_on_cmdline "-c -C -d --detach")" ]; then
>  			__git_complete_refs --mode="refs"
> -		elif [ -n "$(__git_find_on_cmdline "--track")" ]; then
> +		elif [ -n "$(__git_find_on_cmdline "-t --track")" ]; then
>  			__git_complete_refs --mode="remote-heads"
>  		else
>  			__git_complete_refs $dwim_opt --mode="heads"

The fallback behaviours are different, which was adequately
described in the proposed log message.  As "switch" does not want to
auto-detach upon receiving a ref that is not a local branch, while
"checkout" does, the difference is justifiable.

> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 8835e16e811..df8bc44c285 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '
>  '
>  
>  test_expect_success 'git switch - with --track, complete only remote branches' '
> -	test_completion "git switch --track " <<-\EOF
> +:	test_completion "git switch --track " <<-\EOF &&
> +	other/branch-in-other Z
> +	other/main-in-other Z
> +	EOF
> +	test_completion "git switch -t " <<-\EOF
>  	other/branch-in-other Z
>  	other/main-in-other Z
>  	EOF
>  '

So, this demonstrates that '-t' behaves the same way as '--track'.

>  test_expect_success 'git checkout - with --track, complete only remote branches' '
> -	test_completion "git checkout --track " <<-\EOF
> +	test_completion "git checkout --track " <<-\EOF &&
> +	other/branch-in-other Z
> +	other/main-in-other Z
> +	EOF
> +	test_completion "git checkout -t " <<-\EOF
>  	other/branch-in-other Z
>  	other/main-in-other Z
>  	EOF

This is, too.  If we had a test for "-t" without the code fix, we
would have seen local branches in its output, but now we can see the
candidates are limited to the remote ones.

Good.

Will queue.  Thanks.